> Sashiko report shows that drivers are rather inconsistent here Yes I missed it, I am working on fixes.
> On mlx5 with striding RQ this also ended in a NULL pointer > dereference in mlx5e_post_rx_wqes(). So in my case I received kernel panic when I turned of striding rq off and the rx ring was full, but the panic itself was mitigated by the fixes from @Jerome Tollet in the mlx driver. I described the problem with linked fixes here: https://forums.developer.nvidia.com/t/ofed-backport-of-mainline-mlx5e-xsk-stale-buffer-release-fix-kernel-panic-in-mlx5e-post-rx-wqes/384752/5 But the real root cause is the problem I'm trying to fix. > Did you verify this or is this rather a speculation? xp_free() protects > against double-free cases. I verified this, the origin bug on our own was that we received the same UMEM frame several twice even we haven't push it to the ring, that was causing serious damages in our system, we need to add special detection duplicated frames after receiving frames on rx ring, when I removed this additional xdp free, the problem was gone. > Have you thought about massaging mlx driver instead of intel + core? > Even when that would be correct approach ixgbe would miss being addressed. I was considering and I did as workaround, but IMO the final fix should be placed in the AF XDP, because there is inconsistency between different kind of errors, look at xsk_rcv_check() there isn't any freeing buffer, so why only ENOBUFS causes that? Maybe there is some reason that I don't know. Moreover, if I fix it only in mlx, the problem in the other driver at certain path will persist including Intel, only ZC with needs_wakup flag path is properly handled there. Yes ixgbe is my gap, I missed it and I can apply the patch there as well. But eventually this is your call where to fix it, of course. > A note to ourselves, we should write a section in the documentation > for AF_XDP that defines the expected driver behavior. There are very > likely more inconsistencies out there since this was never explicitly > specified anywhere This is good point, some documented unification of behavor across happy path and different kind of errors would be nice. ________________________________________ From: Magnus Karlsson <[email protected]> Sent: Tuesday, 20 October 2026 08:09 To: Maciej Fijalkowski Cc: Marcin Knap; Tony Nguyen; Przemek Kitszel; Andrew Lunn; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Magnus Karlsson; Stanislav Fomichev; Simon Horman; Alexei Starovoitov; Daniel Borkmann; Jesper Dangaard Brouer; John Fastabend; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] Subject: Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] On Tue, 20 Oct 2026 at 00:14, Maciej Fijalkowski <[email protected]> wrote: > > On Mon, Oct 19, 2026 at 10:22:02AM +0200, Marcin Knap via B4 Relay wrote: > > From: Marcin Knap <[email protected]> > > > > When an XDP program redirects a zero-copy buffer to an XSK and the > > socket's Rx ring is full, xsk_rcv_zc() frees the buffer and returns > > -ENOBUFS to xdp_do_redirect(). > > > > A failed xdp_do_redirect() leaves the buffer owned by the caller, and > > drivers rely on that: mlx5e treats the failure as XDP_DROP and releases > > Sashiko report shows that drivers are rather inconsistent here A note to ourselves, we should write a section in the documentation for AF_XDP that defines the expected driver behavior. There are very likely more inconsistencies out there since this was never explicitly specified anywhere. > > the buffer when the Rx WQE is refilled, and i40e and ice free it on > > XDP_CONSUMED. The buffer is therefore freed twice. It gets reused for > > another Rx descriptor, the second free puts an in-use buffer back on > > the pool free list, and the same UMEM frame is later handed to user > > space twice. > > Did you verify this or is this rather a speculation? xp_free() protects > against double-free cases. > > > On mlx5 with striding RQ this also ended in a NULL pointer > > dereference in mlx5e_post_rx_wqes(). > > Please more info on this then. > > > > > Leave the buffer with the caller on both the single-buffer and the > > multi-buffer -ENOBUFS paths, as xsk_rcv() and xsk_rcv_check() already > > do. i40e and ice relied on the core freeing the buffer on their > > need_wakeup -ENOBUFS (XDP_EXIT) path, so make them free it there. > > > > Seen on Mellanox ConnectX-6 DX MT2892 (FW 22.47.1026) with AF_XDP > > zero-copy on 7.3.0-rc6 when the user-space consumer could not keep up > > with the Rx ring. With this change no duplicate frames were seen with > > striding RQ on or off. > > Have you thought about massaging mlx driver instead of intel + core? > Even when that would be correct approach ixgbe would miss being addressed. > > > > > Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full") > > Signed-off-by: Marcin Knap <[email protected]> > > --- > > Tested on mlx5 (ConnectX-6 DX) in zero-copy single-buffer mode. > > The i40e and ice changes and the multi-buffer path are build-tested > > only, as I don't have that hardware available. > > --- > > drivers/net/ethernet/intel/i40e/i40e_xsk.c | 1 + > > drivers/net/ethernet/intel/ice/ice_xsk.c | 1 + > > net/xdp/xsk.c | 15 +++------------ > > 3 files changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > index 9f47388eaba5..d32c7255f0b3 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > > @@ -358,6 +358,7 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring > > *rx_ring, > > return; > > > > if (xdp_res == I40E_XDP_EXIT) { > > + xsk_buff_free(xdp_buff); > > *failure = true; > > return; > > } > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c > > b/drivers/net/ethernet/intel/ice/ice_xsk.c > > index 0643017541c3..7c1dd4c87a71 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > > @@ -636,6 +636,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, > > if (likely(xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR))) { > > xdp_xmit |= xdp_res; > > } else if (xdp_res == ICE_XDP_EXIT) { > > + xsk_buff_free(first); > > failure = true; > > first = NULL; > > break; > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c > > index 33475b180ea6..0edc160c660f 100644 > > --- a/net/xdp/xsk.c > > +++ b/net/xdp/xsk.c > > @@ -224,21 +224,15 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct > > xdp_buff *xdp, u32 len) > > struct list_head *xskb_list; > > u32 contd = 0; > > u32 num_desc; > > - int err; > > > > - if (likely(!frags)) { > > - err = __xsk_rcv_zc(xs, xskb, len, contd); > > - if (err) > > - goto err; > > - return 0; > > - } > > + if (likely(!frags)) > > + return __xsk_rcv_zc(xs, xskb, len, contd); > > > > contd = XDP_PKT_CONTD; > > num_desc = xdp_get_shared_info_from_buff(xdp)->nr_frags + 1; > > if (xskq_prod_nb_free(xs->rx, num_desc) < num_desc) { > > xs->rx_queue_full++; > > - err = -ENOBUFS; > > - goto err; > > + return -ENOBUFS; > > } > > > > __xsk_rcv_zc_safe(xs, xskb, len, contd); > > @@ -252,9 +246,6 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct > > xdp_buff *xdp, u32 len) > > } > > > > return 0; > > -err: > > - xsk_buff_free(xdp); > > - return err; > > } > > > > static void *xsk_copy_xdp_start(struct xdp_buff *from) > > > > --- > > base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc > > change-id: 20261019-xsk-zc-rx-double-free-1a3b059cd41f > > > > Best regards, > > -- > > Marcin Knap <[email protected]> > > > > >
