Re: [PATCH net] bnxt: properly flush XDP redirect lists

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 23/06/2025 18.06, Yan Zhai wrote:
> We encountered following crash when testing a XDP_REDIRECT feature
> in production:
> 
[...]
> 
>  From the crash dump, we found that the cpu_map_flush_list inside
> redirect info is partially corrupted: its list_head->next points to
> itself, but list_head->prev points to a valid list of unflushed bq
> entries.
> 
> This turned out to be a result of missed XDP flush on redirect lists. By
> digging in the actual source code, we found that
> commit 7f0a168b0441 ("bnxt_en: Add completion ring pointer in TX and RX
> ring structures") incorrectly overwrites the event mask for XDP_REDIRECT
> in bnxt_rx_xdp.

(To Andy + Michael:)
The initial bug was introduced in [1] commit a7559bc8c17c ("bnxt:
support transmit and free of aggregation buffers") in bnxt_rx_xdp()
where case XDP_TX zeros the *event, that also carries the XDP-redirect
indication.
I'm wondering if the driver should not reset the *event value?
(all other drive code paths doesn't)


> We can stably reproduce this crash by returning XDP_TX
> and XDP_REDIRECT randomly for incoming packets in a naive XDP program.
> Properly propagate the XDP_REDIRECT events back fixes the crash.
> 
> Fixes: 7f0a168b0441 ("bnxt_en: Add completion ring pointer in TX and RX ring structures")

We should also add:

Fixes: a7559bc8c17c ("bnxt: support transmit and free of aggregation 
buffers")

  [0] https://git.kernel.org/torvalds/c/7f0a168b0441 - v6.8-rc1
  [1] https://git.kernel.org/torvalds/c/a7559bc8c17c - v5.19-rc1

> Tested-by: Andrew Rzeznik <arzeznik@cloudflare.com>
> Signed-off-by: Yan Zhai <yan@cloudflare.com>

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 2cb3185c442c..ae89a981e052 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -2989,6 +2989,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
>   {
>   	struct bnxt_napi *bnapi = cpr->bnapi;
>   	u32 raw_cons = cpr->cp_raw_cons;
> +	bool flush_xdp = false;
>   	u32 cons;
>   	int rx_pkts = 0;
>   	u8 event = 0;
> @@ -3042,6 +3043,8 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
>   			else
>   				rc = bnxt_force_rx_discard(bp, cpr, &raw_cons,
>   							   &event);
> +			if (event & BNXT_REDIRECT_EVENT)
> +				flush_xdp = true;
>   			if (likely(rc >= 0))
>   				rx_pkts += rc;
>   			/* Increment rx_pkts when rc is -ENOMEM to count towards
> @@ -3066,7 +3069,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
>   		}
>   	}
>   
> -	if (event & BNXT_REDIRECT_EVENT) {
> +	if (flush_xdp) {
>   		xdp_do_flush();
>   		event &= ~BNXT_REDIRECT_EVENT;
>   	}