2022-01-14 16:48 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > On Wed, Jan 12, 2022 at 3:51 PM Song Liu <song@xxxxxxxxxx> wrote: >> >> On Wed, Jan 12, 2022 at 4:15 AM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote: >>> >>> 2022-01-12 11:49 UTC+0000 ~ Usama Arif <usama.arif@xxxxxxxxxxxxx> >>>> 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@xxxxxxxxxxxxx> >>> >>> Reviewed-by: Quentin Monnet <quentin@xxxxxxxxxxxxx> >> >> Acked-by: Song Liu <songliubraving@xxxxxx> >> >> Thanks! > > Would be great if we could also enforce minimal formatting consistency > (i.e., that Description and Return sections are present and that empty > line before the next function definition is present), but it's an > improvement anyway. Fixed up don't -> doesn't and applied to bpf-next. Just noting here for the record - Another possible follow-up could be to add the same check as you did for the documentation of the syscall subcommands in the same script (parse_syscall()), to make sure no bpf() subcommand is misformatted or missing in the doc. Quentin
