[PATCH net v2] net: mii: Accept link settings read back while the link is down

[PATCH net v2] net: mii: Accept link settings read back while the link is down

[Anthony Pighin]

Since commit f6f2e946aa4d ("net: mii: Fix the Speed display when the
network cable is not connected"), mii_ethtool_get_link_ksettings()
reports SPEED_UNKNOWN whenever the link is down. ethtool changes link
settings by reading the current ones, updating only the requested
fields, and writing the result back. mii_ethtool_set_link_ksettings()
rejects the unknown speed with -EINVAL, in both autoneg and forced mode.
For example, on an ax88179_178a USB adapter while the link is down:

  # ethtool -s eth0 autoneg on advertise 0x2f
  Cannot set new settings: Invalid argument
    not setting autoneg

  # ethtool -s eth0 speed 1000 duplex full autoneg off
  # ethtool -s eth0 duplex half
  Cannot set new settings: Invalid argument
    not setting duplex

pch_gbe_set_link_ksettings() already works around the same failure by
replacing an unknown speed with SPEED_1000 before it calls
mii_ethtool_set_link_ksettings().

In forced mode, BMCR holds the configured speed whether or not the link
is up, so report SPEED_UNKNOWN only when autoneg is enabled, as
genphy_read_status() does. In autoneg mode the speed is not used, so
accept SPEED_UNKNOWN there. All other speed and duplex checks are
unchanged.

Fixes: f6f2e946aa4d ("net: mii: Fix the Speed display when the network cable is not connected")
Cc: stable@vger.kernel.org
Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
Changes in v2:
- Report SPEED_UNKNOWN only with autoneg enabled, so writing back the
  settings also works in forced mode.
- With autoneg enabled, accept only SPEED_UNKNOWN and keep the other
  speed and duplex checks. v1 accepted any speed and duplex there.
- New subject, since speed is still checked with autoneg enabled.
- Tested on an ax88179_178a adapter running v6.18.54, where
  drivers/net/mii.c is identical to net. Both ethtool examples above now
  succeed. A direct ETHTOOL_SLINKSETTINGS with autoneg enabled accepts
  SPEED_UNKNOWN and still rejects an invalid speed.

v1: https://lore.kernel.org/netdev/20261106195544.4146568-1-anthony.pighin@nokia.com/

 drivers/net/mii.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 37bc3131d31a..225488f2438b 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -199,6 +199,9 @@ void mii_ethtool_get_link_ksettings(struct mii_if_info *mii,
 			cmd->base.speed = SPEED_10;
 			cmd->base.duplex = !!(nego & ADVERTISED_10baseT_Full);
 		}
+
+		if (!(bmsr & BMSR_LSTATUS))
+			cmd->base.speed = SPEED_UNKNOWN;
 	} else {
 		cmd->base.autoneg = AUTONEG_DISABLE;
 
@@ -213,9 +216,6 @@ void mii_ethtool_get_link_ksettings(struct mii_if_info *mii,
 		lp_advertising = 0;
 	}
 
-	if (!(bmsr & BMSR_LSTATUS))
-		cmd->base.speed = SPEED_UNKNOWN;
-
 	mii->full_duplex = cmd->base.duplex;
 
 	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
@@ -333,9 +333,11 @@ int mii_ethtool_set_link_ksettings(struct mii_if_info *mii,
 	struct net_device *dev = mii->dev;
 	u32 speed = cmd->base.speed;
 
+	/* Autoneg ignores speed, which is SPEED_UNKNOWN while the link is down */
 	if (speed != SPEED_10 &&
 	    speed != SPEED_100 &&
-	    speed != SPEED_1000)
+	    speed != SPEED_1000 &&
+	    !(cmd->base.autoneg == AUTONEG_ENABLE && speed == SPEED_UNKNOWN))
 		return -EINVAL;
 	if (cmd->base.duplex != DUPLEX_HALF && cmd->base.duplex != DUPLEX_FULL)
 		return -EINVAL;

base-commit: 30c71476d842b094651bd7712f6c8ee8716147d0
-- 
2.43.0

Re: [PATCH net v2] net: mii: Accept link settings read back while the link is down

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] On ksz884x, `ethtool -s ethX autoneg on` while the link is
  down now succeeds but writes a half-duplex-only advertisement to the…

--

net: mii: Accept link settings read back while the link is down

This patch changes mii_ethtool_get_link_ksettings() so that it reports
SPEED_UNKNOWN only when autoneg is enabled and the link is down. It also
makes mii_ethtool_set_link_ksettings() accept SPEED_UNKNOWN when autoneg
is enabled. With both changes, ethtool's read-modify-write of link
settings succeeds while the link is down.

