Message-ID: <CAJ8uoz2XOnamDB5t2RDefhnfmRYQVLkLRf6H3k5oyartHsjWOw@mail.gmail.com>
Date:   Wed, 27 Mar 2019 15:18:57 +0100
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     Maxim Mikityanskiy <maximmi@...lanox.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Jonathan Lemon <bsd@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        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 Wed, Mar 27, 2019 at 3:10 PM Björn Töpel <bjorn.topel@...il.com> wrote:
>
> On Wed, 27 Mar 2019 at 14:34, Maxim Mikityanskiy <maximmi@...lanox.com> wrote:
> >
> > > -----Original Message-----
> > > From: Magnus Karlsson <magnus.karlsson@...il.com>
> > > Sent: 26 March, 2019 18:24
> > > To: Jonathan Lemon <bsd@...com>
> > > Cc: Maxim Mikityanskiy <maximmi@...lanox.com>; Magnus Karlsson
> > > <magnus.karlsson@...el.com>; netdev@...r.kernel.org; Björn Töpel
> > > <bjorn.topel@...el.com>; 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 Tue, Mar 26, 2019 at 5:13 PM Jonathan Lemon <bsd@...com> wrote:
> > > >
> > > > The rationale (IIRC) was that it would be easier for new users to
> > > > get started using AF_XDP by providing everything that was needed
> > > > by default.
> >
> > Well, no matter whether the XDP program is compiled separately or
> > hardcoded as bytecode, it's libbpf's implementation details, and a new
> > user shouldn't notice any difference in usage.
> >
> > However, when the user is no longer new and is not satisfied with the
> > sample application, they should be able to tweak it. If the sample is
> > not modifiable, the user is forced to rewrite all the code. The
> > threshold of entry is low, but then you have to jump a huge step to
> > start doing something not included into the sample. It doesn't make
> > sense to me when there is an option to have a modifiable sample without
> > increasing the threshold of entry which makes further fiddling with the
> > sample easier.
> >
> > > > Passing in XSK_LIBBPF_FLAGS__INHIBIT_PROG to the library will
> > > > bypass loading the sample program, so a user application may still
> > > > use the library with their own bpf program.
> >
> > Yes, thanks, but it's not what I want, see below.
> >
> > > > I'll admit that the change likely makes it harder to simply modify
> > > > the sample program for other uses, but that's not really the point
> > > > of the samples.
> >
> > I'm not trying to adapt the sample to transform it to some real world
> > application. But the ability to tinker with sample code is vital to get
> > understanding on how the feature works. This is the point of samples.
> >
> > > > --
> > > > Jonathan
> > > >
> > > > On 26 Mar 2019, at 8:46, Maxim Mikityanskiy wrote:
> > > >
> > > > > Hi Magnus and all,
> > > > >
> > > > > https://patchwork.ozlabs.org/cover/1045921/
> > > > >
> > > > > This series removes xdpsock_kern.c and replaces it by the bytecode
> > > > > hardcoded in libbpf. I am wondering whether there is some real issue
> > > > > with having the XDP program in a separate C file, just like before,
> > > > > because this change made it far less convenient to modify the XDP
> > > > > program. Could you give any comments?
> > > > >
> > > > > Thanks,
> > > > > Max
> > >
> > > How about we reintroduce a sample C XDP program once we have a reason
> > > to use one in the xdpsock program, i.e. for something not covered by
> > > libbpf? I do not have such a use case at the moment, but do you Max?
> >
> > Even at the moment the XDP program hardcoded into libbpf doesn't support
> > shared UMEMs that used to be supported in the old xdpsock. If this
> > feature is added at some point, it will require modifying both the XDP
> > program and libbpf. It's an obvious example of a thing not covered by
> > libbpf.
> >
> > There are also two reasons to ship the C code of the XDP program:
> >
> > 1. First of all, it's a sample. When someone starts looking at it, they
> > may want to make some modifications to understand it better. It may not
> > be enough to just look at the comment above.
> >
> > 2. The AF_XDP feature is evolving. Some new things may appear worth
> > showing in the sample. I want to highlight that I'm not talking about
> > the case when someone takes xdpsock+libbpf and tries to fit it to their
> > needs. It's all about putting the reference implementation of new AF_XDP
> > features to the sample. These features may require modification of both
> > libbpf and the XDP program.
> >
> > In any case, the repository should contain source code and tools to
> > build it, not binaries. BPF bytecode is not the source code, unless it
> > was written manually, but the C code in the comment above proves the
> > opposite. Everyone should be able to modify the code and to rebuild it.
> > I pointed out three real cases (showing the reference implementation of
> > shared UMEMs, fiddling with the sample while learning it, adding future
> > features) when modification of the code is necessary, and other people
> > may have their own motivations to modify the code.
> >
>
> 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.
>
> Let's make an "advanced user" sample as well, and add shared umem
> support to libbpf! ...and as always, patches are very much welcome!
>
>
> Thanks,
> Björn

+1 for a shared umem sample app that uses a bpf program in C. Would be
very useful.

/Magnus

> > Thanks,
> > Max
> >
> > > If so, as you say, it would be good to have an example on how to
> > > accomplish this using the XSK_LIBBPF_FLAGS__INHIBIT_PROG that Jonathan
> > > mentioned.
> > >
> > > /Magnus

Powered by blists - more mailing lists