netdev - Re: [PATCH net-next V2 1/9] eth: fbnic: Add support for HDS configuration

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: <5a3ffacd-bae8-4ca6-85a4-4369b687e718@linux.dev>
Date: Tue, 12 Aug 2025 11:47:26 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Mohsin Bashir <mohsin.bashr@...il.com>, netdev@...r.kernel.org
Cc: kuba@...nel.org, alexanderduyck@...com, andrew+netdev@...n.ch,
 ast@...nel.org, bpf@...r.kernel.org, corbet@....net, daniel@...earbox.net,
 davem@...emloft.net, edumazet@...gle.com, hawk@...nel.org, horms@...nel.org,
 jdamato@...tly.com, john.fastabend@...il.com, kernel-team@...a.com,
 pabeni@...hat.com, sdf@...ichev.me, aleksander.lobakin@...el.com
Subject: Re: [PATCH net-next V2 1/9] eth: fbnic: Add support for HDS
 configuration

On 11/08/2025 22:13, Mohsin Bashir wrote:
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
> index f9543d03485f..c80cbde50925 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
> @@ -2232,13 +2232,22 @@ static void fbnic_enable_rcq(struct fbnic_napi_vector *nv,
>   {
>   	struct fbnic_net *fbn = netdev_priv(nv->napi.dev);
>   	u32 log_size = fls(rcq->size_mask);
> -	u32 rcq_ctl;
> +	u32 hds_thresh = fbn->hds_thresh;
> +	u32 rcq_ctl = 0;

I don't think this initialization is needed ...

>   
>   	fbnic_config_drop_mode_rcq(nv, rcq);
>   
> +	/* Force lower bound on MAX_HEADER_BYTES. Below this, all frames should
> +	 * be split at L4. It would also result in the frames being split at
> +	 * L2/L3 depending on the frame size.
> +	 */
> +	if (fbn->hds_thresh < FBNIC_HDR_BYTES_MIN) {
> +		rcq_ctl = FBNIC_QUEUE_RDE_CTL0_EN_HDR_SPLIT;
> +		hds_thresh = FBNIC_HDR_BYTES_MIN;
> +	}
> +
>   	rcq_ctl = FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_PADLEN_MASK, FBNIC_RX_PAD) |

because you still unconditionally rewrite the value here. at the same
time FBNIC_QUEUE_RDE_CTL0_EN_HDR_SPLIT value will be lost, so I believe
it should be

  rcq_ctl |= FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_PADLEN_MASK, FBNIC_RX_PAD) |

and then the init code above makes sense.

> -		   FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_MAX_HDR_MASK,
> -			      FBNIC_RX_MAX_HDR) |
> +		   FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_MAX_HDR_MASK, hds_thresh) |
>   		   FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_PAYLD_OFF_MASK,
>   			      FBNIC_RX_PAYLD_OFFSET) |
>   		   FIELD_PREP(FBNIC_QUEUE_RDE_CTL1_PAYLD_PG_CL_MASK,