netdev - Re: [PATCH v2] net: core: avoid extra pskb_expand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iLFO8W6zDEBfoPzUMsAK4Pvei_gcUYufu+s8EYcit=MFg@mail.gmail.com>
Date: Fri, 6 Jun 2025 07:14:11 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Dmitry Antipov <dmantipov@...dex.ru>
Cc: Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v2] net: core: avoid extra pskb_expand_head() when
 adjusting headroom

On Fri, Jun 6, 2025 at 12:55 AM Dmitry Antipov <dmantipov@...dex.ru> wrote:
>
> In 'skb_realloc_headroom()' and 'skb_expand_head()', using 'skb_clone()'
> with following 'pskb_expand_head()' looks suboptimal, and it's expected to
> be a bit faster to do 'skb_copy_expand()' with desired headroom instead.
>
> Signed-off-by: Dmitry Antipov <dmantipov@...dex.ru>
> ---
> v2: pass 'skb_tailroom(...)' to 'skb_copy_expand(...)' since
> the latter expects the final size rather than an increment
> ---
>  net/core/skbuff.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 85fc82f72d26..afd346c842e4 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2316,14 +2316,10 @@ struct sk_buff *skb_realloc_headroom(struct sk_buff *skb, unsigned int headroom)
>
>         if (delta <= 0)
>                 skb2 = pskb_copy(skb, GFP_ATOMIC);
> -       else {
> -               skb2 = skb_clone(skb, GFP_ATOMIC);
> -               if (skb2 && pskb_expand_head(skb2, SKB_DATA_ALIGN(delta), 0,
> -                                            GFP_ATOMIC)) {
> -                       kfree_skb(skb2);
> -                       skb2 = NULL;
> -               }
> -       }
> +       else
> +               skb2 = skb_copy_expand(skb, (skb_headroom(skb) +
> +                                            SKB_DATA_ALIGN(delta)),
> +                                      skb_tailroom(skb), GFP_ATOMIC);
>         return skb2;

Please read the comment about this function :

/* Make private copy of skb with writable head and some headroom */

So the skb_clone() is needed.