Date:   Wed, 27 Mar 2019 13:32:59 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     Magnus Karlsson <magnus.karlsson@...il.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Jonathan Lemon <bsd@...com>
CC:     "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

> -----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,
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