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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <af10599b-a698-4a60-a7d2-35a898b9a04a@lunn.ch>
Date: Thu, 23 Oct 2025 15:14:15 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Johannes Eigner <johannes.eigner@...berle.de>
Cc: netdev@...r.kernel.org, Michal Kubecek <mkubecek@...e.cz>,
	Danielle Ratson <danieller@...dia.com>,
	Stephan Wurm <stephan.wurm@...berle.de>
Subject: Re: [PATCH ethtool 2/2] sff-common: Fix naming of JSON keys for
 thresholds

> > 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