netdev - Re: [PATCH net-next v2 4/5] net: ethtool: remove the compat code for _rxfh

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: <11d9b133-0e98-4fb9-9290-40d8cbb08fa6@gmail.com>
Date: Wed, 2 Jul 2025 19:54:36 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 andrew+netdev@...n.ch, horms@...nel.org, andrew@...n.ch,
 przemyslaw.kitszel@...el.com, anthony.l.nguyen@...el.com,
 sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com,
 bbhushan2@...vell.com, tariqt@...dia.com, mbloch@...dia.com,
 leon@...nel.org, gal@...dia.com
Subject: Re: [PATCH net-next v2 4/5] net: ethtool: remove the compat code for
 _rxfh_context ops

On 02/07/2025 04:06, Jakub Kicinski wrote:
> All drivers are now converted to dedicated _rxfh_context ops.
> Remove the use of >set_rxfh() to manage additional contexts.
> 
> Reviewed-by: Gal Pressman <gal@...dia.com>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
...
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7ee808eb068e..ee32d87eb411 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
...
> @@ -1670,7 +1668,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>  	rxfh_dev.rss_context = rxfh.rss_context;
>  	rxfh_dev.input_xfrm = rxfh.input_xfrm;
>  
> -	if (rxfh.rss_context && ops->create_rxfh_context) {
> +	if (rxfh.rss_context) {
>  		if (create) {
>  			ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev,
>  						       extack);

A few lines after this we have
	if (ret) {
		if (create) {
			/* failed to create, free our new tracking entry */
			if (ops->create_rxfh_context)
				xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context);
			kfree(ctx);
		}
		goto out;
	}
AFAICT that 'if (ops->create_rxfh_context)' guard can also go, because it's
 only false in the compat case.

> @@ -1712,37 +1710,6 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>  		else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
>  			dev->priv_flags |= IFF_RXFH_CONFIGURED;
>  	}
> -	/* Update rss_ctx tracking */

This comment applies to not just the code being removed, but also the part
 below that (if(delete), else if (ctx)) that's retained, please keep it.

Other than that this looks good.  (Patches 1-3 look plausible but I don't
 feel competent to review those drivers to the level of giving tags.)