Skip to contentSkip to conversation

net: phylink: support !autoneg configuration for SFPs

PATCH Applied Russell King (Oracle) 6 participants 18 messages Jul 2, 2025 – Jul 10, 2025

Subsystems (4)
lore
Share & tools
lore

Russell King (Oracle)

author maintainer
#0 lore raw

cover net: phylink: support !autoneg configuration for SFPs

This series comes from discussion during a patch series that was posted at the beginning of April, but these patches were never posted (I was too busy!)

We restrict ->sfp_interfaces to those that the host system supports, and ensure that ->sfp_interfaces is cleared when a SFP is removed. We then add phylink_sfp_select_interface_speed() which will select an appropriate interface from ->sfp_interfaces for the speed, and use that in our phylink_ethtool_ksettings_set() when a SFP bus is present on a directly connected host (not with a PHY.)

 drivers/net/phy/phylink.c | 60 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 6 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

PATCH 1/3 net: phylink: restrict SFP interfaces to those that are supported

When configuring an optical SFP interface, restrict the bitmap of SFP interfaces (pl->sfp_interfaces) to those that are supported by the host, rather than calculating this in a local variable.

This will allow us to avoid recomputing this in the phylink_ethtool_ksettings_set() path.

---
 drivers/net/phy/phylink.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/net/phy/phylink.c +4 -5
index 67218d278ce6..6420e76f8ab1 100644
@@ -3582,7 +3582,6 @@ static int phylink_sfp_config_phy(struct phylink *pl, struct phy_device *phy)
 static int phylink_sfp_config_optical(struct phylink *pl)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
 	struct phylink_link_state config;
 	phy_interface_t interface;
 	int ret;
@@ -3596,9 +3595,9 @@ static int phylink_sfp_config_optical(struct phylink *pl)
 	/* Find the union of the supported interfaces by the PCS/MAC and
 	 * the SFP module.
 	 */
