RE: [PATCH ethtool-next 08/14] cmis: Enable JSON output support in CMIS modules
From: Danielle Ratson <hidden>
Date: 2025-02-02 18:22:22
From: Jakub Kicinski <kuba@kernel.org> Sent: Thursday, 30 January 2025 18:25 To: Danielle Ratson <redacted> Cc: netdev@vger.kernel.org; mkubecek@suse.cz; matt@traverse.com.au; daniel.zahka@gmail.com; Amit Cohen [off-list ref]; NBU-mlxsw [off-list ref] Subject: Re: [PATCH ethtool-next 08/14] cmis: Enable JSON output support in CMIS modules On Thu, 30 Jan 2025 12:38:56 +0000 Danielle Ratson wrote:quoted
quoted
quoted
Yes, the unit is implied by the key is hardcoded. Same as for the regular output, it should give the costumer idea about the scale. There are also temperature fields that could be either F or C degrees. So overall , the units fields should align all the fields that implies some sort of scale.Some sort of a schema would be a better place to document the unit of the fields, IMO.So should the units fields be removed entirely? And only be documented in the json schema file?Yes, more than happy to hear from others but a schema file would be my first choice. Short of that as long as the unit is the same as in the plain text output there should also not be any ambiguity.
This is the output without the units fields:
$ ethtool -j -m swp23
[ {
"identifier": 24,
"identifier_description": "QSFP-DD Double Density 8X Pluggable Transceiver (INF-8628)",
"power_class": 5,
"max_power": 10,
"connector": 40,
"connector_description": "MPO 1x16",
"cable_assembly_length": 0,
"tx_cdr_bypass_control": false,
"rx_cdr_bypass_control": false,
"tx_cdr": true,
"rx_cdr": true,
"transmitter_technology": 0,
"transmitter_technology_description": "850 nm VCSEL",
"laser_wavelength": 850,
"laser_wavelength_tolerance": 94.8,
"length_(smf)": 0,
"length_(om5)": 0,
"length_(om4)": 100,
"length_(om3)": 70,
"length_(om2)": 0,
"vendor_name": "FINISAR CORP.",
"vendor_oui": [ 0,144,101 ],
"vendor_pn": "FTCD8613E1PCM",
"vendor_rev": "A0",
"vendor_sn": "X6LBE6H",
"date_code": "211229__",
"revision_compliance": {
"major": 4,
"minor": 0
},
"rx_loss_of_signal": [ "Yes","Yes","Yes","Yes","Yes","Yes","Yes","Yes" ],
"tx_loss_of_signal": false,
"rx_loss_of_lock": false,
"tx_loss_of_lock": false,
"tx_fault": false,
"module_state": 3,
"module_state_description": "ModuleReady",
"low_pwr_allow_request_hw": false,
"low_pwr_request_sw": false,
"module_temperature": 37.3477,
"module_voltage": 3.3406,
"laser_tx_bias_current": [ 0,0,0,0,0,0,0,0 ],
"transmit_avg_optical_power": [ 0.0001,0.0001,0.0001,0.0001,0.0001,0.0001,0.0001,0.0001 ],
"rx_power": {
"values": [ 0.0001,0.0001,0.0001,0.0001,0.0001,0.0001,0.0001,0.0001 ],
"type": "Rcvr signal avg optical power"
},
"module_temperature_high_alarm": false,
"module_temperature_low_alarm": false,
"module_temperature_high_warning": false,
"module_temperature_low_warning": false,
"module_voltage_high_alarm": false,
"module_voltage_low_alarm": false,
"module_voltage_high_warning": false,
"module_voltage_low_warning": false,
"laser_bias_current_high_alarm": [ false,false,false,false,false,false,false,false ],
"laser_bias_current_low_alarm": [ false,false,false,false,false,false,false,false ],
"laser_bias_current_high_warning": [ false,false,false,false,false,false,false,false ],
"laser_bias_current_low_warning": [ false,false,false,false,false,false,false,false ],
"laser_tx_power_high_alarm": [ false,false,false,false,false,false,false,false ],
"laser_tx_power_low_alarm": [ false,false,false,false,false,false,false,false ],
"laser_tx_power_high_warning": [ false,false,false,false,false,false,false,false ],
"laser_tx_power_low_warning": [ false,false,false,false,false,false,false,false ],
"laser_rx_power_high_alarm": [ false,false,false,false,false,false,false,false ],
"laser_rx_power_low_alarm": [ false,false,false,false,false,false,false,false ],
"laser_rx_power_high_warning": [ false,false,false,false,false,false,false,false ],
"laser_rx_power_low_warning": [ false,false,false,false,false,false,false,false ],
"laser_bias_current": {
"high_alarm_threshold": 13,
"low_alarm_threshold": 3,
"high_warning_threshold": 11,
"low_warning_threshold": 5
},
"laser_output_power": {
"high_alarm_threshold": 3.1623,
"low_alarm_threshold": 0.1,
"high_warning_threshold": 1.9953,
"low_warning_threshold": 0.1585
},
"module_temperature": {
"high_alarm_threshold": 75,
"low_alarm_threshold": -5,
"high_warning_threshold": 70,
"low_warning_threshold": 0
},
"module_voltage": {
"high_alarm_threshold": 3.465,
"low_alarm_threshold": 3.135,
"high_warning_threshold": 3.45,
"low_warning_threshold": 3.15
},
"laser_rx_power": {
"high_alarm_threshold": 3.1623,
"low_alarm_threshold": 0.0398,
"high_warning_threshold": 2.5119,
"low_warning_threshold": 0.0794
},
"active_firmware_version": {
"major": 2,
"minor": 7
},
"inactive_firmware_version": {
"major": 2,
"minor": 7
},
"cdb_instances": 1,
"cdb_background_mode": "Supported",
"cdb_epl_pages": 0,
"cdb_maximum_epl_rw_length": 128,
"cdb_maximum_lpl_rw_length": 128,
"cdb_trigger_method": "Single write"
} ]
And here is a schema file that should match the dump:
cat module_info.json
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"title": "Ethtool module-info JSON Output Schema",
"type": "array",
"items": {
"type": "object",
"properties": {
"br_nominal_units": {
"type": "string",
"enum": ["Mbps"] },
"length_(smf)_units": {
"type": "string",
"enum": ["km"] },
"length_(om5)_units": {
"type": "string",
"enum": ["m"] },
"length_(om4)_units": {
"type": "string",
"enum": ["m"] },
"length_(om3)_units": {
"type": "string",
"enum": ["m"] },
"length_(om2)_units": {
"type": "string",
"enum": ["m"] },
"length_(om1)_units": {
"type": "string",
"enum": ["m"] },
"length_(copper_or_active_cable)_units": {
"type": "string",
"enum": ["m"] },
"laser_wavelength_units": {
"type": "string",
"enum": ["nm"] },
"laser_wavelength_tolerance_units": {
"type": "string",
"enum": ["nm"] },
"module_temperature_units": {
"type": "string",
"enum": ["degrees C"] },
"module_voltage_units": {
"type": "string",
"enum": ["V"] },
"laser_tx_bias_current_units": {
"type": "string",
"enum": ["mA"] },
"transmit_avg_optical_power_units": {
"type": "string",
"enum": ["mW"] },
"rx_power": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["mW"] }
},
"required": ["units"]
},
"laser_bias_current": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["mA"] }
},
"required": ["units"]
},
"laser_output_power": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["mW"] }
},
"required": ["units"]
},
"module_temperature": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["degrees C"] }
},
"required": ["units"]
},
"module_voltage": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["V"] }
},
"required": ["units"]
},
"laser_rx_power": {
"type": "object",
"properties": {
"units": {
"type": "string",
"enum": ["mW"] }
},
"required": ["units"]
}
},
"required": [
"br_nominal_units", "length_(smf)_units",
"length_(om5)_units", "length_(om4)_units",
"length_(om3)_units", "length_(om2)_units",
"length_(om1)_units",
"length_(copper_or_active_cable)_units",
"laser_wavelength_units",
"laser_wavelength_tolerance_units",
"module_temperature_units", "module_voltage_units",
"laser_tx_bias_current_units",
"transmit_avg_optical_power_units", "rx_power",
"laser_bias_current", "laser_output_power",
"module_temperature", "module_voltage", "laser_rx_power"
]
}
}
Does it look reasonable to you? As I understand, this is a standalone file, the is only relevant for documentation, so nothing supposed to be missing besides removing the units fields from the code.
Thanks!
Danielle