| Message ID | E1rPMJW-001Ahf-L0@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 97eb5d51b4a584a60e5d096bdb6b33edc9f50d8d |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: sfp-bus: fix SFP mode detect from bitrate | expand |
Hello Russell, On Mon, 15 Jan 2024 12:43:38 +0000 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > The referenced commit moved the setting of the Autoneg and pause bits > early in sfp_parse_support(). However, we check whether the modes are > empty before using the bitrate to set some modes. Setting these bits > so early causes that test to always be false, preventing this working, > and thus some modules that used to work no longer do. > > Move them just before the call to the quirk. > > Fixes: 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits") > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> I don't have modules to trigger the bug, however the fix looks OK to me. Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Thanks, Maxime
On Mon, Jan 15, 2024 at 04:58:48PM +0100, Maxime Chevallier wrote: > Hello Russell, > > On Mon, 15 Jan 2024 12:43:38 +0000 > "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > > > The referenced commit moved the setting of the Autoneg and pause bits > > early in sfp_parse_support(). However, we check whether the modes are > > empty before using the bitrate to set some modes. Setting these bits > > so early causes that test to always be false, preventing this working, > > and thus some modules that used to work no longer do. > > > > Move them just before the call to the quirk. > > > > Fixes: 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits") > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > I don't have modules to trigger the bug, however the fix looks OK to me. > > Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> In case there's interest: Identifier : 0x03 (SFP) Extended identifier : 0x04 (GBIC/SFP defined by 2- wire interface ID) Connector : 0x07 (LC) Transceiver codes : 0x00 0x00 0x00 0x00 0x20 0x1 0 0x01 0x00 0x00 Transceiver type : FC: intermediate distance (I ) Transceiver type : FC: Longwave laser (LL) Transceiver type : FC: Single Mode (SM) Encoding : 0x01 (8B/10B) BR, Nominal : 1300MBd ... Laser wavelength : 1550nm Vendor name : FiberStore Vendor OUI : 00:00:00 Vendor PN : SFP-GE-BX Vendor rev : A0 which is the module I use for my internet connectivity between the critical internal systems and the rest of the planet... so the regression got noticed when I upgraded the kernel on Friday!
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 15 Jan 2024 12:43:38 +0000 you wrote: > The referenced commit moved the setting of the Autoneg and pause bits > early in sfp_parse_support(). However, we check whether the modes are > empty before using the bitrate to set some modes. Setting these bits > so early causes that test to always be false, preventing this working, > and thus some modules that used to work no longer do. > > Move them just before the call to the quirk. > > [...] Here is the summary with links: - [net] net: sfp-bus: fix SFP mode detect from bitrate https://git.kernel.org/netdev/net/c/97eb5d51b4a5 You are awesome, thank you!
Hi stable team, > > On Mon, 15 Jan 2024 12:43:38 +0000 > > "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > > > > > The referenced commit moved the setting of the Autoneg and pause bits > > > early in sfp_parse_support(). However, we check whether the modes are > > > empty before using the bitrate to set some modes. Setting these bits > > > so early causes that test to always be false, preventing this working, > > > and thus some modules that used to work no longer do. > > > > > > Move them just before the call to the quirk. > > > > > > Fixes: 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits") > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> please apply this patch also to Linux stable down to v6.4 which are affected by problems introduced by commit 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits"). The fix has been applied to net tree as commit 97eb5d51b4a5 ("net: sfp-bus: fix SFP mode detect from bitrate") but never picked for older kernel versions affected as well. Thank you! Daniel
On Thu, May 30, 2024 at 11:39:55AM +0100, Daniel Golle wrote: > Hi stable team, > > > > On Mon, 15 Jan 2024 12:43:38 +0000 > > > "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > > > > > > > The referenced commit moved the setting of the Autoneg and pause bits > > > > early in sfp_parse_support(). However, we check whether the modes are > > > > empty before using the bitrate to set some modes. Setting these bits > > > > so early causes that test to always be false, preventing this working, > > > > and thus some modules that used to work no longer do. > > > > > > > > Move them just before the call to the quirk. > > > > > > > > Fixes: 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits") > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > please apply this patch also to Linux stable down to v6.4 which are > affected by problems introduced by commit 8110633db49d ("net: sfp-bus: > allow SFP quirks to override Autoneg and pause bits"). > > The fix has been applied to net tree as commit 97eb5d51b4a5 ("net: > sfp-bus: fix SFP mode detect from bitrate") but never picked for older > kernel versions affected as well. Ok, applied to 6.6 only. greg k-h
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c index 6fa679b36290..db39dec7f247 100644 --- a/drivers/net/phy/sfp-bus.c +++ b/drivers/net/phy/sfp-bus.c @@ -151,10 +151,6 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id, unsigned int br_min, br_nom, br_max; __ETHTOOL_DECLARE_LINK_MODE_MASK(modes) = { 0, }; - phylink_set(modes, Autoneg); - phylink_set(modes, Pause); - phylink_set(modes, Asym_Pause); - /* Decode the bitrate information to MBd */ br_min = br_nom = br_max = 0; if (id->base.br_nominal) { @@ -339,6 +335,10 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id, } } + phylink_set(modes, Autoneg); + phylink_set(modes, Pause); + phylink_set(modes, Asym_Pause); + if (bus->sfp_quirk && bus->sfp_quirk->modes) bus->sfp_quirk->modes(id, modes, interfaces);
The referenced commit moved the setting of the Autoneg and pause bits early in sfp_parse_support(). However, we check whether the modes are empty before using the bitrate to set some modes. Setting these bits so early causes that test to always be false, preventing this working, and thus some modules that used to work no longer do. Move them just before the call to the quirk. Fixes: 8110633db49d ("net: sfp-bus: allow SFP quirks to override Autoneg and pause bits") Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/sfp-bus.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)