netdev - Re: [PATCH net-next] net: save some cycles when doing skb_attempt_defer

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: <CANn89i+2XdNxYHFNwC5LHupT3je1EaZXMxMJG9343ZO9vCzAsg@mail.gmail.com>
Date: Thu, 11 Apr 2024 07:27:36 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: pablo@...filter.org, kuba@...nel.org, pabeni@...hat.com, 
	davem@...emloft.net, horms@...nel.org, aleksander.lobakin@...el.com, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next] net: save some cycles when doing skb_attempt_defer_free()

On Thu, Apr 11, 2024 at 5:25 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> From: Jason Xing <kernelxing@...cent.com>
>
> Normally, we don't face these two exceptions very often meanwhile
> we have some chance to meet the condition where the current cpu id
> is the same as skb->alloc_cpu.
>
> One simple test that can help us see the frequency of this statement
> 'cpu == raw_smp_processor_id()':
> 1. running iperf -s and iperf -c [ip] -P [MAX CPU]
> 2. using BPF to capture skb_attempt_defer_free()
>
> I can see around 4% chance that happens to satisfy the statement.
> So moving this statement at the beginning can save some cycles in
> most cases.
>
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> ---
>  net/core/skbuff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index ab970ded8a7b..b4f252dc91fb 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7002,9 +7002,9 @@ void skb_attempt_defer_free(struct sk_buff *skb)
>         unsigned int defer_max;
>         bool kick;
>
> -       if (WARN_ON_ONCE(cpu >= nr_cpu_ids) ||
> +       if (cpu == raw_smp_processor_id() ||
>             !cpu_online(cpu) ||
> -           cpu == raw_smp_processor_id()) {
> +           WARN_ON_ONCE(cpu >= nr_cpu_ids)) {
>  nodefer:       kfree_skb_napi_cache(skb);
>                 return;
>         }

Wrong patch.

cpu_online(X) is undefined and might crash if X is out of bounds on CONFIG_SMP=y