mbox series

[bpf-next,0/7] tools: bpftool: update, synchronise and

Message ID 20210729162932.30365-1-quentin@isovalent.com (mailing list archive)
Headers show
Series tools: bpftool: update, synchronise and | expand

Message

Quentin Monnet July 29, 2021, 4:29 p.m. UTC
To work with the different program types, map types, attach types etc.
supported by eBPF, bpftool needs occasional updates to learn about the new
features supported by the kernel. When such types translate into new
keyword for the command line, updates are expected in several locations:
typically, the help message displayed from bpftool itself, the manual page,
and the bash completion file should be updated. The options used by the
different commands for bpftool should also remain synchronised at those
locations.

Several omissions have occurred in the past, and a number of types are
still missing today. This set is an attempt to improve the situation. It
brings up-to-date the lists of types or options in bpftool, and also adds a
Python script to the BPF selftests to automatically check that most of
these lists remain synchronised.

Quentin Monnet (7):
  tools: bpftool: slightly ease bash completion updates
  selftests/bpf: check consistency between bpftool source, doc,
    completion
  tools: bpftool: complete and synchronise attach or map types
  tools: bpftool: update and synchronise option list in doc and help msg
  selftests/bpf: update bpftool's consistency script for checking
    options
  tools: bpftool: document and add bash completion for -L, -B options
  tools: bpftool: complete metrics list in "bpftool prog profile" doc

 .../bpf/bpftool/Documentation/bpftool-btf.rst |  48 +-
 .../bpftool/Documentation/bpftool-cgroup.rst  |   3 +-
 .../bpftool/Documentation/bpftool-feature.rst |   2 +-
 .../bpf/bpftool/Documentation/bpftool-gen.rst |   9 +-
 .../bpftool/Documentation/bpftool-iter.rst    |   2 +
 .../bpftool/Documentation/bpftool-link.rst    |   3 +-
 .../bpf/bpftool/Documentation/bpftool-map.rst |   3 +-
 .../bpf/bpftool/Documentation/bpftool-net.rst |   2 +-
 .../bpftool/Documentation/bpftool-perf.rst    |   2 +-
 .../bpftool/Documentation/bpftool-prog.rst    |  36 +-
 .../Documentation/bpftool-struct_ops.rst      |   2 +-
 tools/bpf/bpftool/Documentation/bpftool.rst   |  12 +-
 tools/bpf/bpftool/bash-completion/bpftool     |  69 ++-
 tools/bpf/bpftool/btf.c                       |   3 +-
 tools/bpf/bpftool/cgroup.c                    |   3 +-
 tools/bpf/bpftool/common.c                    |  76 +--
 tools/bpf/bpftool/feature.c                   |   1 +
 tools/bpf/bpftool/gen.c                       |   3 +-
 tools/bpf/bpftool/iter.c                      |   2 +
 tools/bpf/bpftool/link.c                      |   3 +-
 tools/bpf/bpftool/main.c                      |   3 +-
 tools/bpf/bpftool/main.h                      |   3 +-
 tools/bpf/bpftool/map.c                       |   5 +-
 tools/bpf/bpftool/net.c                       |   1 +
 tools/bpf/bpftool/perf.c                      |   5 +-
 tools/bpf/bpftool/prog.c                      |   8 +-
 tools/bpf/bpftool/struct_ops.c                |   2 +-
 tools/testing/selftests/bpf/Makefile          |   1 +
 .../selftests/bpf/test_bpftool_synctypes.py   | 586 ++++++++++++++++++
 29 files changed, 802 insertions(+), 96 deletions(-)
 create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py

Comments

