Date:   Mon, 10 Dec 2018 15:01:16 +0100
From:   Björn Töpel <bjorn.topel@...il.com>
To:     "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>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     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 0/7] Add XDP_ATTACH bind() flag to AF_XDP sockets

Den fre 7 dec. 2018 kl 12:44 skrev Björn Töpel <bjorn.topel@...il.com>:
>
> From: Björn Töpel <bjorn.topel@...el.com>
>
> Hi!
>
> This patch set adds support for a new XDP socket bind option,
> XDP_ATTACH.
>
> The rationale behind attach is performance and ease of use. Many XDP
> socket users just need a simple way of creating/binding a socket and
> receiving frames right away without loading an XDP program.
>
> XDP_ATTACH adds a mechanism we call "builtin XDP program" that simply
> is a kernel provided XDP program that is installed to the netdev when
> XDP_ATTACH is being passed as a bind() flag.
>
> 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);
>   }
>
> The builtin program loaded via XDP_ATTACH behaves, from an
> install-to-netdev/uninstall-from-netdev point of view, differently
> 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, regular XDP program 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.
>
> The builtin XDP program functionally adds even more complexity to the
> already hard to read dev_change_xdp_fd. Maybe it would be simpler to
> store the program in the struct net_device together with the install
> flags instead of calling the ndo_bpf multiple times?
>
> The outline of the series is as following:
>   patch 1-2: Introduce the first part of XDP_ATTACH, simply adding
>              the socket to the netdev structure.
>   patch 3:   Add a new BPF function, bpf_xsk_redirect, that
>              redirects a frame to an attached socket.
>   patch 4-5: Preparatory commits for built in BPF programs
>   patch 6:   Make XDP_ATTACH load a builtin XDP program
>   patch 7:   Extend the samples application with XDP_ATTACH
>              support
>
> Patch 1 through 3 gives the performance boost and make it possible to
> use AF_XDP sockets without an XSKMAP, but still requires an explicit
> XDP program to be loaded.
>
> Patch 4 through 6 make it possible to use XDP socket without explictly
> loading an XDP program.
>
> The performance numbers for rxdrop (Intel(R) Xeon(R) Gold 6154 CPU @
> 3.00GHz):
>
> XDP_SKB:
> XSKMAP:     2.8 Mpps
> XDP_ATTACH: 2.9 Mpps
>
> XDP_DRV - copy:
> XSKMAP:     8.5 Mpps
> XDP_ATTACH: 9.3 Mpps
>
> XDP_DRV - zero-copy:
> XSKMAP:     15.1 Mpps
> XDP_ATTACH: 17.3 Mpps
>
> Thanks!
> Björn
>

Firstly, thanks for all the feedback on the series.

I will do a respin of the series, but without the builtin
functionality, IOW only the XDP_ATTACH bind() option to assign an XDP
socket to a netdev Rx queue, and the corresponding bpf_xsk_redirect
(patch 1 to 3). This will give a perfomance boost, and also requires a
less complicated XDP program.

The builtin/sticky XDP program has too many down-sides:

* Explicit BPF instructions in the kernel.
* Composability: mix regular BPF programs with builtin ones.
* The sticky XDP program is not intuitive for a user, e.g. "ip link
  dev eth0 xdp off" will not disable the sticky one
* More complex XDP install code.

We'll this route instead: Introduce XDP_ATTACH/bpf_xsk_redirect (as
above) and let libbpf load the trivial AF_XDP program.


Björn

>
> Björn Töpel (7):
>   xsk: simplify AF_XDP socket teardown
>   xsk: add XDP_ATTACH bind() flag
>   bpf: add bpf_xsk_redirect function
>   bpf: prepare for builtin bpf program
>   bpf: add function to load builtin BPF program
>   xsk: load a builtin XDP program on XDP_ATTACH
>   samples: bpf: add support for XDP_ATTACH to xdpsock
>
>  include/linux/bpf.h         |   2 +
>  include/linux/filter.h      |   4 +
>  include/linux/netdevice.h   |  11 +++
>  include/net/xdp_sock.h      |   2 +
>  include/trace/events/xdp.h  |  61 +++++++++++++++
>  include/uapi/linux/bpf.h    |  14 +++-
>  include/uapi/linux/if_xdp.h |   9 ++-
>  kernel/bpf/syscall.c        |  91 ++++++++++++++--------
>  net/core/dev.c              |  84 +++++++++++++++++++--
>  net/core/filter.c           | 100 ++++++++++++++++++++++++
>  net/xdp/xsk.c               | 146 +++++++++++++++++++++++++++++-------
>  samples/bpf/xdpsock_user.c  | 108 ++++++++++++++++----------
>  12 files changed, 524 insertions(+), 108 deletions(-)
>
> --
> 2.19.1
>

Powered by blists - more mailing lists