linux-kernel - Re: [PATCH net-next v5 09/13] net: phylink: Use phy_caps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-wQ1Ml_9xNz0XtV@shell.armlinux.org.uk>
Date: Tue, 1 Apr 2025 17:14:12 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Alexander H Duyck <alexander.duyck@...il.com>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net,
	Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
	linux-arm-kernel@...ts.infradead.org,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Herve Codina <herve.codina@...tlin.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Köry Maincent <kory.maincent@...tlin.com>,
	Oleksij Rempel <o.rempel@...gutronix.de>,
	Simon Horman <horms@...nel.org>,
	Romain Gantois <romain.gantois@...tlin.com>
Subject: Re: [PATCH net-next v5 09/13] net: phylink: Use phy_caps_lookup for
 fixed-link configuration

On Tue, Apr 01, 2025 at 08:28:29AM -0700, Alexander H Duyck wrote:
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 16a1f31f0091..380e51c5bdaa 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -713,17 +713,24 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>                 phylink_warn(pl, "fixed link specifies half duplex for %dMbps link?\n",
>                              pl->link_config.speed);
>  
> -       linkmode_zero(pl->supported);
> -       phylink_fill_fixedlink_supported(pl->supported);
> -
> +       linkmode_fill(pl->supported);
>         linkmode_copy(pl->link_config.advertising, pl->supported);
>         phylink_validate(pl, pl->supported, &pl->link_config);
>  
>         c = phy_caps_lookup(pl->link_config.speed, pl->link_config.duplex,
>                             pl->supported, true);
> -       if (c)
> +       if (c) {
>                 linkmode_and(match, pl->supported, c->linkmodes);
>  
> +               /* Compatbility with the legacy behaviour:
> +                * Report one single BaseT mode.
> +                */
> +               phylink_fill_fixedlink_supported(mask);
> +               if (linkmode_intersects(match, mask))
> +                       linkmode_and(match, match, mask);
> +               linkmode_zero(mask);
> +       }
> +
>         linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, mask);
>         linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask);
>         linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, mask);
> 
> Basically we still need the value to be screened by the pl->supported.
> The one change is that we have to run the extra screening on the
> intersect instead of skipping the screening, or doing it before we even
> start providing bits.
> 
> With this approach we will even allow people to use non twisted pair
> setups regardless of speed as long as they don't provide any twisted
> pair modes in the standard set.
> 
> I will try to get this tested today and if it works out I will submit
> it for net. I just need to test this and an SFP ksettings_set issue I
> found when we aren't using autoneg.

This code used to be so simple... and that makes me wonder whether
Maxime's work is really the best approach. It seems that the old way
was better precisely because it was more simple.

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