netdev - Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and bpftool documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Sep 2020 15:48:23 +0100
From:   Quentin Monnet <quentin@...valent.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next 0/3] bpf: format fixes for BPF helpers and
 bpftool documentation

On 04/09/2020 22:40, Andrii Nakryiko wrote:
> On Fri, Sep 4, 2020 at 9:15 AM Quentin Monnet <quentin@...valent.com> wrote:
>>
>> This series contains minor fixes (or harmonisation edits) for the
>> bpftool-link documentation (first patch) and BPF helpers documentation
>> (last two patches), so that all related man pages can build without errors.
>>
>> Quentin Monnet (3):
>>   tools: bpftool: fix formatting in bpftool-link documentation
>>   bpf: fix formatting in documentation for BPF helpers
>>   tools, bpf: synchronise BPF UAPI header with tools
>>
>>  include/uapi/linux/bpf.h                      | 87 ++++++++++---------
>>  .../bpftool/Documentation/bpftool-link.rst    |  2 +-
>>  tools/include/uapi/linux/bpf.h                | 87 ++++++++++---------
>>  3 files changed, 91 insertions(+), 85 deletions(-)
>>
>> --
>> 2.20.1
>>
> 
> This obviously looks good to me:
> 
> Acked-by: Andrii Nakryiko <andriin@...com>
> 
> But do you think we can somehow prevent issues like this? Consider
> adding building/testing of documentation to selftests or something.
> Not sure if that will catch all the issues you've fixed, but that
> would be a good start.
> 

Thanks for the review.

As for preventing future issues, I see two cases. Some minor fixes done
to harmonise the look of the description for the different helpers could
be checked with some kind of dedicated checkpatch-like script that would
validate new helpers I suppose, but I'm not sure whether that's worth
the trouble of creating that script, creating rules and then enforcing them.

The issues that do raise warnings are more important to fix, and easier
to detect. We could simply build bpftool's documentation (which also
happens to build the doc for eBPF helpers) and checks for warnings. We
already have a script to test bpftool build in the selftests, so I can
add it there as a follow-up and make doc build fail on warnings.

On a somewhat related note I also started to work on a script to check
that bpftool is correctly sync'ed (with kernel regarding prog names /
map names etc., and between source code / doc / bash completion) but I
haven't found the time to finish that work yet.

Quentin