Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

[Magnus Karlsson]

On Tue, 20 Oct 2026 at 00:14, Maciej Fijalkowski
<maciej.fijalkowski@xxxxxxxxx> wrote:
>
> On Mon, Oct 19, 2026 at 10:22:02AM +0200, Marcin Knap via B4 Relay wrote:
> > From: Marcin Knap <marcin.knap@xxxxxxxxxx>
> >
> > 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 <marcin.knap@xxxxxxxxxx>
> > ---
> > 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 <marcin.knap@xxxxxxxxxx>
> >
> >
>