Re: [PATCH bpf-next v1 1/2] xdp: recycle Page Pool backed skbs built from XDP frames

Alexander Lobakin
2023-03-03T13:28:01Z

From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Fri, 3 Mar 2023 20:44:24 +0800

> On 2023/3/3 19:22, Alexander Lobakin wrote:
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>> Date: Thu, 2 Mar 2023 10:30:13 +0800

[...]

>> And they are fixed :D
>> No drivers currently which use Page Pool mix PP pages with non-PP. And
>
> The wireless adapter which use Page Pool *does* mix PP pages with
> non-PP, see below discussion:
>
> https://lore.kernel.org/netdev/156f3e120bd0757133cb6bc11b76889637b5e0a6.camel@gmail.com/

Ah right, I remember that (also was fixed).
Not that I think it is correct to mix them -- for my PoV, a driver
shoule either give *all* its Rx buffers as PP-backed or not use PP at all.

[...]

>> As Jesper already pointed out, not having a quick way to check whether
>> we have to check ::pp_magic at all can decrease performance. So it's
>> rather a shortcut.
>
> When we are freeing a page by updating the _refcount, I think
> we are already touching the cache of ::pp_magic.

But no page freeing happens before checking for skb->pp_recycle, neither
in skb_pp_recycle() (skb_free_head() etc.)[0] nor in skb_frag_unref()[1].

>
> Anyway, I am not sure checking ::pp_magic is correct when a
> page will be passing between different subsystem and back to
> the network stack eventually, checking ::pp_magic may not be
> correct if this happens.
>
> Another way is to use the bottom two bits in bv_page, see:
> https://www.spinics.net/lists/netdev/msg874099.html
>
>>
>>>
>>>>
>>>> /* Allow SKB to reuse area used by xdp_frame */
>>>> xdp_scrub_frame(xdpf);
>>>>
>>
>> Thanks,
>> Olek
>> .
>>

[0] https://elixir.bootlin.com/linux/latest/source/net/core/skbuff.c#L808
[1]
https://elixir.bootlin.com/linux/latest/source/include/linux/skbuff.h#L3385

Thanks,
Olek