netdev - Re: [PATCH net-next] net: skbuff: sprinkle more __GFP

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoBNPUCCBhH_7iy4cNXQ0Mtrpe597DXos+s+NS7FVQ__zg@mail.gmail.com>
Date: Fri, 2 Aug 2024 12:52:06 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com
Subject: Re: [PATCH net-next] net: skbuff: sprinkle more __GFP_NOWARN on
 ingress allocs

Hello Jakub,

On Fri, Aug 2, 2024 at 8:20 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> build_skb() and frag allocations done with GFP_ATOMIC will
> fail in real life, when system is under memory pressure,

It's true. It can frequently happen under huge pressure.

> and there's nothing we can do about that. So no point
> printing warnings.

As you said, we cannot handle it because of that flag, but I wonder if
we at least let users/admins know about this failure, like: adding MIB
counter or trace_alloc_skb() tracepoint, which can also avoid printing
too many useless/necessary warnings. Or else, people won't know what
exactly happens in the kernel.

Thanks,
Jason

>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  net/core/skbuff.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 83f8cd8aa2d1..de2a044cc665 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -314,8 +314,8 @@ void *__napi_alloc_frag_align(unsigned int fragsz, unsigned int align_mask)
>         fragsz = SKB_DATA_ALIGN(fragsz);
>
>         local_lock_nested_bh(&napi_alloc_cache.bh_lock);
> -       data = __page_frag_alloc_align(&nc->page, fragsz, GFP_ATOMIC,
> -                                      align_mask);
> +       data = __page_frag_alloc_align(&nc->page, fragsz,
> +                                      GFP_ATOMIC | __GFP_NOWARN, align_mask);
>         local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
>         return data;
>
> @@ -330,7 +330,8 @@ void *__netdev_alloc_frag_align(unsigned int fragsz, unsigned int align_mask)
>                 struct page_frag_cache *nc = this_cpu_ptr(&netdev_alloc_cache);
>
>                 fragsz = SKB_DATA_ALIGN(fragsz);
> -               data = __page_frag_alloc_align(nc, fragsz, GFP_ATOMIC,
> +               data = __page_frag_alloc_align(nc, fragsz,
> +                                              GFP_ATOMIC | __GFP_NOWARN,
>                                                align_mask);
>         } else {
>                 local_bh_disable();
> @@ -349,7 +350,7 @@ static struct sk_buff *napi_skb_cache_get(void)
>         local_lock_nested_bh(&napi_alloc_cache.bh_lock);
>         if (unlikely(!nc->skb_count)) {
>                 nc->skb_count = kmem_cache_alloc_bulk(net_hotdata.skbuff_cache,
> -                                                     GFP_ATOMIC,
> +                                                     GFP_ATOMIC | __GFP_NOWARN,
>                                                       NAPI_SKB_CACHE_BULK,
>                                                       nc->skb_cache);
>                 if (unlikely(!nc->skb_count)) {
> @@ -418,7 +419,8 @@ struct sk_buff *slab_build_skb(void *data)
>         struct sk_buff *skb;
>         unsigned int size;
>
> -       skb = kmem_cache_alloc(net_hotdata.skbuff_cache, GFP_ATOMIC);
> +       skb = kmem_cache_alloc(net_hotdata.skbuff_cache,
> +                              GFP_ATOMIC | __GFP_NOWARN);
>         if (unlikely(!skb))
>                 return NULL;
>
> @@ -469,7 +471,8 @@ struct sk_buff *__build_skb(void *data, unsigned int frag_size)
>  {
>         struct sk_buff *skb;
>
> -       skb = kmem_cache_alloc(net_hotdata.skbuff_cache, GFP_ATOMIC);
> +       skb = kmem_cache_alloc(net_hotdata.skbuff_cache,
> +                              GFP_ATOMIC | __GFP_NOWARN);
>         if (unlikely(!skb))
>                 return NULL;
>
> --
> 2.45.2
>
>