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

[Tom Herbert <tom@herbertland.com>]

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.

Tom

>
>
>