Commit 0a0d55ef authored by Eyal Birger's avatar Eyal Birger Committed by Andrii Nakryiko

bpf/scripts: Assert helper enum value is aligned with comment order

The helper value is ABI as defined by enum bpf_func_id.
As bpf_helper_defs.h is used for the userpace part, it must be consistent
with this enum.

Before this change the comments order was used by the bpf_doc script in
order to set the helper values defined in the helpers file.

When adding new helpers it is very puzzling when the userspace application
breaks in weird places if the comment is inserted instead of appended -
because the generated helper ABI is incorrect and shifted.

This commit sets the helper value to the enum value.

In addition it is currently the practice to have the comments appended
and kept in the same order as the enum. As such, add an assertion
validating the comment order is consistent with enum value.

In case a different comments ordering is desired, this assertion can
be lifted.
Signed-off-by: default avatarEyal Birger <eyal.birger@gmail.com>
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Reviewed-by: default avatarQuentin Monnet <quentin@isovalent.com>
Link: https://lore.kernel.org/bpf/20220824181043.1601429-1-eyal.birger@gmail.com
parent 7184aef9
...@@ -50,6 +50,10 @@ class Helper(APIElement): ...@@ -50,6 +50,10 @@ class Helper(APIElement):
@desc: textual description of the helper function @desc: textual description of the helper function
@ret: description of the return value of the helper function @ret: description of the return value of the helper function
""" """
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.enum_val = None
def proto_break_down(self): def proto_break_down(self):
""" """
Break down helper function protocol into smaller chunks: return type, Break down helper function protocol into smaller chunks: return type,
...@@ -92,6 +96,7 @@ class HeaderParser(object): ...@@ -92,6 +96,7 @@ class HeaderParser(object):
self.commands = [] self.commands = []
self.desc_unique_helpers = set() self.desc_unique_helpers = set()
self.define_unique_helpers = [] self.define_unique_helpers = []
self.helper_enum_vals = {}
self.desc_syscalls = [] self.desc_syscalls = []
self.enum_syscalls = [] self.enum_syscalls = []
...@@ -248,30 +253,54 @@ class HeaderParser(object): ...@@ -248,30 +253,54 @@ class HeaderParser(object):
break break
def parse_define_helpers(self): def parse_define_helpers(self):
# Parse the number of FN(...) in #define __BPF_FUNC_MAPPER to compare # Parse FN(...) in #define __BPF_FUNC_MAPPER to compare later with the
# later with the number of unique function names present in description. # number of unique function names present in description and use the
# correct enumeration value.
# Note: seek_to(..) discards the first line below the target search text, # Note: seek_to(..) discards the first line below the target search text,
# resulting in FN(unspec) being skipped and not added to self.define_unique_helpers. # resulting in FN(unspec) being skipped and not added to self.define_unique_helpers.
self.seek_to('#define __BPF_FUNC_MAPPER(FN)', self.seek_to('#define __BPF_FUNC_MAPPER(FN)',
'Could not find start of eBPF helper definition list') 'Could not find start of eBPF helper definition list')
# Searches for either one or more FN(\w+) defines or a backslash for newline # Searches for one FN(\w+) define or a backslash for newline
p = re.compile('\s*(FN\(\w+\))+|\\\\') p = re.compile('\s*FN\((\w+)\)|\\\\')
fn_defines_str = '' fn_defines_str = ''
i = 1 # 'unspec' is skipped as mentioned above
while True: while True:
capture = p.match(self.line) capture = p.match(self.line)
if capture: if capture:
fn_defines_str += self.line fn_defines_str += self.line
self.helper_enum_vals[capture.expand(r'bpf_\1')] = i
i += 1
else: else:
break break
self.line = self.reader.readline() self.line = self.reader.readline()
# Find the number of occurences of FN(\w+) # Find the number of occurences of FN(\w+)
self.define_unique_helpers = re.findall('FN\(\w+\)', fn_defines_str) self.define_unique_helpers = re.findall('FN\(\w+\)', fn_defines_str)
def assign_helper_values(self):
seen_helpers = set()
for helper in self.helpers:
proto = helper.proto_break_down()
name = proto['name']
try:
enum_val = self.helper_enum_vals[name]
except KeyError:
raise Exception("Helper %s is missing from enum bpf_func_id" % name)
# Enforce current practice of having the descriptions ordered
# by enum value.
seen_helpers.add(name)
desc_val = len(seen_helpers)
if desc_val != enum_val:
raise Exception("Helper %s comment order (#%d) must be aligned with its position (#%d) in enum bpf_func_id" % (name, desc_val, enum_val))
helper.enum_val = enum_val
def run(self): def run(self):
self.parse_desc_syscall() self.parse_desc_syscall()
self.parse_enum_syscall() self.parse_enum_syscall()
self.parse_desc_helpers() self.parse_desc_helpers()
self.parse_define_helpers() self.parse_define_helpers()
self.assign_helper_values()
self.reader.close() self.reader.close()
############################################################################### ###############################################################################
...@@ -796,7 +825,7 @@ class PrinterHelpers(Printer): ...@@ -796,7 +825,7 @@ class PrinterHelpers(Printer):
comma = ', ' comma = ', '
print(one_arg, end='') print(one_arg, end='')
print(') = (void *) %d;' % len(self.seen_helpers)) print(') = (void *) %d;' % helper.enum_val)
print('') print('')
############################################################################### ###############################################################################
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment