On Fri, Aug 23, 2019 at 3:27 AM Jesper Dangaard Brouer <brouer@xxxxxxxxxx> wrote: > > On Wed, 21 Aug 2019 13:30:09 -0700 > Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > > 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). > > For production use-cases, libbpf really need an easier higher-level API > for re-using pinned maps, for establishing shared maps between > programs. The existing libbpf API bpf_object__pin_maps() and > bpf_object__unpin_maps(), which don't re-use pinned maps, are not > really usable, because they pin/unpin ALL maps in the ELF file. > > What users really need is an easy way to specify, on a per map basis, > what kind of pinning and reuse/sharing they want. E.g. like iproute2 > have, "global", "object-scope", and "no-pinning". ("ifindex-scope" would > be nice for XDP). I totally agree and I think this is easy to add both for BTF-defined and "classic" bpf_map_def maps. Daniel mentioned in one of the previous threads that in practice object-scope doesn't seem to be used, so I'd say we should start with no-pinning + global pinning as two initial supported values for pinning attribute. ifindex-scope is interesting, but I'd love to hear a bit more about the use cases. > Today users have to split/reimplement bpf_prog_load_xattr(), and > use/add bpf_map__reuse_fd(). Which is that I ended doing for Honestly, bpf_prog_load_xattr() existence seems redundant to me. It's basically just bpf_object__open + bpf_object__load. There is a piece in the middle with "guessing" program types, but it should just be moved into bpf_object__open and happen by default. Using open + load gives more control and isn't really harder than bpf_prog_load_xattr. bpf_prog_load_xattr which might be slightly more convenient for simple use case, but falls apart immediately if you need to tune anything before load. > xdp-cpumap-tc[2] (used in production at ISP) resulting in 142 lines of > extra code[3] that should have been hidden inside libbpf. And worse, > in this solution[4] the maps for reuse-pinning is specified in the code > by name. Thus, they cannot use a generic loader. That I why, I want > to mark the maps via a pinning member, like iproute2. > > I really hope this moves in a practical direction, as I have the next > production request lined up (also from an ISP), and I hate to have to > advice them to choose the same route as [3]. It seems to me that map pinning doesn't need much discussion at this point, let's start with no-pinning + global pinning. To accommodate pinning at custom root, bpf_object__open_xattr should accept extra argument with non-default pinning root path. That should solve your case completely, shouldn't it? Ultimately, with BTF-defined maps it should be possible to specify custom pinning path on per-map basis for cases where user needs ultimate non-uniform manual control. > > > [2] https://github.com/xdp-project/xdp-cpumap-tc/ > [3] https://github.com/xdp-project/xdp-cpumap-tc/blob/master/src/xdp_iphash_to_cpu_user.c#L262-L403 > [4] https://github.com/xdp-project/xdp-cpumap-tc/blob/master/src/xdp_iphash_to_cpu_user.c#L431-L441 > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer
