netdev - Re: [PATCH RFC net-next 07/13] net: phylink: simplify ksettings

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: <VI1PR0402MB3871B35587375E9EFE155611E07B0@VI1PR0402MB3871.eurprd04.prod.outlook.com>
Date:   Mon, 20 Jul 2020 10:24:49 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
CC:     Vladimir Oltean <vladimir.oltean@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        "michael@...le.cc" <michael@...le.cc>,
        "olteanv@...il.com" <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC net-next 07/13] net: phylink: simplify ksettings_set()
 implementation

On 6/30/20 5:29 PM, Russell King wrote:
> Simplify the ksettings_set() implementation to look more like phylib's
> implementation; use a switch() for validating the autoneg setting, and
> use the linkmode_modify() helper to set the autoneg bit in the
> advertisement mask.
> 
> Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>

Reviewed-by: Ioana Ciornei <ioana.ciornei@....com>

> ---
>   drivers/net/phy/phylink.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 424a927d7889..103d2a550415 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1314,25 +1314,24 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
>   	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
>   	struct ethtool_link_ksettings our_kset;
>   	struct phylink_link_state config;
> +	const struct phy_setting *s;
>   	int ret;
>   
>   	ASSERT_RTNL();
>   
> -	if (kset->base.autoneg != AUTONEG_DISABLE &&
> -	    kset->base.autoneg != AUTONEG_ENABLE)
> -		return -EINVAL;
> -
>   	linkmode_copy(support, pl->supported);
>   	config = pl->link_config;
> +	config.an_enabled = kset->base.autoneg == AUTONEG_ENABLE;
>   
> -	/* Mask out unsupported advertisements */
> +	/* Mask out unsupported advertisements, and force the autoneg bit */
>   	linkmode_and(config.advertising, kset->link_modes.advertising,
>   		     support);
> +	linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising,
> +			 config.an_enabled);
>   
>   	/* FIXME: should we reject autoneg if phy/mac does not support it? */
> -	if (kset->base.autoneg == AUTONEG_DISABLE) {
> -		const struct phy_setting *s;
> -
> +	switch (kset->base.autoneg) {
> +	case AUTONEG_DISABLE:
>   		/* Autonegotiation disabled, select a suitable speed and
>   		 * duplex.
>   		 */
> @@ -1351,19 +1350,19 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
>   
>   		config.speed = s->speed;
>   		config.duplex = s->duplex;
> -		config.an_enabled = false;
> +		break;
>   
> -		__clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising);
> -	} else {
> +	case AUTONEG_ENABLE:
>   		/* If we have a fixed link, refuse to enable autonegotiation */
>   		if (pl->cur_link_an_mode == MLO_AN_FIXED)
>   			return -EINVAL;
>   
>   		config.speed = SPEED_UNKNOWN;
>   		config.duplex = DUPLEX_UNKNOWN;
> -		config.an_enabled = true;
> +		break;
>   
> -		__set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising);
> +	default:
> +		return -EINVAL;
>   	}
>   
>   	if (pl->phydev) {
>