Netdev List
 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: 7+ 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: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]

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox