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. While neither problem is insurmountable, they come up frequently enough and are annoying enough to warrant improving the situation. And for the backporting the problem can easily go unnoticed for a while, especially if backport is done with people not very familiar with BPF subsystem overall. Anyways, it's easy to fix this by making sure that __BPF_FUNC_MAPPER macro provides explicit helper IDs. Unfortunately that would potentially break existing users that use UAPI-exposed __BPF_FUNC_MAPPER and are expected to pass macro that accepts only symbolic helper identifier (e.g., map_lookup_elem for bpf_map_lookup_elem() helper). As such, we need to introduce a new macro (___BPF_FUNC_MAPPER) which would specify both identifier and integer ID, but in such a way as to allow existing __BPF_FUNC_MAPPER be expressed in terms of new ___BPF_FUNC_MAPPER macro. And that's what this patch is doing. To avoid duplication and allow __BPF_FUNC_MAPPER stay *exactly* the same, ___BPF_FUNC_MAPPER accepts arbitrary "context" arguments, which can be used to pass any extra macros, arguments, and whatnot. In our case we use this to pass original user-provided macro that expects single argument and __BPF_FUNC_MAPPER is using it's own three-argument __BPF_FUNC_MAPPER_APPLY intermediate macro to impedance-match new and old "callback" macros. Once we resolve this, we use new ___BPF_FUNC_MAPPER to define enum bpf_func_id with explicit values. The other users of __BPF_FUNC_MAPPER in kernel (namely in kernel/bpf/disasm.c) are kept exactly the same both as demonstration that backwards compat works, but also to avoid unnecessary code churn. Note that new ___BPF_FUNC_MAPPER() doesn't forcefully insert comma between values, as that might not be appropriate in all possible cases where ___BPF_FUNC_MAPPER might be used by users. This doesn't reduce usability, as it's trivial to insert that comma inside "callback" macro. To validate all the manually specified IDs are exactly right, we used BTF to compare before and after values: $ bpftool btf dump file ~/linux-build/default/vmlinux | rg bpf_func_id -A 211 > after.txt $ git stash # stach UAPI changes $ make -j90 ... re-building kernel without UAPI changes ... $ bpftool btf dump file ~/linux-build/default/vmlinux | rg bpf_func_id -A 211 > before.txt $ diff -u before.txt after.txt --- before.txt 2022-10-05 10:48:18.119195916 -0700 +++ after.txt 2022-10-05 10:46:49.446615025 -0700 @@ -1,4 +1,4 @@ -[14576] ENUM 'bpf_func_id' encoding=UNSIGNED size=4 vlen=211 +[9560] ENUM 'bpf_func_id' encoding=UNSIGNED size=4 vlen=211 'BPF_FUNC_unspec' val=0 'BPF_FUNC_map_lookup_elem' val=1 'BPF_FUNC_map_update_elem' val=2 As can be seen from diff above, the only thing that changed was resulting BTF type ID of ENUM bpf_func_id, not any of the enumerators, their names or integer values. The only other place that needed fixing was scripts/bpf_doc.py used to generate man pages and bpf_helper_defs.h header for libbpf and selftests. That script is tightly-coupled to exact shape of ___BPF_FUNC_MAPPER macro definition, so had to be trivially adapted.
Showing the changes to scripts (1 of 3 files). Show all
Unified · Split
| @@ -253,28 +253,27 @@ class HeaderParser(object): · 252 unmodified lines | |||
| 253 | break | 253 | break |
| 254 | 254 | ||
| 255 | def parse_define_helpers(self): | 255 | def parse_define_helpers(self): |
| 256 | # Parse FN(...) in #define __BPF_FUNC_MAPPER to compare later with the | 256 | # Parse FN(...) in #define ___BPF_FUNC_MAPPER to compare later with the |
| 257 | # number of unique function names present in description and use the | 257 | # number of unique function names present in description and use the |
| 258 | # correct enumeration value. | 258 | # correct enumeration value. |
| 259 | # Note: seek_to(..) discards the first line below the target search text, | 259 | # Note: seek_to(..) discards the first line below the target search text, |
| 260 | # resulting in FN(unspec) being skipped and not added to self.define_unique_helpers. | 260 | # resulting in FN(unspec, 0, ##ctx) being skipped and not added to |
| 261 | self.seek_to('#define __BPF_FUNC_MAPPER(FN)', | 261 | # self.define_unique_helpers. |
| 262 | self.seek_to('#define ___BPF_FUNC_MAPPER(FN, ctx...)', | ||
| 262 | 'Could not find start of eBPF helper definition list') | 263 | 'Could not find start of eBPF helper definition list') |
| 263 | # Searches for one FN(\w+) define or a backslash for newline | 264 | # Searches for one FN(\w+) define or a backslash for newline |
| 264 | p = re.compile('\s*FN\((\w+)\)|\\\\') | 265 | p = re.compile('\s*FN\((\w+), (\d+), ##ctx\)|\\\\') |
| 265 | fn_defines_str = '' | 266 | fn_defines_str = '' |
| 266 | i = 1 # 'unspec' is skipped as mentioned above | ||
| 267 | while True: | 267 | while True: |
| 268 | capture = p.match(self.line) | 268 | capture = p.match(self.line) |
| 269 | if capture: | 269 | if capture: |
| 270 | fn_defines_str += self.line | 270 | fn_defines_str += self.line |
| 271 | self.helper_enum_vals[capture.expand(r'bpf_\1')] = i | 271 | self.helper_enum_vals[capture.expand(r'bpf_\1')] = int(capture[2]) |
| 272 | i += 1 | ||
| 273 | else: | 272 | else: |
| 274 | break | 273 | break |
| 275 | self.line = self.reader.readline() | 274 | self.line = self.reader.readline() |
| 276 | # Find the number of occurences of FN(\w+) | 275 | # Find the number of occurences of FN(\w+) |
| 277 | self.define_unique_helpers = re.findall('FN\(\w+\)', fn_defines_str) | 276 | self.define_unique_helpers = re.findall('FN\(\w+, \d+, ##ctx\)', fn_defines_str) |
| 278 | 277 | ||
| 279 | def assign_helper_values(self): | 278 | def assign_helper_values(self): |
| 280 | seen_helpers = set() | 279 | seen_helpers = set() |
| @@ -423,7 +422,7 @@ class PrinterHelpersRST(PrinterRST): · 142 unmodified lines | |||
| 423 | """ | 422 | """ |
| 424 | def __init__(self, parser): | 423 | def __init__(self, parser): |
| 425 | self.elements = parser.helpers | 424 | self.elements = parser.helpers |
| 426 | self.elem_number_check(parser.desc_unique_helpers, parser.define_unique_helpers, 'helper', '__BPF_FUNC_MAPPER') | 425 | self.elem_number_check(parser.desc_unique_helpers, parser.define_unique_helpers, 'helper', '___BPF_FUNC_MAPPER') |
| 427 | 426 | ||
| 428 | def print_header(self): | 427 | def print_header(self): |
| 429 | header = '''\ | 428 | header = '''\ |
| @@ -636,7 +635,7 @@ class PrinterHelpers(Printer): · 206 unmodified lines | |||
| 636 | """ | 635 | """ |
| 637 | def __init__(self, parser): | 636 | def __init__(self, parser): |
| 638 | self.elements = parser.helpers | 637 | self.elements = parser.helpers |
| 639 | self.elem_number_check(parser.desc_unique_helpers, parser.define_unique_helpers, 'helper', '__BPF_FUNC_MAPPER') | 638 | self.elem_number_check(parser.desc_unique_helpers, parser.define_unique_helpers, 'helper', '___BPF_FUNC_MAPPER') |
| 640 | 639 | ||
| 641 | type_fwds = [ | 640 | type_fwds = [ |
| 642 | 'struct bpf_fib_lookup', | 641 | 'struct bpf_fib_lookup', |