netdev - Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.

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]

Date:   Thu, 18 Oct 2018 16:32:29 -0700
From:   Nitin Hande <nitin.hande@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Joe Stringer <joe@...d.net.nz>, netdev <netdev@...r.kernel.org>,
        ast@...nel.org, Jesper Brouer <brouer@...hat.com>,
        john fastabend <john.fastabend@...il.com>
Subject: Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP
 hookpoint.

On Thu, 18 Oct 2018 23:20:17 +0200
Daniel Borkmann <daniel@...earbox.net> wrote:

> On 10/18/2018 11:06 PM, Joe Stringer wrote:
> > On Thu, 18 Oct 2018 at 11:54, Nitin Hande <nitin.hande@...il.com> wrote:  
> [...]
> >> Open Issue
> >> * The underlying code relies on presence of an skb to find out the
> >> right sk for the case of REUSEPORT socket option. Since there is
> >> no skb available at XDP hookpoint, the helper function will return
> >> the first available sk based off the 5 tuple hash. If the desire
> >> is to return a particular sk matching reuseport_cb function, please
> >> suggest way to tackle it, which can be addressed in a future commit.  
> 
> >> Signed-off-by: Nitin Hande <Nitin.Hande@...il.com>  
> > 
> > Thanks Nitin, LGTM overall.
> > 
> > The REUSEPORT thing suggests that the usage of this helper from XDP
> > layer may lead to a different socket being selected vs. the equivalent
> > call at TC hook, or other places where the selection may occur. This
> > could be a bit counter-intuitive.
> > 
> > One thought I had to work around this was to introduce a flag,
> > something like BPF_F_FIND_REUSEPORT_SK_BY_HASH. This flag would
> > effectively communicate in the API that the bpf_sk_lookup_xxx()
> > functions will only select a REUSEPORT socket based on the hash and
> > not by, for example BPF_PROG_TYPE_SK_REUSEPORT programs. The absence
> > of the flag would support finding REUSEPORT sockets by other
> > mechanisms (which would be allowed for now from TC hooks but would be
> > disallowed from XDP, since there's no specific plan to support this). 

Thanks Joe for the quick response.This certainly looks feasible. With the
flag, both tc and XDP hookpoints will be consistent in their approach.
 
> 
> Hmm, given skb is NULL here the only way to lookup the socket in such
> scenario is based on hash, that is, inet_ehashfn() / inet6_ehashfn(),
> perhaps alternative is to pass this hash in from XDP itself to the
> helper so it could be custom selector.

Interesting, and this will be an additional helper or done within this
sk_lookup() helper ?

 Do you have a specific use case
> on this for XDP (just curious)?

Yes, this is for a dual-stack solution. The XDP program functions as a
demux, if there is a receiver the packet will be ingressed on Linux
networking stack, else it enters the other stack path.

Thanks
Nitin

> 
> Thanks,
> Daniel