Andrii Nakryiko July 30, 2021, 7:06 p.m. UTC | #1
On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> To work with the different program types, map types, attach types etc.
> supported by eBPF, bpftool needs occasional updates to learn about the new
> features supported by the kernel. When such types translate into new
> keyword for the command line, updates are expected in several locations:
> typically, the help message displayed from bpftool itself, the manual page,
> and the bash completion file should be updated. The options used by the
> different commands for bpftool should also remain synchronised at those
> locations.
>
> Several omissions have occurred in the past, and a number of types are
> still missing today. This set is an attempt to improve the situation. It
> brings up-to-date the lists of types or options in bpftool, and also adds a
> Python script to the BPF selftests to automatically check that most of
> these lists remain synchronised.
>
> Quentin Monnet (7):
>   tools: bpftool: slightly ease bash completion updates
>   selftests/bpf: check consistency between bpftool source, doc,
>     completion
>   tools: bpftool: complete and synchronise attach or map types
>   tools: bpftool: update and synchronise option list in doc and help msg
>   selftests/bpf: update bpftool's consistency script for checking
>     options
>   tools: bpftool: document and add bash completion for -L, -B options
>   tools: bpftool: complete metrics list in "bpftool prog profile" doc
>
>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  48 +-
>  .../bpftool/Documentation/bpftool-cgroup.rst  |   3 +-
>  .../bpftool/Documentation/bpftool-feature.rst |   2 +-
>  .../bpf/bpftool/Documentation/bpftool-gen.rst |   9 +-
>  .../bpftool/Documentation/bpftool-iter.rst    |   2 +
>  .../bpftool/Documentation/bpftool-link.rst    |   3 +-
>  .../bpf/bpftool/Documentation/bpftool-map.rst |   3 +-
>  .../bpf/bpftool/Documentation/bpftool-net.rst |   2 +-
>  .../bpftool/Documentation/bpftool-perf.rst    |   2 +-
>  .../bpftool/Documentation/bpftool-prog.rst    |  36 +-
>  .../Documentation/bpftool-struct_ops.rst      |   2 +-
>  tools/bpf/bpftool/Documentation/bpftool.rst   |  12 +-
>  tools/bpf/bpftool/bash-completion/bpftool     |  69 ++-
>  tools/bpf/bpftool/btf.c                       |   3 +-
>  tools/bpf/bpftool/cgroup.c                    |   3 +-
>  tools/bpf/bpftool/common.c                    |  76 +--
>  tools/bpf/bpftool/feature.c                   |   1 +
>  tools/bpf/bpftool/gen.c                       |   3 +-
>  tools/bpf/bpftool/iter.c                      |   2 +
>  tools/bpf/bpftool/link.c                      |   3 +-
>  tools/bpf/bpftool/main.c                      |   3 +-
>  tools/bpf/bpftool/main.h                      |   3 +-
>  tools/bpf/bpftool/map.c                       |   5 +-
>  tools/bpf/bpftool/net.c                       |   1 +
>  tools/bpf/bpftool/perf.c                      |   5 +-
>  tools/bpf/bpftool/prog.c                      |   8 +-
>  tools/bpf/bpftool/struct_ops.c                |   2 +-
>  tools/testing/selftests/bpf/Makefile          |   1 +
>  .../selftests/bpf/test_bpftool_synctypes.py   | 586 ++++++++++++++++++
>  29 files changed, 802 insertions(+), 96 deletions(-)
>  create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
>
> --
> 2.30.2
>

The patch set name ends abruptly at "synchronise and "... And what? I
need to know :)

Overall, it looks good, though I can't speak Python much, so I trust
the script works and we'll fix whatever is necessary as we go. I had
one small real nit about not re-formatting tons of existing lines for
no good reason, let's keep Git blame a bit more useful.

