Re: [PATCH] bpf/scripts: add warning if the correct number of helpers are not auto-generated

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Dec 21, 2021 at 9:48 AM Usama Arif <usama.arif@bytedance.com> 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
> 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@bytedance.com>
> ---
>  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..736bd853155b 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:
> +                    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 = len(re.findall('FN\(\w+\)', fn_defines_str))
> +
>      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)

It fails in CI:
Traceback (most recent call last):
File "/home/runner/work/bpf/bpf/scripts/bpf_doc.py", line 778, in <module>
printer.print_all()
File "/home/runner/work/bpf/bpf/scripts/bpf_doc.py", line 257, in print_all
self.print_header()
File "/home/runner/work/bpf/bpf/scripts/bpf_doc.py", line 659, in print_header
nr_define_unique_helpers = len(self.define_unique_helpers)
TypeError: object of type 'int' has no len()