ethdev: fix SFF-8472 calibration overflow

PATCH Applied Roman Khromenok 2 participants 5 messages Oct 2

Share & tools

Roman Khromenok

author

2026-10-02 12:53 UTC #0 lore raw

cover ethdev: fix SFF-8472 calibration overflow

This follows up on the review of the SFF-8472 Rx power calibration fix, where the automated review pointed out the remaining issues in sff_8472_calibration().

Calibration formula 1 (temperature, voltage, Tx bias, Tx power) scales the 16-bit fields in place by a slope of up to 255.996 and adds a signed 16-bit offset. A result out of the field range is undefined behavior for the float to integer conversion; a negative sum wraps around. UBSan reports float-cast-overflow on randomly generated externally calibrated SFP dumps.

Patch 1 computes formula 1 in double precision and saturates the result to the field range, as already done for the Rx power. It is intended for stable.
Patch 2 rounds the calibrated values to the nearest integer instead of truncating them: RX_PWR(1) = 0.7 is stored as 0.69999999 and a raw reading of 1000 gave 699 instead of 700.
Patch 3 adds unit tests for saturation at both ends and for rounding.

Based on dpdk-next-net for-main.

Depends-on: series-39455 ("ethdev: fix SFF-8472 external Rx power calibration")

Roman Khromenok (3):
ethdev: fix SFF-8472 calibration overflow
ethdev: round SFF-8472 calibrated values
test: check SFF-8472 calibration saturation and rounding

 app/test/test_ethdev_module_eeprom.c | 96 ++++++++++++++++++++++++++++
 lib/ethdev/sff_8472.c                | 55 +++++++++++-----
 2 files changed, 134 insertions(+), 17 deletions(-)

Signature · 1 line

2.47.3

Roman Khromenok

author

2026-10-02 12:53 UTC #1 lore raw

PATCH 1/3 ethdev: fix SFF-8472 calibration overflow

With external calibration, SFF-8472 computes temperature, voltage, TX bias and TX power as slope * x + offset, where the slope goes up to 255.996 and the offset is a signed 16-bit integer.

The decoder multiplied the 16-bit fields in place by the slope and then added the offset. A result out of the field range is undefined behavior for the float to integer conversion, and an integer sum below zero wraps around: a bias reading of 100 with an offset of -1000 was reported as 129.272 mA.

Compute the formula in double precision and saturate the result to the range of the field, as already done for the RX power.

Fixes 0caf7f376b08 ("ethdev: support SFF-8472 module telemetry")

Cc [email protected]

Signed-off-by Roman Khromenok <[email protected]>

---
 lib/ethdev/sff_8472.c | 51 ++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 17 deletions(-)

lib/ethdev/sff_8472.c +34 ~~-17~~

index 3c73a829a8..8cd41161e3 100644
@@ -186,6 +186,25 @@ static float befloattoh(const uint8_t *source)
 	return converter.dst;
 }
 
