Dear Paul,
thank you for your feedback!
On 15/10/2025 9:59 am, Paul Menzel wrote:
Am 14.10.25 um 06:18 schrieb Birger Koblitz:
Adds support for 10G-BX modules, i.e. 10GBit Ethernet over a single
strand
Single-Mode fiber
I’d use imperative mood, and add a dot/period at the end.
I will put this into the next patch-version.
@@ -1678,6 +1680,31 @@ int ixgbe_identify_sfp_module_generic(struct
ixgbe_hw *hw)
else
hw->phy.sfp_type =
ixgbe_sfp_type_1g_bx_core1;
+ /* Support Ethernet 10G-BX, checking the Bit Rate
+ * Nominal Value as per SFF-8472 to be 12.5 Gb/s (67h) and
+ * Single Mode fibre with at least 1km link length
+ */
+ } else if ((!comp_codes_10g) && (bitrate_nominal == 0x67) &&
+ (!(cable_tech & IXGBE_SFF_DA_PASSIVE_CABLE)) &&
+ (!(cable_tech & IXGBE_SFF_DA_ACTIVE_CABLE))) {
+ status = hw->phy.ops.read_i2c_eeprom(hw,
+ IXGBE_SFF_SM_LENGTH_KM,
+ &sm_length_km);
+ if (status != 0)
+ goto err_read_i2c_eeprom;
Should an error be logged?
This needs to be read in the context of the rest of the SFP
identification function. Several bytes of the EEPROM have already been
read for module identification by the existing code before reaching this
point, and failure is handled everywhere by the same goto. What will
happen if EEPROM reading fails is that an error message will be logged
that the Module is not supported. This is because the type is not filled
in and the module therefore considered unsupported. The actual error
(ret_val = -ENOENT) is ignored e.g. in
ixgbe_52599/ixgbe_init_phy_ops_82599(). The error logged is probably
good enough: the module cannot be positively identified and is not
enabled. I say good enough, because this is actually what is the case:
the EEPROM is broken and ther
+ status = hw->phy.ops.read_i2c_eeprom(hw,
+ IXGBE_SFF_SM_LENGTH_100M,
+ &sm_length_100m);
+ if (status != 0)
+ goto err_read_i2c_eeprom;
Should an error be logged?
Same here.
+ if (sm_length_km > 0 || sm_length_100m >= 10) {
+ if (hw->bus.lan_id == 0)
+ hw->phy.sfp_type =
+ ixgbe_sfp_type_10g_bx_core0;
+ else
+ hw->phy.sfp_type =
+ ixgbe_sfp_type_10g_bx_core1;
I’d prefer the ternary operator, if only the same variable is assigned
in both branches.
Me, too. But this is merely code that can be found verbosely the same in
several places before in this identification function, for each type of
module identified basically once. If the same code would be written
differently in this place, it would probably confuse readers who would
wonder what is different.
Cheers,
Birger
