netdev - Re: [PATCH bpf-next 2/9] net: add SO_BUSY_POLL

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: <03a5a7d5-e5c9-5c61-8e8e-9393e8772d88@intel.com>
Date:   Thu, 12 Nov 2020 15:45:35 +0100
From:   Björn Töpel <bjorn.topel@...el.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        Björn Töpel <bjorn.topel@...il.com>
Cc:     netdev <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        magnus.karlsson@...el.com, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        maciej.fijalkowski@...el.com,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        qi.z.zhang@...el.com, Jakub Kicinski <kuba@...nel.org>,
        Jonathan Lemon <jonathan.lemon@...il.com>, maximmi@...dia.com
Subject: Re: [PATCH bpf-next 2/9] net: add SO_BUSY_POLL_BUDGET socket option

On 2020-11-12 15:36, Eric Dumazet wrote:
> On Thu, Nov 12, 2020 at 12:41 PM Björn Töpel <bjorn.topel@...il.com> wrote:
>>
>> From: Björn Töpel <bjorn.topel@...el.com>
>>
>> This option lets a user set a per socket NAPI budget for
>> busy-polling. If the options is not set, it will use the default of 8.
>>
>> Signed-off-by: Björn Töpel <bjorn.topel@...el.com>
>> ---
>>
> 
> ...
> 
>>   #else /* CONFIG_NET_RX_BUSY_POLL */
>>   static inline unsigned long net_busy_loop_on(void)
>> @@ -106,7 +108,8 @@ static inline void sk_busy_loop(struct sock *sk, int nonblock)
>>
>>          if (napi_id >= MIN_NAPI_ID)
>>                  napi_busy_loop(napi_id, nonblock ? NULL : sk_busy_loop_end, sk,
>> -                              READ_ONCE(sk->sk_prefer_busy_poll));
>> +                              READ_ONCE(sk->sk_prefer_busy_poll),
>> +                              sk->sk_busy_poll_budget ?: BUSY_POLL_BUDGET);
> 
> Please use :
> 
>         READ_ONCE(sk->sk_busy_poll_budget) ?: BUSY_POLL_BUDGET
> 
> Because sk_busy_loop() is usually called without socket lock being held.
> 
> This will prevent yet another KCSAN report.
> 
>>   #endif
>>   }
>>
> 
> ...
> 
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -1165,6 +1165,16 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>>                  else
>>                          sk->sk_prefer_busy_poll = valbool;
>>                  break;
>> +       case SO_BUSY_POLL_BUDGET:
>> +               if (val > sk->sk_busy_poll_budget && !capable(CAP_NET_ADMIN)) {
>> +                       ret = -EPERM;
>> +               } else {
>> +                       if (val < 0)
> 
>                 if (val < 0 || val > (u16)~0)
> 
>> +                               ret = -EINVAL;
>> +                       else
>> +                               sk->sk_busy_poll_budget = val;
> 
> 
>                                 WRITE_ONCE(sk->sk_busy_poll_budget, val);
>

Thanks for the review! I'll address it all.