On Wed, Mar 27, 2019 at 3:10 PM Björn Töpel <bjorn.to...@gmail.com> wrote: > > On Wed, 27 Mar 2019 at 14:34, Maxim Mikityanskiy <maxi...@mellanox.com> wrote: > > > > > -----Original Message----- > > > From: Magnus Karlsson <magnus.karls...@gmail.com> > > > Sent: 26 March, 2019 18:24 > > > To: Jonathan Lemon <b...@fb.com> > > > Cc: Maxim Mikityanskiy <maxi...@mellanox.com>; Magnus Karlsson > > > <magnus.karls...@intel.com>; netdev@vger.kernel.org; Björn Töpel > > > <bjorn.to...@intel.com>; Daniel Borkmann <dan...@iogearbox.net>; Eran Ben > > > Elisha <era...@mellanox.com>; Tariq Toukan <tar...@mellanox.com>; Saeed > > > Mahameed <sae...@mellanox.com> > > > Subject: Re: New xdpsock sample > > > > > > On Tue, Mar 26, 2019 at 5:13 PM Jonathan Lemon <b...@fb.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
