Re: [PATCH v6 bpf 0/3] xdp: fix page_pool leaks

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 11/5/26 09:17, Slawomir Stepien wrote:
> 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.
> 

I like approach where __do_xdp_generic() is called from veth.
Look at patch link[0] below - how much (ugly) code is removed from veth.c

>> 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/
>>
I like how much code this [0] patch removes. Especially removing the
ugly veth_xdp_get() function that manipulate the refcnt directly on pages.

Thanks for working on this, much appreciated!

--Jesper

Cc. Tianyi Gao <tianyi@cloudflare.com> whom is working on improving veth
for Cloudflare use-cases.