Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and
Date: Fri, 30 Jul 2021 14:58:19 -0700	[thread overview]
Message-ID: <CAEf4BzZZHBmCvbpza0VavP0L7uGk2bR1fF5zqjzLNdc_-bRmtA@mail.gmail.com> (raw)
In-Reply-To: <c61560c7-47eb-bd86-45cb-131b15cd89c1@isovalent.com>

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.

      reply	other threads:[~2021-07-30 21:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 16:29 Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 1/7] tools: bpftool: slightly ease bash completion updates Quentin Monnet
2021-07-30 18:45   ` Andrii Nakryiko
2021-07-30 21:46     ` Quentin Monnet
2021-07-30 21:54       ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 2/7] selftests/bpf: check consistency between bpftool source, doc, completion Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 3/7] tools: bpftool: complete and synchronise attach or map types Quentin Monnet
2021-07-30 18:52   ` Andrii Nakryiko
2021-07-30 21:47     ` Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 4/7] tools: bpftool: update and synchronise option list in doc and help msg Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 5/7] selftests/bpf: update bpftool's consistency script for checking options Quentin Monnet
2021-07-29 16:29 ` [PATCH bpf-next 6/7] tools: bpftool: document and add bash completion for -L, -B options Quentin Monnet
2021-07-30 18:59   ` Andrii Nakryiko
2021-07-30 21:48     ` Quentin Monnet
2021-07-30 22:10       ` Andrii Nakryiko
2021-07-29 16:29 ` [PATCH bpf-next 7/7] tools: bpftool: complete metrics list in "bpftool prog profile" doc Quentin Monnet
2021-07-30 19:06 ` [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and Andrii Nakryiko
2021-07-30 21:48   ` Quentin Monnet
2021-07-30 21:58     ` Andrii Nakryiko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEf4BzZZHBmCvbpza0VavP0L7uGk2bR1fF5zqjzLNdc_-bRmtA@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    --cc=quentin@isovalent.com \
    --subject='Re: [PATCH bpf-next 0/7] tools: bpftool: update, synchronise and' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox