netdev - Re: [PATCH bpf-next] bpf: add bpf_get_xdp

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 2 Sep 2020 09:56:24 -0600
From:   Tom Herbert <tom@...bertland.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Harshitha Ramamurthy <harshitha.ramamurthy@...el.com>,
        bpf@...r.kernel.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Alexander Duyck <alexander.h.duyck@...el.com>,
        "TOM.HERBERT@...EL.COM" <tom.herbert@...el.com>
Subject: Re: [PATCH bpf-next] bpf: add bpf_get_xdp_hash helper function

On Mon, Aug 31, 2020 at 2:33 PM Daniel Borkmann <daniel@...earbox.net> wrote:
>
> On 8/31/20 9:25 PM, Harshitha Ramamurthy wrote:
> > This patch adds a helper function called bpf_get_xdp_hash to calculate
> > the hash for a packet at the XDP layer. In the helper function, we call
> > the kernel flow dissector in non-skb mode by passing the net pointer
> > to calculate the hash.
>
> So this commit msg says 'what' the patch does, but says nothing about 'why' it is
> needed especially given there's the 1 mio insn limit in place where it should be
> easy to write that up in BPF anyway. The commit msg needs to have a clear rationale
> which describes the motivation behind this helper.. why it cannot be done in BPF
> itself?
>
Daniel,

We already have a fully functional, well tested, and very complete
parser supporting many protocols and packet hash functions in the
kernel in skb_flow_dissect and friends. I suppose we could replicate
all that code in eBPF but I don't think it's fair to say it's easy to
get to the same level in eBPF. eBPF does solve the problem of
extensibility of kernel code, however IMO if someone is looking for an
easy way to get a packet hash then a simple helper function makes
sense and results in a much simpler XDP program.

There's some nice potential follow on work also. If the hardware
provides a quality L4 hash that could be passed into the XDP program
to avoid having to call the helper. If we do invoke the helper it
would make sense to return the hash to the driver so that it can set
in the skbuff to avoid having the stack call skb_flow_dissect again.
We can also have an flow_diessct helper that invokes skb_flow_dissect
and returns the meta data based on input keys. This would be useful
for filtering in XDP etc.

Tom

> > Changes since RFC:
> > - accounted for vlans(David Ahern)
> > - return the correct hash by not using skb_get_hash(David Ahern)
> > - call __skb_flow_dissect in non-skb mode
> >