Message-ID: <CALDO+SZDCJ1fA=neuaDE7E5n6y55OBx88tKfZSjv0S2UKNRfOQ@mail.gmail.com>
Date:   Sat, 30 Mar 2019 08:24:56 -0700
From:   William Tu <u9012063@...il.com>
To:     Maxim Mikityanskiy <maximmi@...lanox.com>
Cc:     Björn Töpel <bjorn.topel@...il.com>,
        Magnus Karlsson <magnus.karlsson@...il.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn.topel@...el.com>,
        Jonathan Lemon <bsd@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>
Subject: Re: New xdpsock sample

On Fri, Mar 29, 2019 at 1:16 AM Maxim Mikityanskiy <maximmi@...lanox.com> wrote:
>
> > -----Original Message-----
> > From: Björn Töpel <bjorn.topel@...il.com>
> > Sent: 28 March, 2019 14:35
> > To: Maxim Mikityanskiy <maximmi@...lanox.com>
> > Cc: Magnus Karlsson <magnus.karlsson@...il.com>; Magnus Karlsson
> > <magnus.karlsson@...el.com>; Björn Töpel <bjorn.topel@...el.com>; Jonathan
> > Lemon <bsd@...com>; netdev@...r.kernel.org; Daniel Borkmann
> > <daniel@...earbox.net>; Eran Ben Elisha <eranbe@...lanox.com>; Tariq Toukan
> > <tariqt@...lanox.com>; Saeed Mahameed <saeedm@...lanox.com>
> > Subject: Re: New xdpsock sample
> >
> > On Thu, 28 Mar 2019 at 10:48, Maxim Mikityanskiy <maximmi@...lanox.com>
> > wrote:
> > >
> > [...]
> > > > > Thanks for the good input, Max! The rationale for making the sample
> > > > > simpler, was that most people was just C&Ping from it and used it in
> > > > > their own code, so we aimed for a simple "fits-most-people" sample.
> > >
> > > I don't think it's easier for them to use binaries in their own code
> > > than proper sources. Having the XDP program built from sources in libbpf
> > > doesn't complicate the sample in any way, though.
> > >
> >
> > Correct, but then Clang would be a libbpf build dependency.
>
> OK, that's the first argument that makes sense for me. You're right, now
> libbpf doesn't depend on clang. However, I think pretty much every
> application using libbpf also needs clang to build BPF programs, so
> adding a clang dependency to libbpf itself shouldn't change anything.
> And we already have the clang dependency in samples/bpf. Or do you have
> an example of an application where libbpf is used, but clang is not?
>
We are adding AF_XDP support to OVS and we'd like to use libbpf, but
not clang/llvm.
The reason is that we don't have much logic in the XDP program, simply
we want to have a high speed packet I/O from driver to userspace.
Then the rest of
processing is in userspace. In this case, I don't want to add clang/llvm into
our build process just to compile a couple lines of eBPF code to do AF_XDP.
Or even in the future, when we put more logic in eBPF/XDP program, I'd
still like
it to be eBPF binary, not C code. After all, the chance that OVS users
understand
eBPF and want to modify it is very low.

The current version of xdpsock_user + libbpf meets our requirement perfectly.
I started with the old xdpsock_user.c and xdpsock_kern.c, and ported to OVS.
I have to add clang/llvm support, copy/paste lots of code, and manage eBPF maps
by myself.  Switching to use libbpf makes the porting much easier and simpler.
For an AF_XDP user, the current approach let them focus on the AF_XDP's API,
the umem and rx/tx/fill/compl queue, without worrying about the
eBPF/XDP program.

On the other hand, I do agree that having xdpsock_kern.c has its
benefit.  It makes better
understanding of how the underlying XDP program forwards packets to
AF_XDP sockets.

Regards,
William

> Another option is to make XSK support optional in libbpf, then the clang
> dependency will be optional.
>
> Another (crazy) option is to make a separate libxsk that would depend on
> libbpf and clang and contain only the XSK code.
>
> > Maybe
> > that's ok? If that is added, I'd be happy do remove the raw BPF
> > instructions.
>
> OK for me, for the reasons explained above. I don't know who else should
> approve it.
>
> > Personally, I don't have a hard time reading a couple of
> > lines of assembly
>
> Neither have I. But it doesn't mean we should read and write programs in
> assembly. I'm perfectly able to understand and modify this XDP program,
> but it's just not the way the programs should be written. I don't need
> to explain why C is better here - you understand it yourself :)
>
> > but I see your point and agree. Having it as C-code
> > would be better for the libbpf developers -- if the Clang build
> > dependency is ok.
> >
> > > > > Let's make an "advanced user" sample as well, and add shared umem
> > > > > support to libbpf!
> > >
> > > Why create another sample if we have this one? Actually, how making
> > > another sample fixes this one? The issue is in libbpf anyway. It has a
> > > blob inside with no tools to regenerate it from C sources. And this lib
> > > is not even a sample, it can be used by real applications. Of course, it
> > > should be editable, otherwise no new feature can be added (without
> > > manually writing bytecode), and it's not the matter of shared UMEM.
> > >
> >
> > Magnus and I took the route to simplify the sample, to make it easier
> > for new users. I still think that was the right path. Should there be
> > a sample showcasing all the knobs/pulleys? Sure.
>
> As Jonathan points, libbpf dictates the layout of xsks_map, so we won't
> be able to provide an advanced example with shared UMEM unless we modify
> libbpf as well.
>
> > > > > ...and as always, patches are very much welcome!
> > >
> > > Good approach - to drop a feature and wait until someone submits a patch
> > > to restore it. And how do you imagine that patch that adds back shared
> > > UMEM? The blob in libbpf has to be edited to accomplish this. It makes
> > > unnecessary trouble for anyone trying to contribute.
> > >
> >
> > I appreciate that you are looking into the code/design with
> > constructive remarks -- but please refrain from being snarky.
>
> I'm sorry if it sounded snarky, I just wanted to convey my point. I
> agree with moving the common AF_XDP code into a lib to make writing new
> applications easier and to avoid repeating the same code. I don't think
> it made the sample simpler though - there are more abstraction layers
> now, and it became more generic. And, of course, we lost an important
> piece of functionality. So, regarding the issue with the samples, I
> think it would be good to have both the old and the new one
> simultaneously. Together they would cover all needs: the old one
> illustrates how to use AF_XDP and is good as sample code for newcomers,
> because it's straightforward and not universal, and the new one
> illustrates how to use libbpf to build the real applications, using the
> generic abstractions libbpf provides. (It doesn't resolve the libbpf
> issue with bytecode, but it's just my opinion on the samples).
>
> >
> > Björn

Powered by blists - more mailing lists