> diff --git a/drivers/net/mii.c b/drivers/net/mii.c
> index 37bc3131d31a8..225488f2438b2 100644
> --- a/drivers/net/mii.c
> +++ b/drivers/net/mii.c
> @@ -199,6 +199,9 @@ void mii_ethtool_get_link_ksettings(struct mii_if_info *mii,
>  			cmd->base.speed = SPEED_10;
>  			cmd->base.duplex = !!(nego & ADVERTISED_10baseT_Full);
>  		}
> +
> +		if (!(bmsr & BMSR_LSTATUS))
> +			cmd->base.speed = SPEED_UNKNOWN;
>  	} else {
>  		cmd->base.autoneg = AUTONEG_DISABLE;

[ ... ]

> @@ -333,9 +333,11 @@ int mii_ethtool_set_link_ksettings(struct mii_if_info *mii,
>  	struct net_device *dev = mii->dev;
>  	u32 speed = cmd->base.speed;
>  
> +	/* Autoneg ignores speed, which is SPEED_UNKNOWN while the link is down */
>  	if (speed != SPEED_10 &&
>  	    speed != SPEED_100 &&
> -	    speed != SPEED_1000)
> +	    speed != SPEED_1000 &&
> +	    !(cmd->base.autoneg == AUTONEG_ENABLE && speed == SPEED_UNKNOWN))
>  		return -EINVAL;
>  	if (cmd->base.duplex != DUPLEX_HALF && cmd->base.duplex != DUPLEX_FULL)
>  		return -EINVAL;

[Severity: Medium]
Can this make ksz884x write a half-duplex-only advertisement to the PHY
when "ethtool -s ethX autoneg on" is run while the link is down?

With autoneg on and the link down, lp_advertising is 0 in
mii_ethtool_get_link_ksettings(). That makes nego 0, so the read-back
is:

    cmd->base.speed  = SPEED_UNKNOWN
    cmd->base.duplex = !!(nego & ADVERTISED_10baseT_Full);  /* DUPLEX_HALF */

The commit message cites genphy_read_status(), but unlike that function
this code does not report the duplex as DUPLEX_UNKNOWN here.

The ksz884x caller, netdev_set_link_ksettings() in
drivers/net/ethernet/micrel/ksz884x.c, treats the read-back speed and
duplex as user intent when autoneg is set and the advertising mask
matches priv->advertising:

    if (cmd->base.autoneg && priv->advertising == advertising) {
        advertising |= ADVERTISED_ALL;
        ...
        if (0 == cmd->base.duplex)
            advertising &=
                ~(ADVERTISED_100baseT_Full |
                ADVERTISED_10baseT_Full);
        ...
    }
    ...
    } else {
        port->duplex = cmd->base.duplex + 1;
        if (1000 != speed)
            port->speed = speed;
        ...
    }

As a result:

- the full-duplex modes are dropped from the advertising mask
- port->duplex becomes 1 (half)
- SPEED_UNKNOWN is truncated to 0xff in the u8 speed field of struct
  ksz_port

Before this patch, mii_ethtool_set_link_ksettings() returned -EINVAL for
SPEED_UNKNOWN and never touched MII_ADVERTISE. Now it continues: it
writes ADVERTISE_10HALF | ADVERTISE_100HALF through the driver's
mdio_write() and restarts autoneg. Once a cable is connected, the PHY
negotiates half duplex.

The legacy ethtool path reaches this directly. "ethtool -s ethX autoneg
on" sets advertising to the current non-mode bits plus all supported
modes. When the PHY already advertises all four 10/100 modes, that
matches priv->advertising. Speed and duplex are passed through
unchanged from the read-back.

port_set_link_speed() also reads port->duplex == 1 as a manual
half-duplex request, and strips the FD bits again on later
reconfiguration:

    if (1 == port->duplex)
        data &= ~(PORT_AUTO_NEG_100BTX_FD |
            PORT_AUTO_NEG_10BT_FD);

The old code already wrote the port fields before mii_ethtool_set_link_ksettings()
returned -EINVAL. What is new is that the PHY now gets reprogrammed right
away.

Could the get and set paths be made consistent here? One option is to
report DUPLEX_UNKNOWN for autoneg with the link down, as genphy does,
and accept it under autoneg in the set path. Another is to make sure
callers like ksz884x do not treat speed and duplex from a link-down
read-back as user intent.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107210236.721901-1-anthony.pighin%40nokia.com