netdev - Re: [PATCH net-next v6 4/9] net: ethtool: add support for configuring hds-thresh

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: <20241218183547.45273b87@kernel.org>
Date: Wed, 18 Dec 2024 18:35:47 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Taehee Yoo <ap420073@...il.com>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
 almasrymina@...gle.com, donald.hunter@...il.com, corbet@....net,
 michael.chan@...adcom.com, andrew+netdev@...n.ch, hawk@...nel.org,
 ilias.apalodimas@...aro.org, ast@...nel.org, daniel@...earbox.net,
 john.fastabend@...il.com, dw@...idwei.uk, sdf@...ichev.me,
 asml.silence@...il.com, brett.creeley@....com, linux-doc@...r.kernel.org,
 netdev@...r.kernel.org, kory.maincent@...tlin.com,
 maxime.chevallier@...tlin.com, danieller@...dia.com,
 hengqi@...ux.alibaba.com, ecree.xilinx@...il.com,
 przemyslaw.kitszel@...el.com, hkallweit1@...il.com, ahmed.zaki@...el.com,
 rrameshbabu@...dia.com, idosch@...dia.com, jiri@...nulli.us,
 bigeasy@...utronix.de, lorenzo@...nel.org, jdamato@...tly.com,
 aleksander.lobakin@...el.com, kaiyuanz@...gle.com, willemb@...gle.com,
 daniel.zahka@...il.com
Subject: Re: [PATCH net-next v6 4/9] net: ethtool: add support for
 configuring hds-thresh

On Wed, 18 Dec 2024 14:45:25 +0000 Taehee Yoo wrote:
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 4e451084d58a..4f407ce9eed1 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -78,6 +78,8 @@ enum {
>   * @cqe_size: Size of TX/RX completion queue event
>   * @tx_push_buf_len: Size of TX push buffer
>   * @tx_push_buf_max_len: Maximum allowed size of TX push buffer
> + * @hds_thresh: Threshold value of header-data-split-thresh
> + * @hds_thresh_max: Maximum allowed threshold of header-data-split-thresh

nit: s/allowed/supported/

> +u8 dev_xdp_sb_prog_count(struct net_device *dev)
> +{
> +	u8 count = 0;
> +	int i;
> +
> +	for (i = 0; i < __MAX_XDP_MODE; i++)
> +		if (dev->xdp_state[i].prog &&
> +		    !dev->xdp_state[i].prog->aux->xdp_has_frags)
> +			count++;
> +	return count;
> +}
> +EXPORT_SYMBOL_GPL(dev_xdp_sb_prog_count);

No need to export this, AFAICT, none of the callers can be built 
as a module.

> +	hds_config_mod = old_hds_config != kernel_ringparam.tcp_data_split;

Does it really matter if we modified the HDS setting for the XDP check?
Whether it was already set or the current config is asking for it to be
set having XDP SB and HDS is invalid, we can return an error.

> +	if (kernel_ringparam.tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_ENABLED &&
> +	    hds_config_mod && dev_xdp_sb_prog_count(dev)) {
> +		NL_SET_ERR_MSG(info->extack,

		NL_SET_ERR_MSG_ATTR(info->extack,
				    tb[ETHTOOL_A_RINGS_TCP_DATA_SPLIT],
				    ...

> +			       "tcp-data-split can not be enabled with single buffer XDP");
> +		return -EINVAL;
> +	}
> +
> +	if (kernel_ringparam.hds_thresh > kernel_ringparam.hds_thresh_max) {
> +		NL_SET_BAD_ATTR(info->extack,
> +				tb[ETHTOOL_A_RINGS_HDS_THRESH_MAX]);
> +		return -ERANGE;
> +	}

Can this condition not be handled by the big if "ladder" below?
I mean like this:

@@ -282,6 +276,8 @@ ethnl_set_rings(struct ethnl_req_info *req_info, struct genl_info *info)
                err_attr = tb[ETHTOOL_A_RINGS_RX_JUMBO];
        else if (ringparam.tx_pending > ringparam.tx_max_pending)
                err_attr = tb[ETHTOOL_A_RINGS_TX];
+       else if (kernel_ringparam.hds_thresh > kernel_ringparam.hds_thresh_max)
+               err_attr = tb[ETHTOOL_A_RINGS_HDS_THRESH_MAX];
        else
                err_attr = NULL;
        if (err_attr) {

>  	/* ensure new ring parameters are within limits */
>  	if (ringparam.rx_pending > ringparam.rx_max_pending)
>  		err_attr = tb[ETHTOOL_A_RINGS_RX];