* Re: [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full
2026-10-30 8:14 [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full Marcin Knap via B4 Relay
@ 2026-10-30 8:47 ` bot+bpf-ci
2026-10-30 12:55 ` Maciej Fijalkowski
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: bot+bpf-ci @ 2026-10-30 8:47 UTC (permalink / raw)
To: marcin.knap, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev,
davem, edumazet, kuba, pabeni, magnus.karlsson,
maciej.fijalkowski, sdf, horms, ast, daniel, hawk, john.fastabend,
jtollet
Cc: intel-wired-lan, netdev, linux-kernel, bpf, marcin.knap, joshwash,
hramamurthy, sgoutham, gakula, rkannoth, sbhatta, bbhushan2, ast,
andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3871 bytes --]
> xsk: Do not free ZC Rx buffer when the Rx ring is full
TL;DR: Commit message only: the code change looks correct, but the "freed
twice" claim seems inaccurate for drivers that free immediately (i40e, ice,
etc.); only deferred-release drivers like mlx5e are affected.
> xsk: Do not free ZC Rx buffer when the Rx ring is full
>
> 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
> most drivers rely on that: mlx5e treats the failure as XDP_DROP and
> releases the buffer when the Rx WQE is refilled, while i40e, ice,
> ixgbe, igb, igc, stmmac, idpf and others free it themselves. 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. ixgbe and igb keep the buffer in the ring on their need_wakeup
> -ENOBUFS (XDP_EXIT) path and run it through XDP again on the next poll,
> after the core has freed it.
>
> 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. A few drivers relied on the core freeing the buffer, so make them
> free it:
>
> - i40e and ice consume the descriptor on XDP_EXIT without freeing it.
> - gve DQO counts the redirect error without freeing the buffer.
> - octeontx2 returns false on a failed redirect, so the caller goes on
> to build an skb from the already freed buffer. Free the buffer and
> report it as consumed instead.
>
> 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.
>
> Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full")
> Signed-off-by: Marcin Knap <marcin.knap@vecima.com>
The changelog says that for i40e, ice, ixgbe, igb, igc, stmmac, idpf and the
others the buffer is "freed twice" and that "the second free puts an in-use
buffer back on the pool free list". Is this accurate for those drivers?
All of them call xsk_buff_free() on the redirect failure path straight after
xdp_do_redirect() returns, before any xp_alloc() can run. xp_free() starts
with:
void xp_free(struct xdp_buff_xsk *xskb)
{
if (!list_empty(&xskb->list_node))
return;
...
and the xsk_buff_free() from the old xsk_rcv_zc() err label has already put
list_node on pool->free_list. So wouldn't the second free simply return
early here?
For a multi-buffer frame, xskb_list is already emptied by then, and the head
hits the same early return.
The corruption described seems to happen only where the second release comes
later, after the buffer may have been reallocated:
- mlx5e releases the buffer on WQE refill.
- ixgbe and igb keep the buffer in the ring on XDP_EXIT and run XDP on it
again.
- dpaa2 calls dpaa2_eth_recycle_buf(), which hands the address back to the
hardware pool while the buffer is also on free_list.
- octeontx2 builds an skb from the freed buffer.
The code change itself looks correct. Could the changelog be reworded to
name the drivers with a deferred second release, or the reuse after free, as
the ones affected, instead of the drivers that free the buffer immediately?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41025962641
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full
2026-10-30 8:14 [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full Marcin Knap via B4 Relay
2026-10-30 8:47 ` bot+bpf-ci
@ 2026-10-30 12:55 ` Maciej Fijalkowski
2026-10-30 13:37 ` Björn Töpel
2026-10-30 17:18 ` Jerome Tollet
3 siblings, 0 replies; 5+ messages in thread
From: Maciej Fijalkowski @ 2026-10-30 12:55 UTC (permalink / raw)
To: marcin.knap
Cc: 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, jtollet,
intel-wired-lan, netdev, linux-kernel, bpf, Joshua Washington,
Harshitha Ramamurthy, Sunil Goutham, Geetha sowjanya,
Ratheesh Kannoth, Subbaraya Sundeep, Bharat Bhushan
On Fri, Oct 30, 2026 at 09:14:17AM +0100, Marcin Knap via B4 Relay wrote:
> From: Marcin Knap <marcin.knap@vecima.com>
>
> 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
> most drivers rely on that: mlx5e treats the failure as XDP_DROP and
> releases the buffer when the Rx WQE is refilled, while i40e, ice,
> ixgbe, igb, igc, stmmac, idpf and others free it themselves. 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. ixgbe and igb keep the buffer in the ring on their need_wakeup
> -ENOBUFS (XDP_EXIT) path and run it through XDP again on the next poll,
> after the core has freed it.
>
> 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. A few drivers relied on the core freeing the buffer, so make them
> free it:
>
> - i40e and ice consume the descriptor on XDP_EXIT without freeing it.
> - gve DQO counts the redirect error without freeing the buffer.
> - octeontx2 returns false on a failed redirect, so the caller goes on
> to build an skb from the already freed buffer. Free the buffer and
> report it as consumed instead.
>
> 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.
>
> Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full")
> Signed-off-by: Marcin Knap <marcin.knap@vecima.com>
> ---
> 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.
> ---
> Changes in v2:
> - Also free the buffer on zero-copy redirect failure in gve DQO and
> octeontx2, which relied on the core freeing it (AI review).
> - Audited the remaining zero-copy drivers against Björn's table.
> ixgbe and igb no longer run XDP on a freed buffer after
> XDP_EXIT; making them drop and consume the descriptor (R2) is
> left for a follow-up unless you prefer it in this patch.
> - Rebased on current bpf.
> - Link to v1: https://patch.msgid.link/20261019-xsk-zc-rx-double-free-v1-1-2aa19f6628fb@vecima.com
>
> To: Joshua Washington <joshwash@google.com>
> To: Harshitha Ramamurthy <hramamurthy@google.com>
> To: Andrew Lunn <andrew+netdev@lunn.ch>
> To: "David S. Miller" <davem@davemloft.net>
> To: Eric Dumazet <edumazet@kernel.org>
> To: Jakub Kicinski <kuba@kernel.org>
> To: Paolo Abeni <pabeni@redhat.com>
> To: Tony Nguyen <anthony.l.nguyen@intel.com>
> To: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> To: Sunil Goutham <sgoutham@marvell.com>
> To: Geetha sowjanya <gakula@marvell.com>
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> To: Subbaraya Sundeep <sbhatta@marvell.com>
> To: Bharat Bhushan <bbhushan2@marvell.com>
> To: Magnus Karlsson <magnus.karlsson@intel.com>
> To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> To: Stanislav Fomichev <sdf@fomichev.me>
> To: Simon Horman <horms@kernel.org>
> To: Alexei Starovoitov <ast@kernel.org>
> To: Daniel Borkmann <daniel@iogearbox.net>
> To: Jesper Dangaard Brouer <hawk@kernel.org>
> To: John Fastabend <john.fastabend@gmail.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: bpf@vger.kernel.org
> ---
> drivers/net/ethernet/google/gve/gve_rx_dqo.c | 4 +++-
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 1 +
> drivers/net/ethernet/intel/ice/ice_xsk.c | 1 +
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 3 ++-
> net/xdp/xsk.c | 15 +++------------
> 5 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> index 82ed5e48b00a..cc544420df11 100644
> --- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> @@ -686,8 +686,10 @@ static void gve_xsk_done_dqo(struct gve_priv *priv, struct gve_rx_ring *rx,
> goto err;
> break;
> case XDP_REDIRECT:
> - if (unlikely(xdp_do_redirect(priv->dev, xdp, xprog)))
> + if (unlikely(xdp_do_redirect(priv->dev, xdp, xprog))) {
> + xsk_buff_free(xdp);
> goto err;
> + }
> break;
> }
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 51a05ce4c7ce..a11ee1f3fa38 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -360,6 +360,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);
This seems to be enough to avoid double-free on non-need_wakeup case when
core was releasing the buffer.
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
One nit is AF_XDP patches are routed via net/net-next since some time, but
I don't know if you have to resend or not.
> failure = true;
> first = NULL;
> break;
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> index 90af9b9044aa..6d10c132f61c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
> @@ -1555,7 +1555,8 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic *pfvf,
> *need_xdp_flush = true;
> return true;
> }
> - return false;
> + xsk_buff_free(xsk_buff);
> + return true;
> }
>
> err = xdp_do_redirect(pfvf->netdev, &xdp, prog);
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 781ba46eb152..89bac2feeff2 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: fb7bd3a2aad70a76232a7168a505386d0a1396af
> change-id: 20261019-xsk-zc-rx-double-free-1a3b059cd41f
>
> Best regards,
> --
> Marcin Knap <marcin.knap@vecima.com>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full
2026-10-30 8:14 [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full Marcin Knap via B4 Relay
2026-10-30 8:47 ` bot+bpf-ci
2026-10-30 12:55 ` Maciej Fijalkowski
@ 2026-10-30 13:37 ` Björn Töpel
2026-10-30 17:18 ` Jerome Tollet
3 siblings, 0 replies; 5+ messages in thread
From: Björn Töpel @ 2026-10-30 13:37 UTC (permalink / raw)
To: Marcin Knap via B4 Relay, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Magnus Karlsson, Maciej Fijalkowski,
Stanislav Fomichev, Simon Horman, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, jtollet
Cc: intel-wired-lan, netdev, linux-kernel, bpf, Marcin Knap,
Joshua Washington, Harshitha Ramamurthy, Sunil Goutham,
Geetha sowjanya, Ratheesh Kannoth, Subbaraya Sundeep,
Bharat Bhushan
Marcin Knap via B4 Relay <devnull+marcin.knap.vecima.com@kernel.org>
writes:
> From: Marcin Knap <marcin.knap@vecima.com>
>
> 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
> most drivers rely on that: mlx5e treats the failure as XDP_DROP and
> releases the buffer when the Rx WQE is refilled, while i40e, ice,
> ixgbe, igb, igc, stmmac, idpf and others free it themselves. 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. ixgbe and igb keep the buffer in the ring on their need_wakeup
> -ENOBUFS (XDP_EXIT) path and run it through XDP again on the next poll,
> after the core has freed it.
>
> 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. A few drivers relied on the core freeing the buffer, so make them
> free it:
>
> - i40e and ice consume the descriptor on XDP_EXIT without freeing it.
> - gve DQO counts the redirect error without freeing the buffer.
> - octeontx2 returns false on a failed redirect, so the caller goes on
> to build an skb from the already freed buffer. Free the buffer and
> report it as consumed instead.
>
> 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.
>
> Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full")
> Signed-off-by: Marcin Knap <marcin.knap@vecima.com>
> ---
> 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.
> ---
> Changes in v2:
> - Also free the buffer on zero-copy redirect failure in gve DQO and
> octeontx2, which relied on the core freeing it (AI review).
> - Audited the remaining zero-copy drivers against Björn's table.
> ixgbe and igb no longer run XDP on a freed buffer after
> XDP_EXIT; making them drop and consume the descriptor (R2) is
> left for a follow-up unless you prefer it in this patch.
> - Rebased on current bpf.
> - Link to v1: https://patch.msgid.link/20261019-xsk-zc-rx-double-free-v1-1-2aa19f6628fb@vecima.com
Thank you for catching, and cleaning up the drivers.
I can follow-up with the documentation [1], or if you respin, feel free
to add it yourself.
Reviewed-by: Björn Töpel <bjorn@kernel.org>
[1] https://gist.github.com/fb-bjorn/d6835c49650bc3433b427a9c9708fbe2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full
2026-10-30 8:14 [PATCH bpf v2] xsk: Do not free ZC Rx buffer when the Rx ring is full Marcin Knap via B4 Relay
` (2 preceding siblings ...)
2026-10-30 13:37 ` Björn Töpel
@ 2026-10-30 17:18 ` Jerome Tollet
3 siblings, 0 replies; 5+ messages in thread
From: Jerome Tollet @ 2026-10-30 17:18 UTC (permalink / raw)
To: Marcin Knap; +Cc: bpf, netdev, Maciej Fijalkowski
Hi Marcin,
I tested this patch on a ConnectX-5 (firmware 16.26.1040) with
AF_XDP zero-copy. Both test kernels include my earlier mlx5e fixes,
e01620844c5c and 63811edf5125. The only source difference between
them is this v2 patch.
The receiver deliberately stopped consuming the RX ring for two
seconds while continuing to supply FQ buffers. Each run recorded
roughly 1.8–2.6 million RX-ring-full events.
With striding RQ enabled, the kernel without this patch reported
225 RX frame ownership errors immediately after consumption resumed.
I reproduced this with need_wakeup both enabled and disabled. With
this patch, both runs completed 15 seconds and received over
15 million packets without an ownership error.
With cyclic RQ, neither kernel reported an ownership error under
the same test. This is consistent with my earlier cyclic reproducer,
which was fixed by the mlx5e change alone.
So this test shows a remaining striding-RQ failure despite both mlx5e
fixes, and confirms that this patch resolves it on my CX5.
Thanks,
Jerome
^ permalink raw reply [flat|nested] 5+ messages in thread