Re: [PATCH 1/1] net: Add SO_REUSEPORT_LISTEN_OFF socket option as drain mode

[Daniel Borkmann <daniel@iogearbox.net>]

On 03/24/2016 06:26 PM, Tom Herbert wrote:
> On Thu, Mar 24, 2016 at 10:01 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Thu, 2016-03-24 at 17:50 +0100, Willy Tarreau wrote:
>>> On Thu, Mar 24, 2016 at 09:33:11AM -0700, Eric Dumazet wrote:
>>>>> --- a/net/ipv4/inet_hashtables.c
>>>>> +++ b/net/ipv4/inet_hashtables.c
>>>>> @@ -189,6 +189,8 @@ static inline int compute_score(struct sock *sk, struct net *net,
>>>>>                                  return -1;
>>>>>                          score += 4;
>>>>>                  }
>>>>> +               if (sk->sk_reuseport)
>>>>> +                       score++;
>>>>
>>>> This wont work with BPF
>>>>
>>>>>                  if (sk->sk_incoming_cpu == raw_smp_processor_id())
>>>>>                          score++;
>>>>
>>>> This one does not work either with BPF
>>>
>>> But this *is* in 4.5. Does this mean that this part doesn't work anymore or
>>> just that it's not usable in conjunction with BPF ? In this case I'm less
>>> worried, because it would mean that we have a solution for non-BPF aware
>>> applications and that BPF-aware applications can simply use BPF.
>>
>> BPF can implement the CPU choice/pref itself. It has everything needed.
>>
>>> I don't try to reimplement something already available, but I'm confused
>>> by a few points :
>>>    - the code above already exists and you mention it cannot be used with BPF
>>
>> _If_ you use BPF, then you can implement a CPU preference using BPF
>> instructions. It is a user choice.
>>
>>>    - for the vast majority of applications not using BPF, would the above *still*
>>>      work (it worked in 4.4-rc at least)
>>
>>>    - it seems to me that for BPF to be usable on process shutting down, we'd
>>>      need to have some form of central knowledge if the goal is to redefine
>>>      how to distribute the load. In my case there are multiple independant
>>>      processes forked on startup, so it's unclear to me how each of them could
>>>      reconfigure BPF when shutting down without risking to break the other ones.
>>>    - the doc makes me believe that BPF would require privileges to be unset, so
>>>      that would not be compatible with a process shutting down which has already
>>>      dropped its privileges after startup, but I could be wrong.
>>>
>>> Thanks for your help on this,
>>> Willy
>>
>> The point is : BPF is the way to go, because it is expandable.
>>
>> No more hard points coded forever in the kernel.
>>
>> Really, when BPF can be the solution, we wont allow adding new stuff in
>> the kernel in the old way.
>
> I completely agree with this, but I wonder if we now need a repository
> of useful BPF modules. So in the case of implementing functionality
> like in SO_REUSEPORT_LISTEN_OFF that might just become a common BPF
> program we could direct people to use.

Good point. There's tools/testing/selftests/net/ containing already reuseport
BPF example, maybe it could be extended.