Re: [PATCH ethtool 2/2] sff-common: Fix naming of JSON keys for thresholds

[Andrew Lunn <andrew@lunn.ch>]

> > As to ABI breakage, we have to consider, did this never work, so it
> > does not matter if we change it?
> > 
> > 1) Does the first patch suggest it has always been impossible to get
> > this part of the module dumped in JSON format?
> 
> Yes for SFP modules it was always impossible to get this part. But for
> QSFP and CMIS modules it should have worked before.

So we are potentially causing regressions, if anybody has an
application using these types of modules, and a forgiving JSON parser.

But the JSON is clearly broken, so we have to do something. The commit
message is important here, because if somebody does have a regression,
they find the commit which makes the change, we want to convince them
the consequences were considered, and they should just accept it,
rather than ask for a revert.

But we also have more flexibility for SFPs, if they never worked, we
can rename properties which are specific to them without issues. But
again, this should be explained in the commit message.

> For SFP modules it never worked, but for QSFP and CMIS modules it should
> have worked before. So I will try to minimize the effect for QSFP and
> CMIS modules.

Agreed

> For the biggest possible backward compatibility, I would suggest:
> * Keep the function sff_show_thresholds_json() as it is, so threshold
>   values remain unchanged in the JSON output
> * Only rename the measured values if needed to avoid duplicated keys
>   * Keep name "rx_power" for all module types
>   * Keep name "laser_tx_bias_current" for QSFP and CMIS modules
>   * Rename "laser_bias_current" to "laser_tx_bias_current" for SFP
>     modules
>   * Keep name "transmit_avg_optical_power" for QSFP and CMIS modules
>   * Rename "laser_output_power" to "transmit_avg_optical_power" for SFP
>     modules
>   * Rename "module_temperature" to "module_temperature_measurement" for
>     all modules
>   * Rename "module_voltage" to "module_voltage_measurement" for all
>     modules
> 
> This results in only two keys renamed for QSFP and CMIS modules. As a
> side effect this also aligns the key names for the different module
> types.

This is good.

Thanks for the research you did into this, looking at difference JSON
libraries, etc.

	Andrew