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

[Edward Cree <ecree.xilinx@gmail.com>]

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@nvidia.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.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.)