On Wed, Oct 5, 2022 at 5:37 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Wed, Oct 05, 2022 at 11:09:32AM -0700, Andrii Nakryiko wrote: > > Historically enum bpf_func_id's BPF_FUNC_xxx enumerators relied on > > implicit sequential values being assigned by compiler. This is > > convenient, as new BPF helpers are always added at the very end, but it > > also has its downsides, some of them being: > > > > - with over 200 helpers now it's very hard to know what's each helper's ID, > > which is often important to know when working with BPF assembly (e.g., > > by dumping raw bpf assembly instructions with llvm-objdump -d > > command). it's possible to work around this by looking into vmlinux.h, > > dumping /sys/btf/kernel/vmlinux, looking at libbpf-provided > > bpf_helper_defs.h, etc. But it always feels like an unnecessary step > > and one should be able to quickly figure this out from UAPI header. > > > > - when backporting and cherry-picking only some BPF helpers onto older > > kernels it's important to be able to skip some enum values for helpers > > that weren't backported, but preserve absolute integer IDs to keep BPF > > helper IDs stable so that BPF programs stay portable across upstream > > and backported kernels. > > I like the macro hack, but it doesn't really address the above goal. Ok, cool, thanks for spotting these problems. I think we can fix them pretty easily. I'll send v2 with bpf_doc.py updates. > I tried: > - FN(map_delete_elem, 3, ##ctx) > > and got libbpf build error: > Exception: Helper bpf_map_delete_elem is missing from enum bpf_func_id > > Then I tried to delete the comment describing map_delete_elem and got: > Exception: Helper bpf_probe_read comment order (#3) must be aligned with its position (#4) in enum bpf_func_id > > So backporting process is a bit easier, but such uapi/bpf.h > is unasable to build libbpf against. > It's not a problem for github/libbpf, since it keeps bpf_helper_defs.h in the git, > but still. > Probably bpf_doc.py needs to get smarter. Correct. And I'd still advocate to use Github version of libbpf for building applications. But we do need to fix libbpf build in backported kernel anyways because libbpf is used during kernel build process itself. > > Maybe 2nd check in bpf_doc.py can be dropped? > Meaning that the comments can now be made out-of-order? > A good thing? > I think we should probably still enforce the order in upstream kernel through code-review. Yep, we should relax this check, I think. I'd still enforce that helpers should be documented in correct order, it's just that enum value != position in comment stream. I'll see how to adjust script to accommodate that. This will handle both latest upstream and backport case nicely. > > Also see cilium/ebpf/asm/func.go > The change to FN() will break that hacky script, but it's an ok trade-off. Yep, I think so too. And it should be very easy to fix up that script. I've cc'ed Timo for heads up.
