From: Slawomir Stepien <sst@poczta.fm>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
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: Thu, 5 Nov 2026 09:17:37 +0100 [thread overview]
Message-ID: <auw7ofYYYwOoGH2d@nr200> (raw)
In-Reply-To: <ausorQSZn/tuxUWg@boxer>
On lis 04, 2026 13:44, Maciej Fijalkowski wrote:
> 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?
That. For me, at this point in time it is easier to reason about the fix first rather than refactor.
> Regardless, thank you for reviving this work, I am to blame for not
> dedicating further effort here.
No worries Maciej! You did a great job and I think it is worth pushing.
> [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
> >
--
Slawomir Stepien
next prev parent reply other threads:[~2026-11-05 8:17 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 ` [PATCH v6 bpf 0/3] xdp: fix page_pool leaks Maciej Fijalkowski
2026-11-05 8:17 ` Slawomir Stepien [this message]
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=auw7ofYYYwOoGH2d@nr200 \
--to=sst@poczta.fm \
--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=maciej.fijalkowski@intel.com \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
--cc=sdf@fomichev.me \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox