Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: > On Wed, Aug 21, 2019 at 4:29 PM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >> >> Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes: >> >> > On Tue, Aug 20, 2019 at 01:47:01PM +0200, Toke Høiland-Jørgensen 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). >> >> - 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. >> > >> > It sounds like you're implying that existing libbpf format is not >> > uapi. >> >> No, that's not what I meant... See below. >> >> > It is and we cannot break it. >> > If patch 1 means breakage for existing pre-compiled .o that won't load >> > with new libbpf then we cannot use this method. >> > Recompiling .o with new libbpf definition of bpf_map_def isn't an option. >> > libbpf has to be smart before/after and recognize both old and iproute2 format. >> >> The libbpf.h definition of struct bpf_map_def is compatible with the one >> used in iproute2. In libbpf.h, the struct only contains five fields >> (type, key_size, value_size, max_entries and flags), and iproute2 adds >> another 4 (id, pinning, inner_id and inner_idx; these are the ones in >> patch 1 in this series). >> >> The issue I was alluding to above is that the bpf_helpers.h file in the >> kernel selftests directory *also* extends the bpf_map_def struct, and >> adds two *different* fields (inner_map_idx and numa_mode). The former is >> used to implement the same map-in-map definition functionality that >> iproute2 has, but with different semantics. The latter is additional to >> that, and I'm planning to add that to this series. >> >> Since bpf_helpers.h is *not* part of libbpf (yet), this will make it > > We should start considering it as if it was, so if we can avoid adding > stuff that I'd need to untangle to move it into libbpf, I'd rather > avoid it. > We've already prepared this move by relicensing bpf_helpers.h. Moving > it into libbpf itself is immediate next thing I'll do when I'm back. Yeah, I figured that with the relicensing, bpf_helpers would probably be making its way into libbpf soon. Which is why I wanted to start this discussion before that: If we do move bpf_helpers as-is, that will put us in the territory of full-on binary incompatibility. So the time to discuss doing this in a compatible way is now, before any such move is made. -Toke
