All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Slawomir Stepien <sst@poczta.fm>
Cc: <ast@kernel.org>, <daniel@iogearbox.net>, <davem@davemloft.net>,
	<kuba@kernel.org>, <hawk@kernel.org>, <john.fastabend@gmail.com>,
	<sdf@fomichev.me>, <netdev@vger.kernel.org>,
	<bpf@vger.kernel.org>, <magnus.karlsson@intel.com>,
	<aleksander.lobakin@intel.com>, <ilias.apalodimas@linaro.org>,
	<toke@redhat.com>, <lorenzo@kernel.org>,
	<ihor.solodrai@linux.dev>, <tavip@google.com>
Subject: Re: [PATCH v6 bpf 0/3] xdp: fix page_pool leaks
Date: Wed, 4 Nov 2026 13:44:29 +0100	[thread overview]
Message-ID: <ausorQSZn/tuxUWg@boxer> (raw)
In-Reply-To: <20261104121316.1155680-1-sst@poczta.fm>

On Wed, Nov 04, 2026 at 01:13:13PM +0100, Slawomir Stepien wrote:
> v1:
> https://lore.kernel.org/bpf/20251003140243.2534865-1-maciej.fijalkowski@intel.com/
> v1->v2:
> - add skb to xdp_buff converter (Jakub)
> - postpone restoration of veth's xdp_rxq_info mem model (AI)
> 
> v2:
> https://lore.kernel.org/bpf/20251017143103.2620164-1-maciej.fijalkowski@intel.com/T/
> v2->v3:
> - make skb->xdp_buff conversion agnostic of current skb->data position
>   (Toke)
> - do not pull mac header in veth (Toke)
> - use virt_to_head_page() when checking if xdp->data comes from
>   page_pool (Olek)
> - add ack from Toke
> 
> v3:
> https://lore.kernel.org/bpf/20251022125209.2649287-1-maciej.fijalkowski@intel.com/
> v3->v4:
> - include skb->head when calculating packet length (Toke, lkp)
> 
> v4:
> https://lore.kernel.org/bpf/20251027121318.2679226-1-maciej.fijalkowski@intel.com/
> v4->v5:
> - rely again on sk_buff::pp_recycle when setting mem type on rxq
>   (Jesper, Jakub)
> - add ack from Toke on patch 2 and restore it on patch 1
> 
> v5:
> https://lore.kernel.org/bpf/20251029221315.2694841-1-maciej.fijalkowski@intel.com/
> v5->v6:
> - rebase onto bpf/master
> - patch 1: set MEM_TYPE_PAGE_POOL unconditionally in
>   xdp_convert_skb_to_buff(); mem type only matters for frags consumed
>   by the program, and those always come from skb_pp_cow_data() (Jakub)
> - patch 1: add DEBUG_NET_WARN_ON_ONCE() for a non-linear skb without
>   pp_recycle and document the expectation in kdoc (Jakub); the warning
>   is limited to non-linear skbs since linear ones legitimately skip
>   the CoW
> - new patch 2: veth_xdp_rcv_one() ran the program on an xdp_frame with
>   whatever mem type the previous packet left on rq->xdp_rxq; use the
>   frame's mem type instead, as cpumap and devmap do. With patch 3 alone
>   veth sends page_pool frames to its peer and the peer's program
>   trimming frags crashed the kernel, so this comes first
> - patch 3 (was 2): veth turns the buff into an xdp_frame on
>   XDP_{TX,REDIRECT}, so the head is released through mem type too;
>   keep the queue's original model unless the buffer was copied into
>   rq->page_pool (Jakub)
> - patch 3: take page_pool refs on compound heads via netmem helpers
> - patch 3: do not restore the rxq mem model after XDP_{TX,REDIRECT}
>   and on the error path; with patch 2 every path sets its own mem type
> - patch 3: veth can now redirect MEM_TYPE_PAGE_POOL frames, which needs
>   commit b3ca872947ef ("bpf: Use the frame mem type in devmap egress
>   XDP") in devmap; list it as a stable prerequisite
> - drop Reviewed-by/Tested-by tags as patches 1 and 3 changed

I mentioned the changes [0] that were developed after v5 revision yet you
decided to go with what you had - that might be okay but to be transparent
it would be worth stating *why* you chose not to pursue further or explore
approach where __do_xdp_generic() is called from veth.

Was it due to semantics change pointed out by Bjorn? Or is it too big a
hammer for fixing page_pool leaks and should be implemented as a
follow-up?

Regardless, thank you for reviving this work, I am to blame for not
dedicating further effort here.

[0]: https://lore.kernel.org/bpf/20260509084858.773921-5-maciej.fijalkowski@intel.com/

Thanks,
MF

> 
> Hi,
> 
> here we fix page_pool leaks which happen when fragment is released
> within XDP program and memory type is set incorrectly. The reports come
> from syzbot and AF_XDP test suite.
> 
> Most of the changes are about pulling out the common code for
> init/prepare xdp_buff based on existing skb and supplying it with
> correct xdp_rxq_info's mem_type that is assigned to xdp_buff. A bit more
> stuff, page_pool related, had to be done on veth side.
> 
> Originally, this work was started by Octavian Purdila and continued by
> Maciej up to v5. Maciej then took the larger veth rework from the v5
> review to net-next as an RFC [1]. This v6 picks up the v5 series, with
> Maciej's agreement, as the minimal fix for bpf and stable, and addresses
> the v5 review. Maciej stays the author of patches 1 and 3 and my changes
> are noted in their commit messages; patch 2 is new.
> 
> For veth, Jakub listed three options: stop turning skbs into xdp_frames
> in veth, decide the head's mem type when the CoW was skipped, or CoW the
> head on the Tx/Redirect path. The first one changes veth's XDP handling
> beyond a fix (it is what the RFC does), so patch 3 takes the second:
> veth knows whether it just copied the buffer into rq->page_pool, and
> only then advertises MEM_TYPE_PAGE_POOL. An skb that skipped the CoW is
> linear and keeps the queue's original model, as before this series.
> The two do not conflict: the RFC can go on top of this series and
> replace the veth parts of patches 2 and 3.
> 
> [1] https://lore.kernel.org/all/20260509084858.773921-1-maciej.fijalkowski@intel.com/
> 
> Patch 2 fixes the same problem for frames in veth_xdp_rcv_one() that
> commit b3ca872947ef ("bpf: Use the frame mem type in devmap egress
> XDP") fixed for devmap; cpumap already handled it.
> 
> Testing (x86_64, QEMU via virtme-ng, KASAN + DEBUG_VM + DEBUG_NET; the
> series ran on bpf/master fb7bd3a2aad7 and, rebased without changes, the
> reproducers, kprobe path checks, devmap test and selftests below ran
> again on 05441d697f7f with the same results; the runs without the series
> on bpf/master c8a8a68d3ab4):
> - reproducer: an xdp.frags program that trims all frags with
>   bpf_xdp_adjust_tail() on 8000-byte pings over a 9000 MTU veth pair;
>   without the series both generic XDP and native veth XDP hit
>   "BUG: Bad page state ... page_pool leak" followed by
>   VM_BUG_ON_PAGE(page_ref_count(page) == 0) and a panic; with the
>   series 20/20 packets are trimmed with no splat
> - native veth XDP_TX of multi-buffer packets, with and without
>   trimming frags first (200 large + 200 small pings each), then veth
>   teardown and a 75s wait for page_pool "stalled pool shutdown" or
>   negative inflight warnings: none
> - BPF selftests on bpf/master with tools/testing/selftests/bpf/config:
>   ./test_progs -t ns_xsk,xdp: all 34 tests (205 subtests) passed, no
>   splats, with and without the series
> - the standalone AF_XDP suite (test_xsk.sh) also runs the tests that
>   test_progs skips as flaky; XDP_ADJUST_TAIL_SHRINK_MULTI_BUFF reports
>   PASS either way, but without the series dmesg shows 60 "page_pool
>   leak" splats from generic XDP (bpf_xdp_adjust_tail -> __xdp_return ->
>   page_frag_free on pages allocated by skb_pp_cow_data()); with the
>   series there are none
> - frame path: one veth bounces 8000-byte pings with XDP_TX, its peer
>   trims all frags of the resulting frames (IPv6 off, so no skb touches
>   the peer's rxq first); with patches 1+3 only: "page_pool leak",
>   VM_BUG_ON_PAGE() and a panic in veth_xdp_rcv_one(); with the whole
>   series 50/50 frames are trimmed, kprobes show all __xdp_return() calls
>   with MEM_TYPE_PAGE_POOL, no splat, also with IPv6 on so that skbs and
>   frames interleave on the peer's queue; bpf/master alone passes as well
>   since veth only sends MEM_TYPE_PAGE_SHARED frames there
> - remaining veth paths, each confirmed with kprobes: skbs that skip the
>   CoW (frames passed up by one veth and forwarded back into it: 1229 of
>   1260 runs without skb_pp_cow_data(), heads from the page_pool),
>   XDP_REDIRECT failing on the skb path (40 xdp_return_buff()) and on the
>   frame path (40 xdp_return_frame()), XDP_PASS of 20 frames after their
>   frags were trimmed; no splat, and none of the page_pool shutdown or
>   inflight warnings 75s after teardown
> - devmap egress after patch 3: veth redirects copied skbs through a
>   devmap whose egress program trims all frags; with the series 20/20
>   trimmed, all __xdp_return() calls with MEM_TYPE_PAGE_POOL, no splat,
>   nothing 75s after teardown; with b3ca872947ef reverted on top of the
>   series: "page_pool leak" from page_frag_free() under
>   dev_map_bpf_prog_run(), VM_BUG_ON_PAGE() and a panic, hence the
>   stable prerequisite in patch 3
> - W=1 builds of the touched files with LLVM=1 and with DEBUG_NET=n: no
>   warnings
> - XDP_ADJUST_TAIL_GROW fails in DRV mode with and without the series,
>   so it is unrelated
> - not tested: page_pool frames from hardware drivers redirected into
>   veth, bpf_xdp_pull_data(), multi-queue load, the full test_progs run
> 
> Performance (the same VM with 8 vCPUs, non-debug kernels with
> PAGE_POOL_STATS, bpf/master fb7bd3a2aad7 with and without the series;
> pktgen sends skbs into one end of a veth pair, the program on the other
> end counts packets; median of 6 runs of 8s over two boots per kernel):
> - 64-byte packets with native XDP_DROP, XDP_TX and XDP_REDIRECT (devmap
>   to another veth), and generic XDP_DROP on 1 and 4 CPUs with 64 and
>   8000-byte packets: within 2% of the base kernel, no regression
> - native veth with 8000-byte packets (3 frags): XDP_TX +8.8%
>   (0.507 -> 0.552 Mpps), XDP_REDIRECT +8.4% (0.613 -> 0.664 Mpps).
>   Without the series veth_xdp_get() takes plain page references on
>   the head and frags, so the pages copied into rq->page_pool are
>   released from the pool instead of recycled (~1.2M/s
>   rx_pp_recycle_released_ref plus slow allocations); with page_pool
>   references they go back to the ring (~1.4M/s rx_pp_recycle_ring, no
>   released_ref)
> Absolute numbers in a VM differ between boots, so only the relative
> change is meaningful.
> 
> This v6 was prepared with an AI coding assistant (Claude) from the v5
> review thread; the assistant wrote the code changes, all of patch 2,
> the changelog updates, and ran the tests listed above.
> 
> 
> 
> Maciej Fijalkowski (2):
>   xdp: introduce xdp_convert_skb_to_buff()
>   veth: update mem type in xdp_buff
> 
> Slawomir Stepien (1):
>   veth: use the frame mem type when running XDP on an xdp_frame
> 
>  drivers/net/veth.c | 56 ++++++++++++++++++++++++++++++----------------
>  include/net/xdp.h  | 36 +++++++++++++++++++++++++++++
>  net/core/dev.c     | 25 ++++-----------------
>  3 files changed, 77 insertions(+), 40 deletions(-)
> 
> 
> base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d
> -- 
> 2.56.0
> 

  parent reply	other threads:[~2026-11-04 12:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04 12:13 [PATCH v6 bpf 0/3] xdp: fix page_pool leaks Slawomir Stepien
2026-11-04 12:13 ` [PATCH v6 bpf 1/3] xdp: introduce xdp_convert_skb_to_buff() Slawomir Stepien
2026-11-04 12:29   ` sashiko-bot
2026-11-04 12:53   ` bot+bpf-ci
2026-11-04 12:13 ` [PATCH v6 bpf 2/3] veth: use the frame mem type when running XDP on an xdp_frame Slawomir Stepien
2026-11-04 12:13 ` [PATCH v6 bpf 3/3] veth: update mem type in xdp_buff Slawomir Stepien
2026-11-04 12:53   ` bot+bpf-ci
2026-11-04 12:44 ` Maciej Fijalkowski [this message]
2026-11-05  8:17   ` [PATCH v6 bpf 0/3] xdp: fix page_pool leaks Slawomir Stepien
2026-11-05  9:59     ` Jesper Dangaard Brouer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ausorQSZn/tuxUWg@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=hawk@kernel.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=ilias.apalodimas@linaro.org \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@fomichev.me \
    --cc=sst@poczta.fm \
    --cc=tavip@google.com \
    --cc=toke@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.