Also, it doesn't seem like you are actually calling a new script from
selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
change in kernel header would require bpftool changes in the same
patch. But once this lands, we should probably run this in
kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
So please follow up with that as well afterwards, that way you won't
be the only one nagging people about missed doc updates.

  [0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
  [1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
Quentin Monnet July 30, 2021, 9:48 p.m. UTC | #2
2021-07-30 12:06 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> To work with the different program types, map types, attach types etc.
>> supported by eBPF, bpftool needs occasional updates to learn about the new
>> features supported by the kernel. When such types translate into new
>> keyword for the command line, updates are expected in several locations:
>> typically, the help message displayed from bpftool itself, the manual page,
>> and the bash completion file should be updated. The options used by the
>> different commands for bpftool should also remain synchronised at those
>> locations.
>>
>> Several omissions have occurred in the past, and a number of types are
>> still missing today. This set is an attempt to improve the situation. It
>> brings up-to-date the lists of types or options in bpftool, and also adds a
>> Python script to the BPF selftests to automatically check that most of
>> these lists remain synchronised.
>>
>> Quentin Monnet (7):
>>   tools: bpftool: slightly ease bash completion updates
>>   selftests/bpf: check consistency between bpftool source, doc,
>>     completion
>>   tools: bpftool: complete and synchronise attach or map types
>>   tools: bpftool: update and synchronise option list in doc and help msg
>>   selftests/bpf: update bpftool's consistency script for checking
>>     options
>>   tools: bpftool: document and add bash completion for -L, -B options
>>   tools: bpftool: complete metrics list in "bpftool prog profile" doc
>>
>>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  48 +-
>>  .../bpftool/Documentation/bpftool-cgroup.rst  |   3 +-
>>  .../bpftool/Documentation/bpftool-feature.rst |   2 +-
>>  .../bpf/bpftool/Documentation/bpftool-gen.rst |   9 +-
>>  .../bpftool/Documentation/bpftool-iter.rst    |   2 +
>>  .../bpftool/Documentation/bpftool-link.rst    |   3 +-
>>  .../bpf/bpftool/Documentation/bpftool-map.rst |   3 +-
>>  .../bpf/bpftool/Documentation/bpftool-net.rst |   2 +-
>>  .../bpftool/Documentation/bpftool-perf.rst    |   2 +-
>>  .../bpftool/Documentation/bpftool-prog.rst    |  36 +-
>>  .../Documentation/bpftool-struct_ops.rst      |   2 +-
>>  tools/bpf/bpftool/Documentation/bpftool.rst   |  12 +-
>>  tools/bpf/bpftool/bash-completion/bpftool     |  69 ++-
>>  tools/bpf/bpftool/btf.c                       |   3 +-
>>  tools/bpf/bpftool/cgroup.c                    |   3 +-
>>  tools/bpf/bpftool/common.c                    |  76 +--
>>  tools/bpf/bpftool/feature.c                   |   1 +
>>  tools/bpf/bpftool/gen.c                       |   3 +-
>>  tools/bpf/bpftool/iter.c                      |   2 +
>>  tools/bpf/bpftool/link.c                      |   3 +-
>>  tools/bpf/bpftool/main.c                      |   3 +-
>>  tools/bpf/bpftool/main.h                      |   3 +-
>>  tools/bpf/bpftool/map.c                       |   5 +-
>>  tools/bpf/bpftool/net.c                       |   1 +
>>  tools/bpf/bpftool/perf.c                      |   5 +-
>>  tools/bpf/bpftool/prog.c                      |   8 +-
>>  tools/bpf/bpftool/struct_ops.c                |   2 +-
>>  tools/testing/selftests/bpf/Makefile          |   1 +
>>  .../selftests/bpf/test_bpftool_synctypes.py   | 586 ++++++++++++++++++
>>  29 files changed, 802 insertions(+), 96 deletions(-)
>>  create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
>>
>> --
>> 2.30.2
>>
> 
> The patch set name ends abruptly at "synchronise and "... And what? I
> need to know :)

"... and validate types and options" is the missing part. I noticed
after sending -_-. My editor wrapped the Subject: line, resulting in a
truncation. I'll fix for v2 to relieve readers from the suspense :).

> 
> Overall, it looks good, though I can't speak Python much, so I trust
> the script works and we'll fix whatever is necessary as we go. I had
> one small real nit about not re-formatting tons of existing lines for
> no good reason, let's keep Git blame a bit more useful.
> 
> Also, it doesn't seem like you are actually calling a new script from
> selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
> change in kernel header would require bpftool changes in the same
> patch.

Hmm. Ha. Certainly I wouldn't do such a thing. Please don't look again
at patch 2, and let's focus on v2. 0:)

> But once this lands, we should probably run this in
> kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
> So please follow up with that as well afterwards, that way you won't
> be the only one nagging people about missed doc updates.
> 
>   [0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
>   [1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
> 

What's the process to add them to the CI (did I miss some doc)? Should I
just go for a GitHub PR once the script is merged in bpf-next, or do you
have a tool to mirror the relevant scripts? Do we need to have the
Python script in the kernel repo if we don't run it as part of the
selftest suite, by the way?
Andrii Nakryiko July 30, 2021, 9:58 p.m. UTC | #3
On Fri, Jul 30, 2021 at 2:48 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-07-30 12:06 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Thu, Jul 29, 2021 at 9:29 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> To work with the different program types, map types, attach types etc.
> >> supported by eBPF, bpftool needs occasional updates to learn about the new
> >> features supported by the kernel. When such types translate into new
> >> keyword for the command line, updates are expected in several locations:
> >> typically, the help message displayed from bpftool itself, the manual page,
> >> and the bash completion file should be updated. The options used by the
> >> different commands for bpftool should also remain synchronised at those
> >> locations.
> >>
> >> Several omissions have occurred in the past, and a number of types are
> >> still missing today. This set is an attempt to improve the situation. It
> >> brings up-to-date the lists of types or options in bpftool, and also adds a
> >> Python script to the BPF selftests to automatically check that most of
> >> these lists remain synchronised.
> >>
> >> Quentin Monnet (7):
> >>   tools: bpftool: slightly ease bash completion updates
> >>   selftests/bpf: check consistency between bpftool source, doc,
> >>     completion
> >>   tools: bpftool: complete and synchronise attach or map types
> >>   tools: bpftool: update and synchronise option list in doc and help msg
> >>   selftests/bpf: update bpftool's consistency script for checking
> >>     options
> >>   tools: bpftool: document and add bash completion for -L, -B options
> >>   tools: bpftool: complete metrics list in "bpftool prog profile" doc
> >>
> >>  .../bpf/bpftool/Documentation/bpftool-btf.rst |  48 +-
> >>  .../bpftool/Documentation/bpftool-cgroup.rst  |   3 +-
> >>  .../bpftool/Documentation/bpftool-feature.rst |   2 +-
> >>  .../bpf/bpftool/Documentation/bpftool-gen.rst |   9 +-
> >>  .../bpftool/Documentation/bpftool-iter.rst    |   2 +
> >>  .../bpftool/Documentation/bpftool-link.rst    |   3 +-
> >>  .../bpf/bpftool/Documentation/bpftool-map.rst |   3 +-
> >>  .../bpf/bpftool/Documentation/bpftool-net.rst |   2 +-
> >>  .../bpftool/Documentation/bpftool-perf.rst    |   2 +-
> >>  .../bpftool/Documentation/bpftool-prog.rst    |  36 +-
> >>  .../Documentation/bpftool-struct_ops.rst      |   2 +-
> >>  tools/bpf/bpftool/Documentation/bpftool.rst   |  12 +-
> >>  tools/bpf/bpftool/bash-completion/bpftool     |  69 ++-
> >>  tools/bpf/bpftool/btf.c                       |   3 +-
> >>  tools/bpf/bpftool/cgroup.c                    |   3 +-
> >>  tools/bpf/bpftool/common.c                    |  76 +--
> >>  tools/bpf/bpftool/feature.c                   |   1 +
> >>  tools/bpf/bpftool/gen.c                       |   3 +-
> >>  tools/bpf/bpftool/iter.c                      |   2 +
> >>  tools/bpf/bpftool/link.c                      |   3 +-
> >>  tools/bpf/bpftool/main.c                      |   3 +-
> >>  tools/bpf/bpftool/main.h                      |   3 +-
> >>  tools/bpf/bpftool/map.c                       |   5 +-
> >>  tools/bpf/bpftool/net.c                       |   1 +
> >>  tools/bpf/bpftool/perf.c                      |   5 +-
> >>  tools/bpf/bpftool/prog.c                      |   8 +-
> >>  tools/bpf/bpftool/struct_ops.c                |   2 +-
> >>  tools/testing/selftests/bpf/Makefile          |   1 +
> >>  .../selftests/bpf/test_bpftool_synctypes.py   | 586 ++++++++++++++++++
> >>  29 files changed, 802 insertions(+), 96 deletions(-)
> >>  create mode 100755 tools/testing/selftests/bpf/test_bpftool_synctypes.py
> >>
> >> --
> >> 2.30.2
> >>
> >
> > The patch set name ends abruptly at "synchronise and "... And what? I
> > need to know :)
>
> "... and validate types and options" is the missing part. I noticed
> after sending -_-. My editor wrapped the Subject: line, resulting in a
> truncation. I'll fix for v2 to relieve readers from the suspense :).
>
> >
> > Overall, it looks good, though I can't speak Python much, so I trust
> > the script works and we'll fix whatever is necessary as we go. I had
> > one small real nit about not re-formatting tons of existing lines for
> > no good reason, let's keep Git blame a bit more useful.
> >
> > Also, it doesn't seem like you are actually calling a new script from
> > selftests/bpf/Makefile, right? That's good, because otherwise any UAPI
> > change in kernel header would require bpftool changes in the same
> > patch.
>
> Hmm. Ha. Certainly I wouldn't do such a thing. Please don't look again
> at patch 2, and let's focus on v2. 0:)

You got it.

>
> > But once this lands, we should probably run this in
> > kernel-patches CI ([0]) and, maybe, not sure, libbpf CI ([1]) as well.
> > So please follow up with that as well afterwards, that way you won't
> > be the only one nagging people about missed doc updates.
> >
> >   [0] https://github.com/kernel-patches/vmtest/tree/master/travis-ci/vmtest
> >   [1] https://github.com/libbpf/libbpf/tree/master/travis-ci/vmtest
> >
>
> What's the process to add them to the CI (did I miss some doc)? Should I
> just go for a GitHub PR once the script is merged in bpf-next, or do you
> have a tool to mirror the relevant scripts? Do we need to have the
> Python script in the kernel repo if we don't run it as part of the
> selftest suite, by the way?

Just normal, nicely prepared and described PRs against respective repos.