netdev - Re: [PATCH bpf-next 6/7] xsk: load a builtin XDP program on 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJ+HfNhdEvByi1GBiODwR6_sXX_Un3Y_Ad1cOMboZcSHWtWpVA@mail.gmail.com>
Date:   Mon, 10 Dec 2018 08:29:00 +0100
From:   Björn Töpel <bjorn.topel@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Magnus Karlsson <magnus.karlsson@...il.com>, ast@...nel.org,
        Daniel Borkmann <daniel@...earbox.net>,
        Netdev <netdev@...r.kernel.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        William Tu <u9012063@...il.com>,
        "Zhang, Qi Z" <qi.z.zhang@...el.com>
Subject: Re: [PATCH bpf-next 6/7] xsk: load a builtin XDP program on XDP_ATTACH

Den mån 10 dec. 2018 kl 03:17 skrev Jakub Kicinski
<jakub.kicinski@...ronome.com>:
>
> On Fri,  7 Dec 2018 12:44:30 +0100, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@...el.com>
> >
> > This commit extends the XDP_ATTACH bind option by loading a builtin
> > XDP program.
> >
> > The builtin program is the simplest program possible to redirect a
> > frame to an attached socket. In restricted C it would look like this:
> >
> >   SEC("xdp")
> >   int xdp_prog(struct xdp_md *ctx)
> >   {
> >       return bpf_xsk_redirect(ctx);
> >   }
> >
> > For many XDP socket users, this program would be the most common one.
> >
> > The builtin program loaded via XDP_ATTACH behaves, from an
> > install-to-netdev/uninstall-from-netdev point of view, different from
> > regular XDP programs. The easiest way to look at it is as a 2-level
> > hierarchy, where regular XDP programs has precedence over the builtin
> > one.
> >
> > If no regular XDP program is installed to the netdev, the builtin will
> > be install. If the builtin program is installed, and a regular is
> > installed, the regular XDP will have precedence over the builtin one.
> >
> > Further, if a regular program is installed, and later removed, the
> > builtin one will automatically be installed.
> >
> > The sxdp_flags field of struct sockaddr_xdp gets two new options
> > XDP_BUILTIN_SKB_MODE and XDP_BUILTIN_DRV_MODE, which maps to the
> > corresponding XDP netlink install flags.
> >
> > Signed-off-by: Björn Töpel <bjorn.topel@...el.com>
>
> Why not add this functionality to libbpf?  Libbpf would really benefit
> from better AF_XDP support, this would be a trivial part of it.
>

Hmm, maybe this would be a better intermediate step (given the
discussions on per-queue programs/builtins). I.e. the first 3 patches
just assigning an AF_XDP socket to a Rx queue, and the
bpf_xsk_redirect, and supply the builtin program as part of libbpf.

Regarding AF_XDP and libbpf -- we're planning to post the first AF_XDP
patches this week.

> Are we going to stop here or take the next logical step of not invoking
> the BPF program (and paying retpoline cost) at all if we know its the
> "built-in/just redirect to the xsk" program?  Because if the answer is
> "yes" we could just cut to the chase we can avoid all this unnecessary
> complexity.

A next step could be avoiding the retpoline by a builtin check in
bpf_prog_run_xdp (unless you're HW offloaded). However, we do not want
to bypass the BPF control-plane! The indirect call in bpf_prog_run_xdp
is really a performance killer. A batching bpf_prog_run_xdp version
would help some, but it wouldn't be as good as removing the call.

What I don't like about my series is having raw BPF-programs (flow of
instructions instructions) in the kernel. It's messy and hard to read.
Maybe it would be possible to teach the verifier to detect that the
program is "a trivial pass everything to a socket". :-)

As you say, making the XDP loading more complex does leave a bad taste
in the mouth.


Björn