On Tue, Aug 20, 2019 at 4:47 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > iproute2 uses its own bpf loader to load eBPF programs, which has > evolved separately from libbpf. Since we are now standardising on > libbpf, this becomes a problem as iproute2 is slowly accumulating > feature incompatibilities with libbpf-based loaders. In particular, > iproute2 has its own (expanded) version of the map definition struct, > which makes it difficult to write programs that can be loaded with both > custom loaders and iproute2. > > This series seeks to address this by converting iproute2 to using libbpf > for all its bpf needs. This version is an early proof-of-concept RFC, to > get some feedback on whether people think this is the right direction. > > What this series does is the following: > > - Updates the libbpf map definition struct to match that of iproute2 > (patch 1). Hi Toke, Thanks for taking a stab at unifying libbpf and iproute2 loaders. I'm totally in support of making iproute2 use libbpf to load/initialize BPF programs. But I'm against adding iproute2-specific fields to libbpf's bpf_map_def definitions to support this. I've proposed the plan of extending libbpf's supported features so that it can be used to load iproute2-style BPF programs earlier, please see discussions in [0] and [1]. I think instead of emulating iproute2 way of matching everything based on user-specified internal IDs, which doesn't provide good user experience and is quite easy to get wrong, we should support same scenarios with better declarative syntax and in a less error-prone way. I believe we can do that by relying on BTF more heavily (again, please check some of my proposals in [0], [1], and discussion with Daniel in those threads). It will feel more natural and be more straightforward to follow. It would be great if you can lend a hand in implementing pieces of that plan! I'm currently on vacation, so my availability is very sparse, but I'd be happy to discuss this further, if need be. [0] https://lore.kernel.org/bpf/CAEf4BzbfdG2ub7gCi0OYqBrUoChVHWsmOntWAkJt47=FE+km+A@xxxxxxxxxxxxxx/ [1] https://www.spinics.net/lists/bpf/msg03976.html > - Adds functionality to libbpf to support automatic pinning of maps when > loading an eBPF program, while re-using pinned maps if they already > exist (patches 2-3). > - Modifies iproute2 to make it possible to compile it against libbpf > without affecting any existing functionality (patch 4). > - Changes the iproute2 eBPF loader to use libbpf for loading XDP > programs (patch 5). > > > As this is an early PoC, there are still a few missing pieces before > this can be merged. Including (but probably not limited to): > > - Consolidate the map definition struct in the bpf_helpers.h file in the > kernel tree. This contains a different, and incompatible, update to > the struct. Since the iproute2 version has actually been released for > use outside the kernel tree (and thus is subject to API stability > constraints), I think it makes the most sense to keep that, and port > the selftests to use it. > > - The iproute2 loader supports automatically populating map-in-map > definitions on load. This needs to be added to libbpf as well. There > is an implementation of this in the selftests in the kernel tree, > which will have to be ported (related to the previous point). > > - The iproute2 port needs to be completed; this means at least > supporting TC eBPF programs as well, figuring out how to deal with > cBPF programs, and getting the verbose output back to the same state > as before the port. Also, I guess the iproute2 maintainers need to ACK > that they are good with adding a dependency on libbpf. > > - Some of the code added to libbpf in patch 2 in this series include > code derived from iproute2, which is GPLv2+. So it will need to be > re-licensed to be usable in libbpf. Since `git blame` indicated that > the original code was written by Daniel, I figure he can ACK that > relicensing before applying the patches :) > > > Please take a look at this series and let me know if you agree > that this is the right direction to go. Assuming there's consensus that > it is, I'll focus on getting the rest of the libbpf patches ready for > merging. I'll send those as a separate series, and hold off on the > iproute2 patches until they are merged; but for this version I'm > including both in one series so it's easier to see the context. > > -Toke > > > libbpf: > Toke Høiland-Jørgensen (3): > libbpf: Add map definition struct fields from iproute2 > libbpf: Add support for auto-pinning of maps with reuse on program > load > libbpf: Add support for specifying map pinning path via callback > > tools/lib/bpf/libbpf.c | 205 +++++++++++++++++++++++++++++++++++++++-- > tools/lib/bpf/libbpf.h | 18 ++++ > 2 files changed, 214 insertions(+), 9 deletions(-) > > iproute2: > Toke Høiland-Jørgensen (2): > iproute2: Allow compiling against libbpf > iproute2: Support loading XDP programs with libbpf > > configure | 16 +++++ > include/bpf_util.h | 6 +- > ip/ipvrf.c | 4 +- > lib/bpf.c | 148 ++++++++++++++++++++++++++++++++++++--------- > 4 files changed, 142 insertions(+), 32 deletions(-) > > > -- > 2.22.1 >