+/* Calibrated values are stored in 16-bit fields, saturate the out of range ones */
+static uint16_t sff_8472_cal_to_u16(double value)
+{
+	if (!(value > 0))
+		return 0;
+	if (value >= UINT16_MAX)
+		return UINT16_MAX;
+	return value;
+}
+
+static int16_t sff_8472_cal_to_s16(double value)
+{
+	if (value <= INT16_MIN)
+		return INT16_MIN;
+	if (value >= INT16_MAX)
+		return INT16_MAX;
+	return value;
+}
+
 static void sff_8472_calibration(const uint8_t *data, struct sff_diags *sd)
 {
 	unsigned long i;

@@ -195,17 +214,21 @@ static void sff_8472_calibration(const uint8_t *data, struct sff_diags *sd)
 	/* Calibration should occur for all values (threshold and current) */
 	for (i = 0; i < RTE_DIM(sd->bias_cur); ++i) {
 		/*
-		 * Apply calibration formula 1 (Temp., Voltage, Bias, Tx Power)
+		 * Apply calibration formula 1 (Temp., Voltage, Bias, Tx Power):
+		 * slope * x + offset
 		 */
-		sd->bias_cur[i]    *= A2_OFFSET_TO_SLP(SFF_A2_CAL_TXI_SLP);
-		sd->tx_power[i]    *= A2_OFFSET_TO_SLP(SFF_A2_CAL_TXPWR_SLP);
-		sd->sfp_voltage[i] *= A2_OFFSET_TO_SLP(SFF_A2_CAL_V_SLP);
-		sd->sfp_temp[i]    *= A2_OFFSET_TO_SLP(SFF_A2_CAL_T_SLP);
-
-		sd->bias_cur[i]    += A2_OFFSET_TO_OFF(SFF_A2_CAL_TXI_OFF);
-		sd->tx_power[i]    += A2_OFFSET_TO_OFF(SFF_A2_CAL_TXPWR_OFF);
-		sd->sfp_voltage[i] += A2_OFFSET_TO_OFF(SFF_A2_CAL_V_OFF);
-		sd->sfp_temp[i]    += A2_OFFSET_TO_OFF(SFF_A2_CAL_T_OFF);
+		sd->bias_cur[i] = sff_8472_cal_to_u16(sd->bias_cur[i] *
+			A2_OFFSET_TO_SLP(SFF_A2_CAL_TXI_SLP) +
+			A2_OFFSET_TO_OFF(SFF_A2_CAL_TXI_OFF));
+		sd->tx_power[i] = sff_8472_cal_to_u16(sd->tx_power[i] *
+			A2_OFFSET_TO_SLP(SFF_A2_CAL_TXPWR_SLP) +
+			A2_OFFSET_TO_OFF(SFF_A2_CAL_TXPWR_OFF));
+		sd->sfp_voltage[i] = sff_8472_cal_to_u16(sd->sfp_voltage[i] *
+			A2_OFFSET_TO_SLP(SFF_A2_CAL_V_SLP) +
+			A2_OFFSET_TO_OFF(SFF_A2_CAL_V_OFF));
+		sd->sfp_temp[i] = sff_8472_cal_to_s16(sd->sfp_temp[i] *
+			A2_OFFSET_TO_SLP(SFF_A2_CAL_T_SLP) +
+			A2_OFFSET_TO_OFF(SFF_A2_CAL_T_OFF));
 
 		/*
 		 * Apply calibration formula 2 (Rx Power only):

@@ -219,13 +242,7 @@ static void sff_8472_calibration(const uint8_t *data, struct sff_diags *sd)
 		rx_power = rx_power * rx_reading + A2_OFFSET_TO_RXPWRx(SFF_A2_CAL_RXPWR1);
 		rx_power = rx_power * rx_reading + A2_OFFSET_TO_RXPWRx(SFF_A2_CAL_RXPWR0);
 
-		/* the result is stored in 0.1 uW units, out of range is not representable */
-		if (!(rx_power > 0))
-			sd->rx_power[i] = 0;
-		else if (rx_power >= UINT16_MAX)
-			sd->rx_power[i] = UINT16_MAX;
-		else
-			sd->rx_power[i] = rx_power;
+		sd->rx_power[i] = sff_8472_cal_to_u16(rx_power);
 	}
 }

Signature · 1 line

2.47.3

Roman Khromenok

author

2026-10-02 12:53 UTC #2 lore raw

PATCH 2/3 ethdev: round SFF-8472 calibrated values

In reply to Roman Khromenok

The externally calibrated SFF-8472 values were truncated when stored in their 16-bit fields. Coefficients that are not exact in binary floating point lose one unit this way: RX_PWR(1) = 0.7 is stored as 0.69999999, and a raw reading of 1000 gave 699 instead of 700.

Round the calibrated values to the nearest integer instead.

Signed-off-by Roman Khromenok <[email protected]>

---
 lib/ethdev/sff_8472.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

lib/ethdev/sff_8472.c +7 -3

index 8cd41161e3..88d1f4a0a5 100644
@@ -3,6 +3,7 @@
  * Implements SFF-8472 optics diagnostics.
  */
 
+#include <math.h>
 #include <stdio.h>
 #include <string.h>

@@ -186,14 +187,17 @@ static float befloattoh(const uint8_t *source)
 	return converter.dst;
 }
 
-/* Calibrated values are stored in 16-bit fields, saturate the out of range ones */
+/*
+ * Calibrated values are stored in 16-bit fields:
+ * round to the nearest integer and saturate the out of range ones.
+ */
 static uint16_t sff_8472_cal_to_u16(double value)
 {
 	if (!(value > 0))
 		return 0;
 	if (value >= UINT16_MAX)
 		return UINT16_MAX;
-	return value;
+	return lround(value);
 }
 
 static int16_t sff_8472_cal_to_s16(double value)