-	phy_interface_and(interfaces, pl->config->supported_interfaces,
+	phy_interface_and(pl->sfp_interfaces, pl->config->supported_interfaces,
 			  pl->sfp_interfaces);
-	if (phy_interface_empty(interfaces)) {
+	if (phy_interface_empty(pl->sfp_interfaces)) {
 		phylink_err(pl, "unsupported SFP module: no common interface modes\n");
 		return -EINVAL;
 	}
@@ -3614,14 +3613,14 @@ static int phylink_sfp_config_optical(struct phylink *pl)
 	 * mask to only those link modes that can be supported.
 	 */
 	ret = phylink_validate_mask(pl, NULL, pl->sfp_support, &config,
-				    interfaces);
+				    pl->sfp_interfaces);
 	if (ret) {
 		phylink_err(pl, "unsupported SFP module: validation with support %*pb failed\n",
 			    __ETHTOOL_LINK_MODE_MASK_NBITS, support);
 		return ret;
 	}
 
-	interface = phylink_choose_sfp_interface(pl, interfaces);
+	interface = phylink_choose_sfp_interface(pl, pl->sfp_interfaces);
 	if (interface == PHY_INTERFACE_MODE_NA) {
 		phylink_err(pl, "failed to select SFP interface\n");
 		return -EINVAL;
Signature · 1 line
2.30.2
Reviewed-by Tested-by on v1 1/3
#2 lore raw

Re: [PATCH net-next 1/3] net: phylink: restrict SFP interfaces to those that are supported

Quoted 8 lines · On Wed, 02 Jul 2025 10:44:24 +0100 "Russell King (Oracle)"…

When configuring an optical SFP interface, restrict the bitmap of SFP interfaces (pl->sfp_interfaces) to those that are supported by the host, rather than calculating this in a local variable.

This will allow us to avoid recomputing this in the phylink_ethtool_ksettings_set() path.

Thanks Russell,

Maxime

#3 lore raw

PATCH 2/3 net: phylink: clear SFP interfaces when not in use

In reply to Russell King (Oracle)

Clear the SFP interfaces bitmap when we're not using it - in other words, when a module is unplugged, or we're using a PHY on the module.

---
 drivers/net/phy/phylink.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/net/phy/phylink.c +10 -0
index 6420e76f8ab1..c92a878ab717 100644
@@ -3536,6 +3536,8 @@ static int phylink_sfp_config_phy(struct phylink *pl, struct phy_device *phy)
 	struct phylink_link_state config;
 	int ret;
 
+	/* We're not using pl->sfp_interfaces, so clear it. */
+	phy_interface_zero(pl->sfp_interfaces);
 	linkmode_copy(support, phy->supported);
 
 	memset(&config, 0, sizeof(config));
@@ -3673,6 +3675,13 @@ static int phylink_sfp_module_insert(void *upstream,
 	return phylink_sfp_config_optical(pl);
 }
 
+static void phylink_sfp_module_remove(void *upstream)
+{
+	struct phylink *pl = upstream;
+
+	phy_interface_zero(pl->sfp_interfaces);
+}
+
 static int phylink_sfp_module_start(void *upstream)
 {
 	struct phylink *pl = upstream;
@@ -3757,6 +3766,7 @@ static const struct sfp_upstream_ops sfp_phylink_ops = {
 	.attach = phylink_sfp_attach,
 	.detach = phylink_sfp_detach,
 	.module_insert = phylink_sfp_module_insert,
+	.module_remove = phylink_sfp_module_remove,
 	.module_start = phylink_sfp_module_start,
 	.module_stop = phylink_sfp_module_stop,
 	.link_up = phylink_sfp_link_up,
Signature · 1 line
2.30.2
Reviewed-by Tested-by on v1 2/3
#4 lore raw

Re: [PATCH net-next 2/3] net: phylink: clear SFP interfaces when not in use

Quoted 5 lines · On Wed, 02 Jul 2025 10:44:29 +0100 "Russell King (Oracle)"…

Clear the SFP interfaces bitmap when we're not using it - in other words, when a module is unplugged, or we're using a PHY on the module.

Thanks,

Maxime

#5 lore raw

PATCH 3/3 net: phylink: add phylink_sfp_select_interface_speed()

In reply to Russell King (Oracle)

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

---
 drivers/net/phy/phylink.c | 41 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)
drivers/net/phy/phylink.c +40 -1
index c92a878ab717..f5473510b762 100644
@@ -2709,6 +2709,39 @@ static phy_interface_t phylink_sfp_select_interface(struct phylink *pl,
 	return interface;
 }
 
+static phy_interface_t phylink_sfp_select_interface_speed(struct phylink *pl,
+							  u32 speed)
+{
+	phy_interface_t best_interface = PHY_INTERFACE_MODE_NA;
+	phy_interface_t interface;
+	u32 max_speed;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(phylink_sfp_interface_preference); i++) {
+		interface = phylink_sfp_interface_preference[i];
+		if (!test_bit(interface, pl->sfp_interfaces))
+			continue;
+
+		max_speed = phylink_interface_max_speed(interface);
+
+		/* The logic here is: if speed == max_speed, then we've found
+		 * the best interface. Otherwise we find the interface that
+		 * can just support the requested speed.
+		 */
+		if (max_speed >= speed)
+			best_interface = interface;
+
+		if (max_speed <= speed)
+			break;
+	}
+
+	if (best_interface == PHY_INTERFACE_MODE_NA)
+		phylink_err(pl, "selection of interface failed, speed %u\n",
+			    speed);
+
+	return best_interface;
+}
+
 static void phylink_merge_link_mode(unsigned long *dst, const unsigned long *b)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask);
@@ -2911,8 +2944,14 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 	 * link can be configured correctly.
 	 */
 	if (pl->sfp_bus) {
-		config.interface = phylink_sfp_select_interface(pl,
+		if (kset->base.autoneg == AUTONEG_ENABLE)
+			config.interface =
+				phylink_sfp_select_interface(pl,
 							config.advertising);
+		else
+			config.interface =
+				phylink_sfp_select_interface_speed(pl,
+							config.speed);
 		if (config.interface == PHY_INTERFACE_MODE_NA)
 			return -EINVAL;
 
Signature · 1 line
2.30.2
Reviewed-by on v1 3/3
#6 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 6 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

I don't have any hardware to perform relevant tests on this :(

Maxime

author maintainer
#7 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 8 lines · Maxime Chevallier
Quoted 5 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

Quoted 5 lines
Quoted 1 line

I don't have any hardware to perform relevant tests on this :(

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#8 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 12 lines · Russell King (Oracle)
Quoted 10 lines · Maxime Chevallier
Quoted 6 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

Quoted 7 lines
Quoted 3 lines

I don't have any hardware to perform relevant tests on this :(

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

It will probably be several days before I can get around to testing it since I am slammed with meetings most of the next two days, then have a holiday weekend coming up. I will need to rebase the patches I had that add support for SFF-8636 and the QSFP support for fbnic before I can get to testing it. As such you can probably look at having this pulled in and I will try to get it tested next week.

To add a bit of complexity to all this I am looking at reworking the fbnic driver to probably add a phydev or some type of c45 swphy to handle the PMA/PMD (serdes PHY) and AN which is mostly handled by the firmware which provides the advertised link config. The main reason for doing this is that the PMD has a 4 second training window that the firmware kicks in after the link comes up, and I need to essentially do an "&=" on the link state to keep the PCS from reporting the link up while training is causing the link to flap.

author maintainer
#9 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 18 lines · Alexander Duyck
Quoted 16 lines · Russell King (Oracle)
Quoted 13 lines · Maxime Chevallier
Quoted 6 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

I don't have any hardware to perform relevant tests on this :(

Quoted 7 lines
Quoted 3 lines

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

It will probably be several days before I can get around to testing it since I am slammed with meetings most of the next two days, then have a holiday weekend coming up.

I, too, have a vacation - from tomorrow for three weeks. I may dip in and out of kernel emails during that period, but it depends what happens each day.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#10 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 24 lines · Russell King (Oracle)
Quoted 22 lines · Alexander Duyck
Quoted 19 lines · Russell King (Oracle)
Quoted 8 lines · Maxime Chevallier
Quoted 5 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

Quoted 5 lines
Quoted 1 line

I don't have any hardware to perform relevant tests on this :(

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

Quoted 7 lines
Quoted 3 lines

It will probably be several days before I can get around to testing it since I am slammed with meetings most of the next two days, then have a holiday weekend coming up.

I, too, have a vacation - from tomorrow for three weeks. I may dip in and out of kernel emails during that period, but it depends what happens each day.

So I was able to go in and test it. I ended up just running the testing in QEMU w/ my patch set that currently enables QSFP support. From what I can tell it appears to be mostly working. Before when I tried to alter the speed to go from 100G to 50G it wouldn't change. After your patch set it appears to change, although I am noticing a slight difference from the default config.

So by default we come up in the 100G w/ the QSFP configuration:
[root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: 100000baseCR2/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 100000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

I then change the speed to 50G and it links back up after a few seconds, however the "Advertised link modes" goes from "100000baseCR2/Full" to "Not reported" as shown here:
[root@localhost fbnic]# ethtool -s enp1s0 speed 50000 [root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: Not reported Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 50000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

So all-in-all it is an improvement over the previous behavior although there may still need to be some work done to improve the consistency so that it more closely matches up with what happens when you initially configure the interface.

author maintainer
#11 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 94 lines · Alexander Duyck
Quoted 24 lines · Russell King (Oracle)
Quoted 22 lines · Alexander Duyck
Quoted 19 lines · Russell King (Oracle)
Quoted 8 lines · Maxime Chevallier
Quoted 5 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

Quoted 5 lines
Quoted 1 line

I don't have any hardware to perform relevant tests on this :(

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

Quoted 7 lines
Quoted 3 lines

It will probably be several days before I can get around to testing it since I am slammed with meetings most of the next two days, then have a holiday weekend coming up.

I, too, have a vacation - from tomorrow for three weeks. I may dip in and out of kernel emails during that period, but it depends what happens each day.

So I was able to go in and test it. I ended up just running the testing in QEMU w/ my patch set that currently enables QSFP support. From what I can tell it appears to be mostly working. Before when I tried to alter the speed to go from 100G to 50G it wouldn't change. After your patch set it appears to change, although I am noticing a slight difference from the default config.

So by default we come up in the 100G w/ the QSFP configuration:
[root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: 100000baseCR2/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 100000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

I then change the speed to 50G and it links back up after a few seconds, however the "Advertised link modes" goes from "100000baseCR2/Full" to "Not reported" as shown here:
[root@localhost fbnic]# ethtool -s enp1s0 speed 50000 [root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: Not reported Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 50000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

Quoted 4 lines

So all-in-all it is an improvement over the previous behavior although there may still need to be some work done to improve the consistency so that it more closely matches up with what happens when you initially configure the interface.

Likely, it's ethtool doing that. Autoneg-off (fixed speed) modes generally don't have an advertisement. I suggest you debug at UAPI level.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#12 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 96 lines · Russell King (Oracle)
Quoted 94 lines · Alexander Duyck
Quoted 24 lines · Russell King (Oracle)
Quoted 22 lines · Alexander Duyck
Quoted 19 lines · Russell King (Oracle)
Quoted 8 lines · Maxime Chevallier
Quoted 5 lines · On Wed, 02 Jul 2025 10:44:34 +0100 "Russell King (Oracle)"…

Add phylink_sfp_select_interface_speed() which attempts to select the SFP interface based on the ethtool speed when autoneg is turned off. This allows users to turn off autoneg for SFPs that support multiple interface modes, and have an appropriate interface mode selected.

Quoted 5 lines
Quoted 1 line

I don't have any hardware to perform relevant tests on this :(

Me neither, I should've said. I'd like to see a t-b from Alexander Duyck who originally had the problem before this is merged.

Quoted 7 lines
Quoted 3 lines

It will probably be several days before I can get around to testing it since I am slammed with meetings most of the next two days, then have a holiday weekend coming up.

I, too, have a vacation - from tomorrow for three weeks. I may dip in and out of kernel emails during that period, but it depends what happens each day.

So I was able to go in and test it. I ended up just running the testing in QEMU w/ my patch set that currently enables QSFP support. From what I can tell it appears to be mostly working. Before when I tried to alter the speed to go from 100G to 50G it wouldn't change. After your patch set it appears to change, although I am noticing a slight difference from the default config.

So by default we come up in the 100G w/ the QSFP configuration:
[root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: 100000baseCR2/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 100000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

I then change the speed to 50G and it links back up after a few seconds, however the "Advertised link modes" goes from "100000baseCR2/Full" to "Not reported" as shown here:
[root@localhost fbnic]# ethtool -s enp1s0 speed 50000 [root@localhost fbnic]# ethtool enp1s0
Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: Not reported Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS
Speed: 50000Mb/s
Duplex: Full
Auto-negotiation: off
Port: Other
PHYAD: 0
Transceiver: internal
Link detected: yes

Quoted 8 lines
Quoted 4 lines

So all-in-all it is an improvement over the previous behavior although there may still need to be some work done to improve the consistency so that it more closely matches up with what happens when you initially configure the interface.

Likely, it's ethtool doing that. Autoneg-off (fixed speed) modes generally don't have an advertisement. I suggest you debug at UAPI level.

Makes sense. If I change the line to:
[root@localhost fbnic]# ethtool -s enp1s0 speed 50000 advertise 0x840000000000000

Then I am able to get it to populate the advertised mode as it did before and we can switch back and forth with the advertised mode listed matching the advertised speed. So it looks like the ksettings_set call just takes in what is there and ANDs it a few times to validate what is being selected.

maintainer
#13 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

In reply to Alexander Duyck
Quoted 7 lines

Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS

Quoted 8 lines

Advertised link modes: 100000baseCR2/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS

This all looks suspicious. If it does not support autoneg, how can it know what the link partner is advertising?

If you look at what phylib does, for plain old 1G devices:

genphy_read_status() calls genphy_read_lpa()

genphy_read_lpa() then looks to see if autoneg is enabled, and if not it does linkmode_zero(phydev->lp_advertising).

So it will not report any link partner information.

What is wrong, that it is reporting LP information, or that it is reporting it does not support autoneg when in fact it is actually doing autoneg?

Andrew

#14 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 24 lines · Andrew Lunn
Quoted 16 lines

Settings for enp1s0:
Supported ports: [ ]
Supported link modes: 50000baseCR/Full
100000baseCR2/Full
Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: No
Supported FEC modes: RS
Advertised link modes: 100000baseCR2/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: No
Advertised FEC modes: RS
Link partner advertised link modes: 100000baseCR2/Full Link partner advertised pause frame use: No Link partner advertised auto-negotiation: No Link partner advertised FEC modes: RS

This all looks suspicious. If it does not support autoneg, how can it know what the link partner is advertising?

If you look at what phylib does, for plain old 1G devices:

genphy_read_status() calls genphy_read_lpa()

Quoted 8 lines

genphy_read_lpa() then looks to see if autoneg is enabled, and if not it does linkmode_zero(phydev->lp_advertising).

So it will not report any link partner information.

What is wrong, that it is reporting LP information, or that it is reporting it does not support autoneg when in fact it is actually doing autoneg?

I have some debug code on here that is reporting the FW config as the "LP Advertised". I had borrowed that approach from the phylink fixedlink config as I thought it was a good way for me to know what the FW was requesting without having to report it out to a log file.

author maintainer
#15 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 9 lines · Alexander Duyck
Quoted 3 lines · Andrew Lunn

What is wrong, that it is reporting LP information, or that it is reporting it does not support autoneg when in fact it is actually doing autoneg?

I have some debug code on here that is reporting the FW config as the "LP Advertised". I had borrowed that approach from the phylink fixedlink config as I thought it was a good way for me to know what the FW was requesting without having to report it out to a log file.

There are a few points to be made here.

1. Fixed link configuration is not the same as !autoneg setting with the presence of a PHY. !autoneg with a PHY present means that the PHY has been instructed not to perform autonegotiation, but to set the specified parameters for the link and only allow the link to operate at the specified speed/duplex. There are exceptions - as users expect 1G to work with "autoneg" disabled, and 1G requires AN in order to bring the link up. Some PHYs support disabling the autoneg function at 1G speed by internally ignoring the request to disable autoneg, and instead only advertising to the link partner that 1G at the specified duplex is supported. We took that and turned it into a software thing for all PHYs as some PHYs decided to go a different route - basically not supporting the AN enable bit being turned off at 1G speeds.

2. Fixed link configuration is a software concept where there is no accessible PHY present. Phylink rejects fixed link configuration with a PHY. There is no support to configure a PHY into fixed speed/duplex if present, and has never been supported prior to phylink.

3. The history. Prior to phylink (and it remains in some cases today), fixed link configuration was created by providing a software emulated PHY to phylib for the MAC driver to use, thus avoiding MAC drivers having to add explicit code for fixed links. They looked just like a normal PHY, but was limited to no faster than 1G speeds as the software emulation is a Clause 22 PHY.

This software emulated PHY replaces the presence of a physical PHY (there is none) and the PHY it emulates looks like a PHY that supports AN, has AN enabled, but only supports a single speed and duplex, only advertises a single baseT(x) speed and duplex, and the link partner agrees on the speed and duplex. This "fools phylib into resolving the speed and duplex as per the fixed link configuration.

However, in reality, there is no AN.

This has become part of the user API, because the MII registers of the fixed link PHY were exported to userspace, and of course through ethtool.

There has never been a MII API for reading the fixed link parameters for speeds > 1G, so while phylink enables fixed link configuration for those speeds, there is no MII register support for this for userspace.

(As an aside)
Someone earlier today sent a reminder about a bug I'd introduced for 10GBASE-R, 5GBASE-R and another interface (I don't recall right now) and I proposed a patch that only cleared the Autoneg bit in the adertising mask. Having been reminded about it, and had Andrew's input on this thread, I'm wondering whether config.advertising should be entirely cleared as in !autoneg mode, the advertising mask makes no sense.

However, I'm supposed to be on my vacation, so I'm not going to start testing anything... this email as a bonus that would've otherwise have been delayed by about two weeks... but the way things are going (family issues) it could turn out to be a lot longer as I may have to become a full time carer. So much for an opportunity to have an opportunity to relax, which I desperately need.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#16 lore raw

Re: [PATCH net-next 3/3] net: phylink: add phylink_sfp_select_interface_speed()

Quoted 51 lines · Russell King (Oracle)
Quoted 9 lines · Alexander Duyck
Quoted 3 lines · Andrew Lunn

What is wrong, that it is reporting LP information, or that it is reporting it does not support autoneg when in fact it is actually doing autoneg?

I have some debug code on here that is reporting the FW config as the "LP Advertised". I had borrowed that approach from the phylink fixedlink config as I thought it was a good way for me to know what the FW was requesting without having to report it out to a log file.

There are a few points to be made here.

1. Fixed link configuration is not the same as !autoneg setting with the presence of a PHY. !autoneg with a PHY present means that the PHY has been instructed not to perform autonegotiation, but to set the specified parameters for the link and only allow the link to operate at the specified speed/duplex. There are exceptions - as users expect 1G to work with "autoneg" disabled, and 1G requires AN in order to bring the link up. Some PHYs support disabling the autoneg function at 1G speed by internally ignoring the request to disable autoneg, and instead only advertising to the link partner that 1G at the specified duplex is supported. We took that and turned it into a software thing for all PHYs as some PHYs decided to go a different route - basically not supporting the AN enable bit being turned off at 1G speeds.

2. Fixed link configuration is a software concept where there is no accessible PHY present. Phylink rejects fixed link configuration with a PHY. There is no support to configure a PHY into fixed speed/duplex if present, and has never been supported prior to phylink.

3. The history. Prior to phylink (and it remains in some cases today), fixed link configuration was created by providing a software emulated PHY to phylib for the MAC driver to use, thus avoiding MAC drivers having to add explicit code for fixed links. They looked just like a normal PHY, but was limited to no faster than 1G speeds as the software emulation is a Clause 22 PHY.

This software emulated PHY replaces the presence of a physical PHY (there is none) and the PHY it emulates looks like a PHY that supports AN, has AN enabled, but only supports a single speed and duplex, only advertises a single baseT(x) speed and duplex, and the link partner agrees on the speed and duplex. This "fools phylib into resolving the speed and duplex as per the fixed link configuration.

However, in reality, there is no AN.

Quoted 8 lines

This has become part of the user API, because the MII registers of the fixed link PHY were exported to userspace, and of course through ethtool.

There has never been a MII API for reading the fixed link parameters for speeds > 1G, so while phylink enables fixed link configuration for those speeds, there is no MII register support for this for userspace.

So the issue is in our setup we have a SerDes PHY so it isn't a real MII based PHY and it is being managed by the firmware so we don't have direct access.

One thought I was playing around with was emulating a 25/50/100G PMA and AN in software and exposing that as the interface to the FW to play the role of the PHY. The FW is playing the role of a device tree configuration in that the EEPROM is pre-programmed with the expected speed/lane/FEC configuration and the FW is reading that to determine what the link configuration should be as we cannot use autoneg as the switch port on the other side doesn't support it. For our production use case we will always be using that speed, but for testing we will need to support the ability to set manual speeds.

Quoted 9 lines

(As an aside)
Someone earlier today sent a reminder about a bug I'd introduced for 10GBASE-R, 5GBASE-R and another interface (I don't recall right now) and I proposed a patch that only cleared the Autoneg bit in the adertising mask. Having been reminded about it, and had Andrew's input on this thread, I'm wondering whether config.advertising should be entirely cleared as in !autoneg mode, the advertising mask makes no sense.

Quoted 6 lines

However, I'm supposed to be on my vacation, so I'm not going to start testing anything... this email as a bonus that would've otherwise have been delayed by about two weeks... but the way things are going (family issues) it could turn out to be a lot longer as I may have to become a full time carer. So much for an opportunity to have an opportunity to relax, which I desperately need.

Enjoy your vacation. It will probably take me a while to try to work out an acceptable solution for how to deal with the buried/hidden PHY behind the firmware anyway. For now this thread has become more about fbnic anyway then your original patch since we have more-or-less verified it works as expected.

#17 lore raw
In reply to Russell King (Oracle)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 4 lines · On Wed, 2 Jul 2025 10:44:38 +0100 you wrote:

This series comes from discussion during a patch series that was posted at the beginning of April, but these patches were never posted (I was too busy!)

Quoted 8 lines

We restrict ->sfp_interfaces to those that the host system supports, and ensure that ->sfp_interfaces is cleared when a SFP is removed. We then add phylink_sfp_select_interface_speed() which will select an appropriate interface from ->sfp_interfaces for the speed, and use that in our phylink_ethtool_ksettings_set() when a SFP bus is present on a directly connected host (not with a PHY.)

[...]

Here is the summary with links:
- [net-next,1/3] net: phylink: restrict SFP interfaces to those that are supported
https://git.kernel.org/netdev/net-next/c/ff1fce1bdd7b
- [net-next,2/3] net: phylink: clear SFP interfaces when not in use

    https://git.kernel.org/netdev/net-next/c/b0fdff22d520
  - [net-next,3/3] net: phylink: add phylink_sfp_select_interface_speed()
    https://git.kernel.org/netdev/net-next/c/320164a6e172

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html