On Mon, Jan 10, 2022 at 6:31 AM Usama Arif <usama.arif@xxxxxxxxxxxxx> wrote:
>
> Currently bpf_helper_defs.h is auto-generated using function documentation
> present in bpf.h. If the documentation for the helper is missing
> or doesn't follow a specific format for e.g. if a function is documented
> as:
> * long bpf_kallsyms_lookup_name( const char *name, int name_sz, int flags, u64 *res )
> instead of
> * long bpf_kallsyms_lookup_name(const char *name, int name_sz, int flags, u64 *res)
> (notice the extra space at the start and end of function arguments)
> then that helper is not dumped in the auto-generated header and results in
> an invalid call during eBPF runtime, even if all the code specific to the
> helper is correct.
>
> This patch checks the number of functions documented within the header file
> with those present as part of #define __BPF_FUNC_MAPPER and generates a
> warning in the header file if they don't match. It is not needed with the
Shall we fail instead of warning?
> currently documented upstream functions, but can help in debugging
> when developing new helpers when there might be missing or misformatted
> documentation.
>
> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxxxxxx>
>
> ---
> v1->v2:
> - Fix CI error reported by Alexei Starovoitov
> ---
> scripts/bpf_doc.py | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
> index a6403ddf5de7..e426d2a727cb 100755
> --- a/scripts/bpf_doc.py
> +++ b/scripts/bpf_doc.py
> @@ -87,6 +87,8 @@ class HeaderParser(object):
> self.line = ''
> self.helpers = []
> self.commands = []
> + self.desc_unique_helpers = set()
> + self.define_unique_helpers = []
>
> def parse_element(self):
> proto = self.parse_symbol()
> @@ -193,19 +195,41 @@ class HeaderParser(object):
> except NoSyscallCommandFound:
> break
>
> - def parse_helpers(self):
> + def parse_desc_helpers(self):
> self.seek_to('* Start of BPF helper function descriptions:',
> 'Could not find start of eBPF helper descriptions list')
> while True:
> try:
> helper = self.parse_helper()
> self.helpers.append(helper)
> + proto = helper.proto_break_down()
> + if proto['name'] not in self.desc_unique_helpers:
The "not in" check is unnecessary for set().
> + self.desc_unique_helpers.add(proto['name'])
> except NoHelperFound:
> break
>
> + def parse_define_helpers(self):
> + # Parse the number of FN(...) in #define __BPF_FUNC_MAPPER to compare
> + # later with the number of unique function names present in description
> + self.seek_to('#define __BPF_FUNC_MAPPER(FN)',
> + 'Could not find start of eBPF helper definition list')
> + # Searches for either one or more FN(\w+) defines or a backslash for newline
> + p = re.compile('\s*(FN\(\w+\))+|\\\\')
> + fn_defines_str = ''
> + while True:
> + capture = p.match(self.line)
> + if capture:
> + fn_defines_str += self.line
> + else:
> + break
> + self.line = self.reader.readline()
> + # Find the number of occurences of FN(\w+)
> + self.define_unique_helpers = re.findall('FN\(\w+\)', fn_defines_str)
How about we only save nr_define_unique_helpers in self?
> +
> def run(self):
> self.parse_syscall()
> - self.parse_helpers()
> + self.parse_desc_helpers()
> + self.parse_define_helpers()
> self.reader.close()
>
> ###############################################################################
> @@ -509,6 +533,8 @@ class PrinterHelpers(Printer):
> """
> def __init__(self, parser):
> self.elements = parser.helpers
> + self.desc_unique_helpers = parser.desc_unique_helpers
> + self.define_unique_helpers = parser.define_unique_helpers
>
> type_fwds = [
> 'struct bpf_fib_lookup',
> @@ -628,6 +654,22 @@ class PrinterHelpers(Printer):
> /* Forward declarations of BPF structs */'''
>
> print(header)
> +
> + nr_desc_unique_helpers = len(self.desc_unique_helpers)
> + nr_define_unique_helpers = len(self.define_unique_helpers)
> + if nr_desc_unique_helpers != nr_define_unique_helpers:
> + header_warning = '''
> +#warning The number of unique helpers in description (%d) don\'t match the number of unique helpers defined in __BPF_FUNC_MAPPER (%d)
> +''' % (nr_desc_unique_helpers, nr_define_unique_helpers)
> + if nr_desc_unique_helpers < nr_define_unique_helpers:
> + # Function description is parsed until no helper is found (which can be due to
> + # misformatting). Hence, only print the first missing/misformatted function.
> + header_warning += '''
> +#warning The description for %s is not present or formatted correctly.
> +''' % (self.define_unique_helpers[nr_desc_unique_helpers])
> + print(header_warning)
> +
> +
> for fwd in self.type_fwds:
> print('%s;' % fwd)
> print('')
> --
> 2.25.1
>
