diff mbox series

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

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

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1082 this patch: 1082
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1096 this patch: 1096
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1097 this patch: 1097
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest pending net-next-2024-01-17--00-00

Commit Message

Russell King (Oracle) Jan. 15, 2024, 12:43 p.m. UTC
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(-)

Comments

Maxime Chevallier Jan. 15, 2024, 3:58 p.m. UTC | #1
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
Russell King (Oracle) Jan. 15, 2024, 4:07 p.m. UTC | #2
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!
patchwork-bot+netdevbpf@kernel.org Jan. 17, 2024, 2 a.m. UTC | #3
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!
Daniel Golle May 30, 2024, 10:39 a.m. UTC | #4
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
Greg KH June 12, 2024, 12:33 p.m. UTC | #5
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 mbox series

Patch

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