netdev - Re: [PATCH net-next] net: atlantic: convert EEE handling to use linkmode bitmaps

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 for Android: free password hash cracker in your pocket

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

Message-ID: <c7979b55-142b-469b-8da3-2662f0fe826e@lunn.ch>
Date: Sat, 3 Feb 2024 22:50:03 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	David Miller <davem@...emloft.net>,
	Russell King - ARM Linux <linux@...linux.org.uk>,
	Igor Russkikh <irusskikh@...vell.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: atlantic: convert EEE handling to use
 linkmode bitmaps

On Sat, Feb 03, 2024 at 10:25:44PM +0100, Heiner Kallweit wrote:
> Convert EEE handling to use linkmode bitmaps. This prepares for
> removing the legacy bitmaps from struct ethtool_keee.
> No functional change intended.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  .../ethernet/aquantia/atlantic/aq_ethtool.c   | 21 +++++++++----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> index 0bd1a0a1a..7cc36517b 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
> @@ -15,6 +15,7 @@
>  #include "aq_macsec.h"
>  #include "aq_main.h"
>  
> +#include <linux/linkmode.h>
>  #include <linux/ptp_clock_kernel.h>
>  
>  static void aq_ethtool_get_regs(struct net_device *ndev,
> @@ -681,20 +682,18 @@ static int aq_ethtool_get_ts_info(struct net_device *ndev,
>  	return 0;
>  }
>  
> -static u32 eee_mask_to_ethtool_mask(u32 speed)
> +static void eee_mask_to_ethtool_mask(unsigned long *mode, u32 speed)
>  {
> -	u32 rate = 0;
> +	linkmode_zero(mode);

Some comment as to bnx2x.

>  static int aq_ethtool_get_eee(struct net_device *ndev, struct ethtool_keee *eee)
> @@ -713,14 +712,14 @@ static int aq_ethtool_get_eee(struct net_device *ndev, struct ethtool_keee *eee)
>  	if (err < 0)
>  		return err;
>  
> -	eee->supported_u32 = eee_mask_to_ethtool_mask(supported_rates);
> +	eee_mask_to_ethtool_mask(eee->supported, supported_rates);
>  
>  	if (aq_nic->aq_nic_cfg.eee_speeds)
> -		eee->advertised_u32 = eee->supported_u32;
> +		linkmode_copy(eee->advertised, eee->supported);

This is again a correct translation. But the underlying implementation
seems wrong. aq_ethtool_set_eee() does not appear to allow the
advertisement to be changed, so advertised does equal
supported. However aq_ethtool_set_eee() does not validate that the
user is not changing what is advertised and returning an error. Lets
leave it broken, and see if Aquantia/Marvell care.

Reviewed-by: Andrew Lunn <andrew@...n.ch>

    Andrew