[PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[Russell King (Oracle)]

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

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);
 
-- 
2.30.2

Re: [PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[Maxime Chevallier]

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

Re: [PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[Russell King (Oracle)]

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!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[Daniel Golle]

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

Re: [PATCH net] net: sfp-bus: fix SFP mode detect from bitrate

[Greg KH]

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