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

[Alexander Lobakin]

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

> On 2023/3/3 19:22, Alexander Lobakin wrote:
>> From: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
>> 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@xxxxxxxxx/

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