[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0913d63c-1df5-407a-a7c0-d5bef0210e8e@amd.com>
Date: Thu, 3 Oct 2024 11:13:02 -0700
From: Brett Creeley <bcreeley@....com>
To: Taehee Yoo <ap420073@...il.com>, davem@...emloft.net, kuba@...nel.org,
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
Cc: 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
Subject: Re: [PATCH net-next v3 4/7] bnxt_en: add support for
tcp-data-split-thresh ethtool command
On 10/3/2024 9:06 AM, Taehee Yoo wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> The bnxt_en driver has configured the hds_threshold value automatically
> when TPA is enabled based on the rx-copybreak default value.
> Now the tcp-data-split-thresh ethtool command is added, so it adds an
> implementation of tcp-data-split-thresh option.
>
> Configuration of the tcp-data-split-thresh is allowed only when
> the tcp-data-split is enabled. The default value of
> tcp-data-split-thresh is 256, which is the default value of rx-copybreak,
> which used to be the hds_thresh value.
>
> # Example:
> # ethtool -G enp14s0f0np0 tcp-data-split on tcp-data-split-thresh 256
> # ethtool -g enp14s0f0np0
> Ring parameters for enp14s0f0np0:
> Pre-set maximums:
> ...
> TCP data split thresh: 256
> Current hardware settings:
> ...
> TCP data split: on
> TCP data split thresh: 256
>
> It enables tcp-data-split and sets tcp-data-split-thresh value to 256.
>
> # ethtool -G enp14s0f0np0 tcp-data-split off
> # ethtool -g enp14s0f0np0
> Ring parameters for enp14s0f0np0:
> Pre-set maximums:
> ...
> TCP data split thresh: 256
> Current hardware settings:
> ...
> TCP data split: off
> TCP data split thresh: n/a
>
> Signed-off-by: Taehee Yoo <ap420073@...il.com>
> ---
>
> v3:
> - Drop validation logic tcp-data-split and tcp-data-split-thresh.
>
> v2:
> - Patch added.
>
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 ++-
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 ++
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++++
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index f046478dfd2a..872b15842b11 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -4455,6 +4455,7 @@ static void bnxt_init_ring_params(struct bnxt *bp)
> {
> bp->rx_copybreak = BNXT_DEFAULT_RX_COPYBREAK;
> bp->flags |= BNXT_FLAG_HDS;
> + bp->hds_threshold = BNXT_DEFAULT_RX_COPYBREAK;
> }
>
> /* bp->rx_ring_size, bp->tx_ring_size, dev->mtu, BNXT_FLAG_{G|L}RO flags must
> @@ -6429,7 +6430,7 @@ static int bnxt_hwrm_vnic_set_hds(struct bnxt *bp, struct bnxt_vnic_info *vnic)
> VNIC_PLCMODES_CFG_REQ_FLAGS_HDS_IPV6);
> req->enables |=
> cpu_to_le32(VNIC_PLCMODES_CFG_REQ_ENABLES_HDS_THRESHOLD_VALID);
> - req->hds_threshold = cpu_to_le16(bp->rx_copybreak);
> + req->hds_threshold = cpu_to_le16(bp->hds_threshold);
> }
> req->vnic_id = cpu_to_le32(vnic->fw_vnic_id);
> return hwrm_req_send(bp, req);
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index 35601c71dfe9..48f390519c35 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2311,6 +2311,8 @@ struct bnxt {
> int rx_agg_nr_pages;
> int rx_nr_rings;
> int rsscos_nr_ctxs;
> +#define BNXT_HDS_THRESHOLD_MAX 256
> + u16 hds_threshold;
Putting this here creates a 2 byte hole right after hds_threshold and
also puts a 4 byte hole after cp_nr_rings.
Since hds_threshold doesn't seem to be used in the hotpath maybe it
would be best to fill a pre-existing hole in struct bnxt to put it?
Thanks,
Brett
>
> u32 tx_ring_size;
> u32 tx_ring_mask;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index e9ef65dd2e7b..af6ed492f688 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -839,6 +839,9 @@ static void bnxt_get_ringparam(struct net_device *dev,
> else
> kernel_ering->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_DISABLED;
>
> + kernel_ering->tcp_data_split_thresh = bp->hds_threshold;
> + kernel_ering->tcp_data_split_thresh_max = BNXT_HDS_THRESHOLD_MAX;
> +
> ering->tx_max_pending = BNXT_MAX_TX_DESC_CNT;
>
> ering->rx_pending = bp->rx_ring_size;
> @@ -871,6 +874,7 @@ static int bnxt_set_ringparam(struct net_device *dev,
> case ETHTOOL_TCP_DATA_SPLIT_UNKNOWN:
> case ETHTOOL_TCP_DATA_SPLIT_ENABLED:
> bp->flags |= BNXT_FLAG_HDS;
> + bp->hds_threshold = (u16)kernel_ering->tcp_data_split_thresh;
> break;
> case ETHTOOL_TCP_DATA_SPLIT_DISABLED:
> bp->flags &= ~BNXT_FLAG_HDS;
> --
> 2.34.1
>
Powered by blists - more mailing lists