netdev - Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

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: <2a4c0db9-d330-441f-bce1-937401657bfe@nvidia.com>
Date: Sun, 13 Jul 2025 14:12:33 +0300
From: Gal Pressman <gal@...dia.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, donald.hunter@...il.com,
 shuah@...nel.org, kory.maincent@...tlin.com, maxime.chevallier@...tlin.com,
 sdf@...ichev.me, ecree.xilinx@...il.com
Subject: Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing
 fields

On 11/07/2025 4:53, Jakub Kicinski wrote:
> Add support for ETHTOOL_SRXFH (setting hashing fields) in RSS_SET.
> 
> The tricky part is dealing with symmetric hashing, user can change
> the hashing fields and symmetric hash in one request. Since fields
> and hash function config are separate driver callback changes to
> the two are not atomic. Keep things simple and validate the settings
> against both pre- and post- change ones. Meaning that we will reject
> the config request if user tries to correct the flow fields and set
> input_xfrm in one request, or disables input_xfrm and makes flow
> fields non-symmetric.

How is it different than what we have in ioctl?

> 
> We can adjust it later if there's a real need. Starting simple feels
> right, and potentially partially applying the settings isn't nice,
> either.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  static void
>  rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
>  		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
> @@ -673,11 +767,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  	struct rss_req_info *request = RSS_REQINFO(req_info);
>  	struct ethtool_rxfh_context *ctx = NULL;
>  	struct net_device *dev = req_info->dev;
> +	bool mod = false, fields_mod = false;

Why not use mod?

>  	struct ethtool_rxfh_param rxfh = {};
>  	struct nlattr **tb = info->attrs;
>  	struct rss_reply_data data = {};
>  	const struct ethtool_ops *ops;
> -	bool mod = false;
>  	int ret;
>  
>  	ops = dev->ethtool_ops;
> @@ -708,14 +802,10 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  	ethnl_update_u8(&rxfh.input_xfrm, tb[ETHTOOL_A_RSS_INPUT_XFRM], &mod);
>  	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */
>  	if (!request->rss_context || ops->rxfh_per_ctx_key)
> -		xfrm_sym = !!rxfh.input_xfrm;
> +		xfrm_sym = rxfh.input_xfrm || data.input_xfrm;
>  	if (rxfh.input_xfrm == data.input_xfrm)
>  		rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
>  
> -	ret = rss_check_rxfh_fields_sym(dev, info, &data, xfrm_sym);
> -	if (ret)
> -		goto exit_clean_data;
> -
>  	mutex_lock(&dev->ethtool->rss_lock);
>  	if (request->rss_context) {
>  		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
> @@ -725,6 +815,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  		}
>  	}
>  
> +	ret = ethnl_set_rss_fields(dev, info, request->rss_context,
> +				   &data, xfrm_sym, &fields_mod);
> +	if (ret)
> +		goto exit_unlock;
> +
>  	if (!mod)
>  		ret = 0; /* nothing to tell the driver */
>  	else if (!rxfh.rss_context)
> @@ -748,7 +843,7 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  exit_clean_data:
>  	rss_cleanup_data(&data.base);
>  
> -	return ret ?: mod;
> +	return ret ?: mod || fields_mod;
>  }