[PATCH 0/3] ethdev: fix SFF-8472 calibration overflow

[PATCH 0/3] ethdev: fix SFF-8472 calibration overflow

[Roman Khromenok]

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

-- 
2.47.3

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

[Roman Khromenok]

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: stable@dpdk.org

Signed-off-by: Roman Khromenok <roma55592@yandex.ru>
---
 lib/ethdev/sff_8472.c | 51 ++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/lib/ethdev/sff_8472.c b/lib/ethdev/sff_8472.c
index 3c73a829a8..8cd41161e3 100644
--- a/lib/ethdev/sff_8472.c
+++ b/lib/ethdev/sff_8472.c
@@ -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);
 	}
 }
 
-- 
2.47.3

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

[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 <roma55592@yandex.ru>
---
 lib/ethdev/sff_8472.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/ethdev/sff_8472.c b/lib/ethdev/sff_8472.c
index 8cd41161e3..88d1f4a0a5 100644
--- a/lib/ethdev/sff_8472.c
+++ b/lib/ethdev/sff_8472.c
@@ -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)
-- 
2.47.3

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

[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 <roma55592@yandex.ru>
---
 app/test/test_ethdev_module_eeprom.c | 96 ++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/app/test/test_ethdev_module_eeprom.c b/app/test/test_ethdev_module_eeprom.c
index b07a18aa06..1956fdd440 100644
--- a/app/test/test_ethdev_module_eeprom.c
+++ b/app/test/test_ethdev_module_eeprom.c
@@ -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),
-- 
2.47.3

Re: [PATCH 0/3] ethdev: fix SFF-8472 calibration overflow

[Stephen Hemminger]

On Fri,  2 Oct 2026 14:52:34 +0200
Roman Khromenok <roma55592@yandex.ru> 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.
> 
> Based on dpdk-next-net for-main.
> 
> Depends-on: series-39455 ("ethdev: fix SFF-8472 external Rx power calibration")

 Applied to next-net