@@ -202,7 +206,7 @@ static int16_t sff_8472_cal_to_s16(double value)
 		return INT16_MIN;
 	if (value >= INT16_MAX)
 		return INT16_MAX;
-	return value;
+	return lround(value);
 }
 
 static void sff_8472_calibration(const uint8_t *data, struct sff_diags *sd)

Signature · 1 line

2.47.3

Roman Khromenok

author

2026-10-02 12:53 UTC #3 lore raw

PATCH 3/3 test: check SFF-8472 calibration saturation and rounding

In reply to Roman Khromenok

Check that externally calibrated SFF-8472 values out of the range of their 16-bit fields saturate at both ends, for formula 1 (temperature, voltage, TX bias and TX power) and for the RX power polynomial, and that calibrated values are rounded to the nearest.

Signed-off-by Roman Khromenok <[email protected]>

---
 app/test/test_ethdev_module_eeprom.c | 96 ++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

app/test/test_ethdev_module_eeprom.c +96 -0

index b07a18aa06..1956fdd440 100644
@@ -305,6 +305,99 @@ test_module_eeprom_sfp_8472_rx_power_polynomial(void)
 	return TEST_SUCCESS;
 }
 
+/* SFF-8472 module externally calibrated with unit slopes and zero offsets */
+static void
+fill_sfp_ext_cal(uint8_t *data)
+{
+	uint8_t *a2 = data + RTE_ETH_MODULE_SFF_8079_LEN;
+
+	fill_sfp(data);
+	/* diagnostics implemented, externally calibrated, average RX power */
+	data[92] = 0x58;
+	a2[76] = 1;	/* TX bias slope */
+	a2[80] = 1;	/* TX power slope */
+	a2[84] = 1;	/* temperature slope */
+	a2[88] = 1;	/* voltage slope */
+	put_be_float(a2, 68, 0x3f800000);	/* RX_PWR(1) = 1.0 */
+}
+
+static int
+test_module_eeprom_sfp_8472_cal_saturate_max(void)
+{
+	uint8_t data[RTE_ETH_MODULE_SFF_8472_LEN];
+	uint8_t *a2 = data + RTE_ETH_MODULE_SFF_8079_LEN;
+
+	fill_sfp_ext_cal(data);
+	/* results above the 16-bit range must saturate */
+	put_u16(a2, 100, 65535);	/* raw TX bias */
+	put_u16(a2, 76, 0xffff);	/* TX bias slope = 255.996 */
+	put_u16(a2, 102, 60000);	/* raw TX power */
+	put_u16(a2, 82, 10000);		/* TX power offset */
+	put_u16(a2, 96, 0x7f00);	/* raw temperature: 127 C */
+	put_u16(a2, 84, 0x0200);	/* temperature slope = 2.0 */
+	put_u16(a2, 98, 60000);		/* raw voltage */
+	put_u16(a2, 90, 10000);		/* voltage offset */
+	put_u16(a2, 104, 65535);	/* raw RX power */
+	put_be_float(a2, 56, 0x3f800000);	/* RX_PWR(4) = 1.0 */
+
+	TEST_ASSERT_SUCCESS(parse(RTE_ETH_MODULE_SFF_8472, data, sizeof(data)),
+		"Failed to parse externally calibrated SFF-8472 data");
+	CHECK_FIELD("Laser bias current", "131.070 mA");
+	CHECK_FIELD("Laser output power", "6.5535 mW / 8.16 dBm");
+	CHECK_FIELD("Module temperature", "128.00 degrees C / 262.39 degrees F");
+	CHECK_FIELD("Module voltage", "6.5535 V");
+	CHECK_FIELD("Receiver signal average optical power", "6.5535 mW / 8.16 dBm");
+	return TEST_SUCCESS;
+}
+
+static int
+test_module_eeprom_sfp_8472_cal_saturate_min(void)
+{
+	uint8_t data[RTE_ETH_MODULE_SFF_8472_LEN];
+	uint8_t *a2 = data + RTE_ETH_MODULE_SFF_8079_LEN;
+
+	fill_sfp_ext_cal(data);
+	/* results below the 16-bit range must saturate */
+	put_u16(a2, 100, 100);		/* raw TX bias */
+	put_u16(a2, 78, -1000);		/* TX bias offset */
+	put_u16(a2, 102, 100);		/* raw TX power */
+	put_u16(a2, 82, -1000);		/* TX power offset */
+	put_u16(a2, 96, 0x8100);	/* raw temperature: -127 C */
+	put_u16(a2, 84, 0x0200);	/* temperature slope = 2.0 */
+	put_u16(a2, 98, 100);		/* raw voltage */
+	put_u16(a2, 90, -1000);		/* voltage offset */
+	put_be_float(a2, 68, 0xbf800000);	/* RX_PWR(1) = -1.0 */
+
+	TEST_ASSERT_SUCCESS(parse(RTE_ETH_MODULE_SFF_8472, data, sizeof(data)),
+		"Failed to parse externally calibrated SFF-8472 data");
+	CHECK_FIELD("Laser bias current", "0.000 mA");
+	CHECK_FIELD("Laser output power", "0.0000 mW / -inf dBm");
+	CHECK_FIELD("Module temperature", "-128.00 degrees C / -198.40 degrees F");
+	CHECK_FIELD("Module voltage", "0.0000 V");
+	CHECK_FIELD("Receiver signal average optical power", "0.0000 mW / -inf dBm");
+	return TEST_SUCCESS;
+}
+
+static int
+test_module_eeprom_sfp_8472_cal_round(void)
+{
+	uint8_t data[RTE_ETH_MODULE_SFF_8472_LEN];
+	uint8_t *a2 = data + RTE_ETH_MODULE_SFF_8079_LEN;
+
+	fill_sfp_ext_cal(data);
+	/* calibrated values must be rounded to the nearest, not truncated */
+	put_u16(a2, 100, 3);		/* raw TX bias */
+	put_u16(a2, 76, 0x0180);	/* TX bias slope = 1.5 */
+	put_u16(a2, 104, 1000);		/* raw RX power */
+	put_be_float(a2, 68, 0x3f333333);	/* RX_PWR(1) = 0.7, stored as 0.69999999 */
+
+	TEST_ASSERT_SUCCESS(parse(RTE_ETH_MODULE_SFF_8472, data, sizeof(data)),
+		"Failed to parse externally calibrated SFF-8472 data");
+	CHECK_FIELD("Laser bias current", "0.010 mA");
+	CHECK_FIELD("Receiver signal average optical power", "0.0700 mW / -11.55 dBm");
+	return TEST_SUCCESS;
+}
+
 static int
 test_module_eeprom_invalid(void)
 {

@@ -355,6 +448,9 @@ static struct unit_test_suite module_eeprom_testsuite = {
 		TEST_CASE(test_module_eeprom_sfp_8472_short),
 		TEST_CASE(test_module_eeprom_sfp_8472_ext_cal_unaligned),
 		TEST_CASE(test_module_eeprom_sfp_8472_rx_power_polynomial),
+		TEST_CASE(test_module_eeprom_sfp_8472_cal_saturate_max),
+		TEST_CASE(test_module_eeprom_sfp_8472_cal_saturate_min),
+		TEST_CASE(test_module_eeprom_sfp_8472_cal_round),
 		TEST_CASE(test_module_eeprom_qsfp_8636),
 		TEST_CASE(test_module_eeprom_qsfp_8636_thresholds),
 		TEST_CASE(test_module_eeprom_invalid),

Signature · 1 line

2.47.3

Stephen Hemminger

Applied

2026-10-02 17:18 UTC #4 lore raw

In reply to Roman Khromenok

Quoted 19 lines · On Fri, 2 Oct 2026 14:52:34 +0200 Roman Khromenok <[email protected]> wrote:

This follows up on the review of the SFF-8472 Rx power calibration fix, where the automated review pointed out the remaining issues in sff_8472_calibration().
Calibration formula 1 (temperature, voltage, Tx bias, Tx power) scales the 16-bit fields in place by a slope of up to 255.996 and adds a signed 16-bit offset. A result out of the field range is undefined behavior for the float to integer conversion; a negative sum wraps around. UBSan reports float-cast-overflow on randomly generated externally calibrated SFP dumps.
Patch 1 computes formula 1 in double precision and saturates the result to the field range, as already done for the Rx power. It is intended for stable.
Patch 2 rounds the calibrated values to the nearest integer instead of truncating them: RX_PWR(1) = 0.7 is stored as 0.69999999 and a raw reading of 1000 gave 699 instead of 700.
Patch 3 adds unit tests for saturation at both ends and for rounding.

Quoted 3 lines

Based on dpdk-next-net for-main.
Depends-on: series-39455 ("ethdev: fix SFF-8472 external Rx power calibration")

Applied to next-net