Re: [PATCH net v9] skb_expand_head() adjust skb->truesize incorrectly

From: Eric Dumazet
Date: Wed Oct 20 2021 - 12:18:22 EST




On 10/4/21 10:57 PM, Vasily Averin wrote:
> On 10/4/21 10:26 PM, Eric Dumazet wrote:

>>
>> Why not re-using is_skb_wmem() here ?
>> Testing != sock_edemux looks strange.
>
> All non-wmem skbs was cloned and then was freed already.
> After pskb_expand_head() call we can have:
> (1) either original wmem skbs
> (2) or cloned skbs:
> (2a) either without sk at all,
> (2b) or with sock_edemux destructor (that was set inside skb_set_owner_w() for !sk_fullsock(sk))
> (2c) or with sock_wfree destructor (that was set inside skb_set_owner_w() for sk_fullsock(sk))
>
> (2a) and (2b) do not require truesize/sk_wmem_alloc update, it was handled inside pskb_expand_head()
> (1) and (2c) cases are processed here.
>
> If required I can add this explanation either into patch description or as comment.
>

sock_edemux is one of the current destructors.

New ones will be added later. We can not expect that in two or three years,
at least one reviewer will remember this special case.

I would prefer you list the known destructors (allow-list, instead of disallow-list)



> Btw I just noticed that we can avoid cloning for original skbs without sk.
> How do you think should I do it?

Lets wait a bit before new features...