Re: [PATCH v6] bpf/scripts: raise an exception if the correct number of helpers are not generated

[Quentin Monnet <quentin@isovalent.com>]

2022-01-12 11:49 UTC+0000 ~ Usama Arif <usama.arif@bytedance.com>
> Currently bpf_helper_defs.h and the bpf helpers man page are 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 raises an
> Exception 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>

Reviewed-by: Quentin Monnet <quentin@isovalent.com>

Looks cleaner with the check in a dedicated function. Thanks a lot!

> ---
>  scripts/bpf_doc.py | 50 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
> index a6403ddf5de7..76c96df095e3 100755
> --- a/scripts/bpf_doc.py
> +++ b/scripts/bpf_doc.py

> @@ -295,6 +320,25 @@ class PrinterRST(Printer):
>  
>          print('')
>  
> +def helper_number_check(desc_unique_helpers, define_unique_helpers):
> +    """
> +    Checks the number of functions documented within the header file
> +    with those present as part of #define __BPF_FUNC_MAPPER and raise an
> +    Exception if they don't match.
> +    """
> +    nr_desc_unique_helpers = len(desc_unique_helpers)
> +    nr_define_unique_helpers = len(define_unique_helpers)
> +    if nr_desc_unique_helpers != nr_define_unique_helpers:
> +        helper_exception = '''
> +The number of unique helpers in description (%d) don\'t match the number of unique helpers defined in __BPF_FUNC_MAPPER (%d)

Nit: don't -> doesn't
(but probably not worth a respin)