netdev - Re: [PATCH net-next v2 01/11] ethtool: rss: initial RSS

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: <737f0ee0-d743-497a-8247-63413060a7d8@nvidia.com>
Date: Tue, 15 Jul 2025 10:20:38 +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 v2 01/11] ethtool: rss: initial RSS_SET
 (indirection table handling)

On 15/07/2025 1:27, Jakub Kicinski wrote:
> +static int
> +rss_set_prep_indir(struct net_device *dev, struct genl_info *info,
> +		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh,
> +		   bool *reset, bool *mod)
> +{
> +	const struct ethtool_ops *ops = dev->ethtool_ops;
> +	struct netlink_ext_ack *extack = info->extack;
> +	struct nlattr **tb = info->attrs;
> +	struct ethtool_rxnfc rx_rings;
> +	size_t alloc_size;
> +	u32 user_size;
> +	int i, err;
> +
> +	if (!tb[ETHTOOL_A_RSS_INDIR])
> +		return 0;
> +	if (!data->indir_size)
> +		return -EOPNOTSUPP;
> +
> +	rx_rings.cmd = ETHTOOL_GRXRINGS;
> +	err = ops->get_rxnfc(dev, &rx_rings, NULL);

Do we need to check for NULL op?

> +	if (err)
> +		return err;
> +
> +	if (nla_len(tb[ETHTOOL_A_RSS_INDIR]) % 4) {
> +		NL_SET_BAD_ATTR(info->extack, tb[ETHTOOL_A_RSS_INDIR]);
> +		return -EINVAL;
> +	}
> +	user_size = nla_len(tb[ETHTOOL_A_RSS_INDIR]) / 4;
> +	if (!user_size) {
> +		if (rxfh->rss_context) {
> +			NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					    "can't reset table for a context");
> +			return -EINVAL;
> +		}
> +		*reset = true;
> +	} else if (data->indir_size % user_size) {
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					"size (%d) mismatch with device indir table (%d)",
> +					user_size, data->indir_size);
> +		return -EINVAL;
> +	}
> +
> +	rxfh->indir_size = data->indir_size;
> +	alloc_size = array_size(data->indir_size, sizeof(rxfh->indir[0]));
> +	rxfh->indir = kzalloc(alloc_size, GFP_KERNEL);
> +	if (!rxfh->indir)
> +		return -ENOMEM;
> +
> +	nla_memcpy(rxfh->indir, tb[ETHTOOL_A_RSS_INDIR], alloc_size);
> +	for (i = 0; i < user_size; i++) {
> +		if (rxfh->indir[i] < rx_rings.data)
> +			continue;
> +
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					"entry %d: queue out of range (%d)",
> +					i, rxfh->indir[i]);
> +		err = -EINVAL;
> +		goto err_free;
> +	}
> +
> +	if (user_size) {
> +		/* Replicate the user-provided table to fill the device table */
> +		for (i = user_size; i < data->indir_size; i++)
> +			rxfh->indir[i] = rxfh->indir[i % user_size];
> +	} else {
> +		for (i = 0; i < data->indir_size; i++)
> +			rxfh->indir[i] =
> +				ethtool_rxfh_indir_default(i, rx_rings.data);
> +	}
> +
> +	*mod |= memcmp(rxfh->indir, data->indir_table, data->indir_size);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(rxfh->indir);
> +	rxfh->indir = NULL;
> +	return err;
> +}
> +
> +static int
> +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;
> +	struct ethtool_rxfh_param rxfh = {};
> +	bool indir_reset = false, indir_mod;
> +	struct nlattr **tb = info->attrs;
> +	struct rss_reply_data data = {};
> +	const struct ethtool_ops *ops;
> +	bool mod = false;
> +	int ret;
> +
> +	ops = dev->ethtool_ops;
> +	data.base.dev = dev;
> +
> +	ret = rss_prepare(request, dev, &data, info);
> +	if (ret)
> +		return ret;
> +
> +	rxfh.rss_context = request->rss_context;
> +
> +	ret = rss_set_prep_indir(dev, info, &data, &rxfh, &indir_reset, &mod);
> +	if (ret)
> +		goto exit_clean_data;
> +	indir_mod = !!tb[ETHTOOL_A_RSS_INDIR];
> +
> +	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
> +	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
> +
> +	mutex_lock(&dev->ethtool->rss_lock);
> +	if (request->rss_context) {
> +		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
> +		if (!ctx) {
> +			ret = -ENOENT;
> +			goto exit_unlock;
> +		}
> +	}
> +
> +	if (!mod)
> +		ret = 0; /* nothing to tell the driver */
> +	else if (!ops->set_rxfh)

Why not do it in validate?