-----Original Message-----
From: Magnus Karlsson <redacted>
Sent: 27 March, 2019 16:19
To: Björn Töpel <redacted>
Cc: Maxim Mikityanskiy <redacted>; Magnus Karlsson
[off-list ref]; Jonathan Lemon [off-list ref];
netdev@vger.kernel.org; Björn Töpel [off-list ref]; Daniel Borkmann
[off-list ref]; Eran Ben Elisha [off-list ref]; Tariq Toukan
[off-list ref]; Saeed Mahameed [off-list ref]
Subject: Re: New xdpsock sample

On Wed, Mar 27, 2019 at 3:10 PM Björn Töpel [off-list ref] wrote:

quoted

On Wed, 27 Mar 2019 at 14:34, Maxim Mikityanskiy [off-list ref]

wrote:

quoted

quoted

quoted

-----Original Message-----
From: Magnus Karlsson <redacted>
Sent: 26 March, 2019 18:24
To: Jonathan Lemon <redacted>
Cc: Maxim Mikityanskiy <redacted>; Magnus Karlsson
[off-list ref]; netdev@vger.kernel.org; Björn Töpel
[off-list ref]; Daniel Borkmann [off-list ref]; Eran

Ben

quoted

quoted

quoted

Elisha [off-list ref]; Tariq Toukan [off-list ref];

Saeed

quoted

quoted

quoted

Mahameed [off-list ref]
Subject: Re: New xdpsock sample

On Tue, Mar 26, 2019 at 5:13 PM Jonathan Lemon [off-list ref] wrote:

quoted

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.

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

--
Jonathan

On 26 Mar 2019, at 8:46, Maxim Mikityanskiy wrote:

quoted

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

quoted

quoted

quoted

quoted

quoted

with having the XDP program in a separate C file, just like

before,

quoted

quoted

quoted

quoted

quoted

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.

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.

quoted

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.

quoted

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

quoted

Thanks,
Björn

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

...and we already had one - the old xdpsock.

/Magnus

quoted

quoted

Thanks,
Max

quoted

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