netdev - Re: [PATCH net-next v3 2/7] bnxt_en: add support for tcp-data-split ethtool command

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: <20241008111926.7056cc93@kernel.org>
Date: Tue, 8 Oct 2024 11:19:26 -0700
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, netdev@...r.kernel.org, linux-doc@...r.kernel.org,
 donald.hunter@...il.com, corbet@....net, michael.chan@...adcom.com,
 kory.maincent@...tlin.com, andrew@...n.ch, 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,
 paul.greenwalt@...el.com, rrameshbabu@...dia.com, idosch@...dia.com,
 asml.silence@...il.com, kaiyuanz@...gle.com, willemb@...gle.com,
 aleksander.lobakin@...el.com, dw@...idwei.uk, sridhar.samudrala@...el.com,
 bcreeley@....com
Subject: Re: [PATCH net-next v3 2/7] bnxt_en: add support for tcp-data-split
 ethtool command

On Thu,  3 Oct 2024 16:06:15 +0000 Taehee Yoo wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index fdecdf8894b3..e9ef65dd2e7b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -829,12 +829,16 @@ static void bnxt_get_ringparam(struct net_device *dev,
>  	if (bp->flags & BNXT_FLAG_AGG_RINGS) {
>  		ering->rx_max_pending = BNXT_MAX_RX_DESC_CNT_JUM_ENA;
>  		ering->rx_jumbo_max_pending = BNXT_MAX_RX_JUM_DESC_CNT;
> -		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
>  	} else {
>  		ering->rx_max_pending = BNXT_MAX_RX_DESC_CNT;
>  		ering->rx_jumbo_max_pending = 0;
> -		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_DISABLED;
>  	}
> +
> +	if (bp->flags & BNXT_FLAG_HDS)
> +		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
> +	else
> +		kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_DISABLED;

This breaks previous behavior. The HDS reporting from get was
introduced to signal to user space whether the page flip based
TCP zero-copy (the one added some years ago not the recent one)
will be usable with this NIC.

When HW-GRO is enabled HDS will be working.

I think that the driver should only track if the user has set the value
to ENABLED (forced HDS), or to UKNOWN (driver default). Setting the HDS
to disabled is not useful, don't support it.

>  	ering->tx_max_pending = BNXT_MAX_TX_DESC_CNT;
>  
>  	ering->rx_pending = bp->rx_ring_size;
> @@ -854,9 +858,25 @@ static int bnxt_set_ringparam(struct net_device *dev,
>  	    (ering->tx_pending < BNXT_MIN_TX_DESC_CNT))
>  		return -EINVAL;
>  
> +	if (kernel_ering->tcp_data_split != ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
> +	    BNXT_RX_PAGE_MODE(bp)) {
> +		NL_SET_ERR_MSG_MOD(extack, "tcp-data-split can not be enabled with XDP");
> +		return -EINVAL;
> +	}

Technically just if the XDP does not support multi-buffer.
Any chance we could do this check in the core?