* Re: [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data()
2026-10-21 15:56 [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data() Dudu Lu
@ 2026-10-21 16:32 ` sashiko-bot
2026-10-21 16:59 ` bot+bpf-ci
2026-10-21 17:40 ` Alexei Starovoitov
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-21 16:32 UTC (permalink / raw)
To: Dudu Lu; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net: bpf_xdp_shrink_data() unrefs page_pool on non-recycled shared frags
- [Low] bpf: multi-line comment style violation in bpf_xdp_shrink_data()
--
commit a0cb1ce1fd03b9aff34c23b2b43d38801d791b8d
Author: Dudu Lu <phx0fer@gmail.com>
net: return page_pool frags to the pool in bpf_xdp_shrink_data()
This commit updates bpf_xdp_shrink_data() in net/core/filter.c to check
whether a released fragment with MEM_TYPE_PAGE_SHARED is backed by a
page_pool and recycle it via napi_pp_put_page() before falling back to
__xdp_return(). This avoids freeing page_pool-backed fragments allocated
by skb_pp_cow_data() through page_frag_free() under generic xdp when the
rxq has no registered memory model.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b7ac145b68b1f..f17113ff29874 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -4408,6 +4409,18 @@ static bool bpf_xdp_shrink_data(struct xdp_buff *xdp, skb_frag_t *frag,
> }
>
> if (release) {
> + /* The frag may be page_pool-backed even when the rxq's
> + * xdp_rxq_info carries no mem model (generic XDP on a
> + * driver that never registers one, e.g. loopback). In that
> + * case mem_type is MEM_TYPE_PAGE_SHARED and __xdp_return()
> + * would free the page through page_frag_free(), leaving the
> + * page_pool's accounting dangling -- "page_pool leak". Route
> + * page_pool pages back through the pool instead.
> + */
[Severity: Low]
This isn't a bug, but in bpf subsystem files such as net/core/filter.c,
should multi-line comments in bpf_xdp_shrink_data() have the opening /* on
its own line with the comment text starting on the following line?
> + if (IS_ENABLED(CONFIG_PAGE_POOL) &&
> + mem_type == MEM_TYPE_PAGE_SHARED && napi_pp_put_page(netmem))
> + return release;
> +
> __xdp_return(netmem, mem_type, false, zc_frag);
[Severity: High]
Can calling napi_pp_put_page(netmem) in bpf_xdp_shrink_data() when
mem_type == MEM_TYPE_PAGE_SHARED corrupt reference counts if the xdp_buff
owns a page->_refcount reference rather than a pp_ref_count reference?
On a veth interface with native xdp enabled, veth_xdp_rcv_skb() calls
veth_convert_skb_to_xdp_buff() -> skb_pp_cow_data(). For any trailing
fragment with size <= PAGE_SIZE / 2 (such as a 4000-byte packet),
page_pool_dev_alloc() -> page_pool_alloc_netmem() calls
page_pool_alloc_frag_netmem(), which fragments the page with BIAS_MAX:
net/core/page_pool.c:page_pool_alloc_frag_netmem() {
...
pool->frag_page = netmem;
frag_reset:
pool->frag_users = 1;
*offset = 0;
pool->frag_offset = size;
page_pool_fragment_netmem(netmem, BIAS_MAX);
return netmem;
}
When bpf_prog_run_xdp() returns XDP_TX or XDP_REDIRECT, veth_xdp_rcv_skb()
transfers ownership from the skb to the xdp_buff and switches the memory
model to rq->xdp_mem (which has type MEM_TYPE_PAGE_SHARED):
drivers/net/veth.c:veth_xdp_rcv_skb() {
...
case XDP_TX:
veth_xdp_get(xdp);
consume_skb(skb);
xdp->rxq->mem = rq->xdp_mem;
if (unlikely(veth_xdp_tx(rq, xdp, bq) < 0)) {
...
}
Here, veth_xdp_get(xdp) increments page->_refcount via __skb_frag_ref(),
and consume_skb(skb) releases the skb's pp_ref_count reference via
napi_pp_put_page() (decrementing pp_ref_count from BIAS_MAX to
BIAS_MAX - 1). Because rq->page_pool->frag_page (or another skb sharing
the fragmented page) still holds the remaining pp_ref_count references,
pp_ref_count does not reach 0 and netmem_is_pp(netmem) remains true.
When a downstream program in veth_xdp_rcv_one(), dev_map_bpf_prog_run(),
or cpu_map_bpf_prog_run_xdp() shrinks and releases that fragment via
bpf_xdp_adjust_tail() or bpf_xdp_pull_data(), bpf_xdp_shrink_data() sees
mem_type == MEM_TYPE_PAGE_SHARED and netmem_is_pp(netmem) == true. Calling
napi_pp_put_page(netmem) decrements pp_ref_count a second time instead of
calling __xdp_return() -> page_frag_free() to drop the page->_refcount
reference.
Wouldn't this leak page->_refcount and cause page_pool_unref_netmem() to
underflow pp_ref_count (or prematurely recycle the page while other skbs
still hold fragments of it) when rq->page_pool drains frag_page?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021155645.69120-1-phx0fer@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data()
2026-10-21 15:56 [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data() Dudu Lu
2026-10-21 16:32 ` sashiko-bot
@ 2026-10-21 16:59 ` bot+bpf-ci
2026-10-21 17:40 ` Alexei Starovoitov
2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-10-21 16:59 UTC (permalink / raw)
To: phx0fer, ast, daniel, andrii, eddyz87, memxor
Cc: netdev, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]
> net: return page_pool frags to the pool in bpf_xdp_shrink_data()
TL;DR: Commit message only: the Fixes tag (4dce1a0d7cf3) may be wrong;
e6d5dbdd20aa (v6.9, generic XDP multi-buff) seems the real origin, so stable
backport scope is understated.
> net: return page_pool frags to the pool in bpf_xdp_shrink_data()
>
> bpf_xdp_shrink_data() releases a fully-shrunk frag through
> __xdp_return(netmem, mem_type, false, zc_frag), selecting the freer from
> the rxq's mem type. Under generic XDP the rxq's xdp_rxq_info often has no
> mem model registered (e.g. loopback), so mem_type stays
> MEM_TYPE_PAGE_SHARED == 0 even though the frag was allocated from a page
> pool by skb_pp_cow_data(). __xdp_return() then frees the page through
> page_frag_free() instead of page_pool_put_full_netmem(), so the page is
> returned to the buddy allocator while its page_pool accounting is still
> live:
>
> BUG: Bad page state in process A-146-S3_S1 pfn:4694b
> page dumped because: page_pool leak
> page_frag_free+0xf0/0x11c
> __xdp_return+0x160/0x640
> bpf_xdp_adjust_tail+0x5a4/0xbac
> bpf_prog_run_generic_xdp+0x4a8/0x1204
>
> Before falling back to __xdp_return(), try the page_pool path: if the
> frag really is a page_pool page, napi_pp_put_page() recycles it through
> the pool and we are done.
>
> Fixes: 4dce1a0d7cf3 ("bpf: Support pulling non-linear xdp data")
> Assisted-by: LLM
> Signed-off-by: Dudu Lu <phx0fer@gmail.com>
Is the Fixes: tag pointing at the right commit?
4dce1a0d7cf3 only added bpf_xdp_pull_data() as a new caller of
bpf_xdp_shrink_data(). The splat in the commit message goes through
bpf_xdp_adjust_tail() -> bpf_xdp_frags_shrink_tail() -> __xdp_return(),
which already freed released frags based on xdp->rxq->mem.type
(MEM_TYPE_PAGE_SHARED when no mem model is registered, as in generic XDP)
since c5114710c8ce ("xsk: fix usage of multi-buffer BPF helpers for ZC XDP")
and bf25146a5595 ("bpf: add frags support to the bpf_xdp_adjust_tail() API").
The mismatch only became possible when e6d5dbdd20aa made
netif_receive_generic_xdp() build non-linear xdp_buffs from page_pool backed
frags via skb_pp_cow_data(), while the generic XDP rxq has no registered mem
model. From then on the adjust_tail shrink path freed page_pool pages with
page_frag_free(), and the "page_pool leak" check already existed via
dba1b8a7ab68 ("mm/page_pool: catch page_pool memory leaks").
e6d5dbdd20aa landed in v6.9 while 4dce1a0d7cf3 landed in v6.18, so the
current tag understates the stable backport scope. Should this instead
include:
Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
---
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/39631548606
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data()
2026-10-21 15:56 [PATCH bpf] net: return page_pool frags to the pool in bpf_xdp_shrink_data() Dudu Lu
2026-10-21 16:32 ` sashiko-bot
2026-10-21 16:59 ` bot+bpf-ci
@ 2026-10-21 17:40 ` Alexei Starovoitov
2 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-21 17:40 UTC (permalink / raw)
To: Dudu Lu, daniel, andrii, eddyz87, memxor; +Cc: netdev, bpf
On Wed, Oct 21, 2026 at 08:56 AM Dudu Lu <phx0fer@gmail.com> wrote:
> Before falling back to __xdp_return(), try the page_pool path: if the
> frag really is a page_pool page, napi_pp_put_page() recycles it through
> the pool and we are done.
This makes it worse.
netmem_is_pp() says that the page is still in a pool. It doesn't say
what kind of reference the frag holds.
On XDP_TX and XDP_REDIRECT veth_xdp_rcv_skb() does get_page() on every
frag in veth_xdp_get(), consume_skb() drops the page_pool references
and the frame leaves as MEM_TYPE_PAGE_SHARED. A frag of 2k or less
shares its page with other frags, so the page is still in
rq->page_pool when the prog of the peer trims that frag in
veth_xdp_rcv_one(). Today page_frag_free() drops the reference that
get_page() took. With this patch napi_pp_put_page() drops pp_ref_count
for the second time and the page reference is leaked.
Jiayuan's v1 in August had the same check in the same place.
See why looking at the page doesn't work:
https://lore.kernel.org/bpf/01fd251d-1134-4251-9c11-79943e367ffe@linux.dev/
rxq->mem.type has to be right when the prog runs.
v3 of that series adds a flag to xdp_buff for that:
https://lore.kernel.org/bpf/20260911135711.109338-1-jiayuan.chen@linux.dev/
I think rxq on stack, like in cpu_map_bpf_prog_run_xdp(), is enough.
dev->_rx[i].xdp_rxq never has a mem model and in
bpf_prog_run_generic_xdp() the frags of a nonlinear skb always come
from skb_pp_cow_data().
Below leaves out veth. It needs the same for skbs, plus
https://lore.kernel.org/bpf/20260917185814.0f944e78@kernel.org/
Something like this (compile tested only):
diff --git a/net/core/dev.c b/net/core/dev.c
index e76762e..ecdac67 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5542,6 +5542,7 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
void *orig_data, *orig_data_end, *hard_start;
struct netdev_rx_queue *rxqueue;
bool orig_bcast, orig_host;
+ struct xdp_rxq_info rxq;
u32 mac_len, frame_sz;
__be16 orig_eth_type;
struct ethhdr *eth;
@@ -5565,6 +5566,12 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
if (skb_is_nonlinear(skb)) {
skb_shinfo(skb)->xdp_frags_size = skb->data_len;
xdp_buff_set_frags_flag(xdp);
+ /* The frags came from skb_pp_cow_data(). xdp_rxq is shared
+ * with other CPUs and with BPF_PROG_TEST_RUN, so use a copy.
+ */
+ rxq = rxqueue->xdp_rxq;
+ rxq.mem.type = MEM_TYPE_PAGE_POOL;
+ xdp->rxq = &rxq;
} else {
xdp_buff_clear_frags_flag(xdp);
}
@@ -5577,6 +5584,7 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
orig_eth_type = eth->h_proto;
act = bpf_prog_run_xdp(xdp_prog, xdp);
+ xdp->rxq = &rxqueue->xdp_rxq;
/* check if bpf_xdp_adjust_head was used */
off = xdp->data - orig_data;
xdp fixes go via net tree.
pw-bot: cr
^ permalink raw reply related [flat|nested] 4+ messages in thread