mbox series

[bpf-next,v3,00/12] libbpf: Textual representation of enums

Message ID 20220519213001.729261-1-deso@posteo.net (mailing list archive)
Headers show
Series libbpf: Textual representation of enums | expand

Message

Daniel Müller May 19, 2022, 9:29 p.m. UTC
This patch set introduces the means for querying a textual representation of
the following BPF related enum types:
- enum bpf_map_type
- enum bpf_prog_type
- enum bpf_attach_type
- enum bpf_link_type

To make that possible, we introduce a new public function for each of the types:
libbpf_bpf_<type>_type_str.

Having a way to query a textual representation has been asked for in the past
(by systemd, among others). Such representations can generally be useful in
tracing and logging contexts, among others. At this point, at least one client,
bpftool, maintains such a mapping manually, which is prone to get out of date as
new enum variants are introduced. libbpf is arguably best situated to keep this
list complete and up-to-date. This patch series adds BTF based tests to ensure
that exhaustiveness is upheld moving forward.

The libbpf provided textual representation can be inferred from the
corresponding enum variant name by removing the prefix and lowercasing the
remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
bpftool does not use such a programmatic approach for some of the
bpf_attach_type variants. We decided changing its behavior to work with libbpf
representations. However, for user inputs, specifically, we do keep support for
the traditionally used names around (please see patch "bpftool: Use
libbpf_bpf_attach_type_str").

The patch series is structured as follows:
- for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
  bpf_link_type}:
  - we first introduce the corresponding public libbpf API function
  - we then add BTF based self-tests
  - we lastly adjust bpftool to use the libbpf provided functionality

Changelog:
v2 -> v3:
- use LIBBPF_1.0.0 section in libbpf.map for newly added exports

v1 -> v2:
- adjusted bpftool to work with algorithmically determined attach types as
  libbpf now uses (just removed prefix from enum name and lowercased the rest)
  - adjusted tests, man page, and completion script to work with the new names
  - renamed bpf_attach_type_str -> bpf_attach_type_input_str
  - for input: added special cases that accept the traditionally used strings as
    well
- changed 'char const *' -> 'const char *'

Signed-off-by: Daniel Müller <deso@posteo.net>
Acked-by: Yonghong Song <yhs@fb.com>
Cc: Quentin Monnet <quentin@isovalent.com>

Daniel Müller (12):
  libbpf: Introduce libbpf_bpf_prog_type_str
  selftests/bpf: Add test for libbpf_bpf_prog_type_str
  bpftool: Use libbpf_bpf_prog_type_str
  libbpf: Introduce libbpf_bpf_map_type_str
  selftests/bpf: Add test for libbpf_bpf_map_type_str
  bpftool: Use libbpf_bpf_map_type_str
  libbpf: Introduce libbpf_bpf_attach_type_str
  selftests/bpf: Add test for libbpf_bpf_attach_type_str
  bpftool: Use libbpf_bpf_attach_type_str
  libbpf: Introduce libbpf_bpf_link_type_str
  selftests/bpf: Add test for libbpf_bpf_link_type_str
  bpftool: Use libbpf_bpf_link_type_str

 .../bpftool/Documentation/bpftool-cgroup.rst  |  16 +-
 .../bpftool/Documentation/bpftool-prog.rst    |   5 +-
 tools/bpf/bpftool/bash-completion/bpftool     |  18 +-
 tools/bpf/bpftool/cgroup.c                    |  49 +++--
 tools/bpf/bpftool/common.c                    |  82 +++----
 tools/bpf/bpftool/feature.c                   |  87 +++++---
 tools/bpf/bpftool/link.c                      |  61 +++---
 tools/bpf/bpftool/main.h                      |  23 +-
 tools/bpf/bpftool/map.c                       |  82 +++----
 tools/bpf/bpftool/prog.c                      |  77 +++----
 tools/lib/bpf/libbpf.c                        | 160 ++++++++++++++
 tools/lib/bpf/libbpf.h                        |  36 +++
 tools/lib/bpf/libbpf.map                      |   6 +
 .../selftests/bpf/prog_tests/libbpf_str.c     | 207 ++++++++++++++++++
 .../selftests/bpf/test_bpftool_synctypes.py   | 163 ++++++--------
 15 files changed, 738 insertions(+), 334 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c

Comments

Andrii Nakryiko May 20, 2022, 11:45 p.m. UTC | #1
On Thu, May 19, 2022 at 2:30 PM Daniel Müller <deso@posteo.net> wrote:
>
> This patch set introduces the means for querying a textual representation of
> the following BPF related enum types:
> - enum bpf_map_type
> - enum bpf_prog_type
> - enum bpf_attach_type
> - enum bpf_link_type
>
> To make that possible, we introduce a new public function for each of the types:
> libbpf_bpf_<type>_type_str.
>
> Having a way to query a textual representation has been asked for in the past
> (by systemd, among others). Such representations can generally be useful in
> tracing and logging contexts, among others. At this point, at least one client,
> bpftool, maintains such a mapping manually, which is prone to get out of date as
> new enum variants are introduced. libbpf is arguably best situated to keep this
> list complete and up-to-date. This patch series adds BTF based tests to ensure
> that exhaustiveness is upheld moving forward.
>
> The libbpf provided textual representation can be inferred from the
> corresponding enum variant name by removing the prefix and lowercasing the
> remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
> bpftool does not use such a programmatic approach for some of the
> bpf_attach_type variants. We decided changing its behavior to work with libbpf
> representations. However, for user inputs, specifically, we do keep support for
> the traditionally used names around (please see patch "bpftool: Use
> libbpf_bpf_attach_type_str").
>
> The patch series is structured as follows:
> - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
>   bpf_link_type}:
>   - we first introduce the corresponding public libbpf API function
>   - we then add BTF based self-tests
>   - we lastly adjust bpftool to use the libbpf provided functionality
>
> Changelog:
> v2 -> v3:
> - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
>
> v1 -> v2:
> - adjusted bpftool to work with algorithmically determined attach types as
>   libbpf now uses (just removed prefix from enum name and lowercased the rest)
>   - adjusted tests, man page, and completion script to work with the new names
>   - renamed bpf_attach_type_str -> bpf_attach_type_input_str
>   - for input: added special cases that accept the traditionally used strings as
>     well
> - changed 'char const *' -> 'const char *'
>
> Signed-off-by: Daniel Müller <deso@posteo.net>
> Acked-by: Yonghong Song <yhs@fb.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
>

So this looks good to me for libbpf and selftests/bpf changes. I'll
wait for Quentin to give his acks at least for bpftool changes.
Quention, please take a look when you get a chance.

Few small nits, please accommodate them in next version, if you happen
to send another one. If not, I'll try to remember to fix it up when
applying.

1. Received Acked-by/Reviewed-by tags should be added to each
individual patch, not cover letter.

2. You are using /** ... */ comments, which are considered to be kdoc
comments and they have some additional formatting, which some of the
tooling run on patches in Patchworks complains about [0]. Please use
just /* ... */ style everywhere where it's not actual kdoc (or libbpf
API documentation).

  [0] https://patchwork.hopto.org/static/nipa/643335/12856068/kdoc/summary

> Daniel Müller (12):
>   libbpf: Introduce libbpf_bpf_prog_type_str
>   selftests/bpf: Add test for libbpf_bpf_prog_type_str
>   bpftool: Use libbpf_bpf_prog_type_str
>   libbpf: Introduce libbpf_bpf_map_type_str
>   selftests/bpf: Add test for libbpf_bpf_map_type_str
>   bpftool: Use libbpf_bpf_map_type_str
>   libbpf: Introduce libbpf_bpf_attach_type_str
>   selftests/bpf: Add test for libbpf_bpf_attach_type_str
>   bpftool: Use libbpf_bpf_attach_type_str
>   libbpf: Introduce libbpf_bpf_link_type_str
>   selftests/bpf: Add test for libbpf_bpf_link_type_str
>   bpftool: Use libbpf_bpf_link_type_str
>
>  .../bpftool/Documentation/bpftool-cgroup.rst  |  16 +-
>  .../bpftool/Documentation/bpftool-prog.rst    |   5 +-
>  tools/bpf/bpftool/bash-completion/bpftool     |  18 +-
>  tools/bpf/bpftool/cgroup.c                    |  49 +++--
>  tools/bpf/bpftool/common.c                    |  82 +++----
>  tools/bpf/bpftool/feature.c                   |  87 +++++---
>  tools/bpf/bpftool/link.c                      |  61 +++---
>  tools/bpf/bpftool/main.h                      |  23 +-
>  tools/bpf/bpftool/map.c                       |  82 +++----
>  tools/bpf/bpftool/prog.c                      |  77 +++----
>  tools/lib/bpf/libbpf.c                        | 160 ++++++++++++++
>  tools/lib/bpf/libbpf.h                        |  36 +++
>  tools/lib/bpf/libbpf.map                      |   6 +
>  .../selftests/bpf/prog_tests/libbpf_str.c     | 207 ++++++++++++++++++
>  .../selftests/bpf/test_bpftool_synctypes.py   | 163 ++++++--------
>  15 files changed, 738 insertions(+), 334 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/libbpf_str.c
>
> --
> 2.30.2
>
Quentin Monnet May 23, 2022, 11:48 a.m. UTC | #2
2022-05-20 16:45 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, May 19, 2022 at 2:30 PM Daniel Müller <deso@posteo.net> wrote:
>>
>> This patch set introduces the means for querying a textual representation of
>> the following BPF related enum types:
>> - enum bpf_map_type
>> - enum bpf_prog_type
>> - enum bpf_attach_type
>> - enum bpf_link_type
>>
>> To make that possible, we introduce a new public function for each of the types:
>> libbpf_bpf_<type>_type_str.
>>
>> Having a way to query a textual representation has been asked for in the past
>> (by systemd, among others). Such representations can generally be useful in
>> tracing and logging contexts, among others. At this point, at least one client,
>> bpftool, maintains such a mapping manually, which is prone to get out of date as
>> new enum variants are introduced. libbpf is arguably best situated to keep this
>> list complete and up-to-date. This patch series adds BTF based tests to ensure
>> that exhaustiveness is upheld moving forward.
>>
>> The libbpf provided textual representation can be inferred from the
>> corresponding enum variant name by removing the prefix and lowercasing the
>> remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
>> bpftool does not use such a programmatic approach for some of the
>> bpf_attach_type variants. We decided changing its behavior to work with libbpf
>> representations. However, for user inputs, specifically, we do keep support for
>> the traditionally used names around (please see patch "bpftool: Use
>> libbpf_bpf_attach_type_str").
>>
>> The patch series is structured as follows:
>> - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
>>   bpf_link_type}:
>>   - we first introduce the corresponding public libbpf API function
>>   - we then add BTF based self-tests
>>   - we lastly adjust bpftool to use the libbpf provided functionality
>>
>> Changelog:
>> v2 -> v3:
>> - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
>>
>> v1 -> v2:
>> - adjusted bpftool to work with algorithmically determined attach types as
>>   libbpf now uses (just removed prefix from enum name and lowercased the rest)
>>   - adjusted tests, man page, and completion script to work with the new names
>>   - renamed bpf_attach_type_str -> bpf_attach_type_input_str
>>   - for input: added special cases that accept the traditionally used strings as
>>     well
>> - changed 'char const *' -> 'const char *'
>>
>> Signed-off-by: Daniel Müller <deso@posteo.net>
>> Acked-by: Yonghong Song <yhs@fb.com>
>> Cc: Quentin Monnet <quentin@isovalent.com>
>>
> 
> So this looks good to me for libbpf and selftests/bpf changes. I'll
> wait for Quentin to give his acks at least for bpftool changes.
> Quention, please take a look when you get a chance.
> 
> Few small nits, please accommodate them in next version, if you happen
> to send another one. If not, I'll try to remember to fix it up when
> applying.
> 
> 1. Received Acked-by/Reviewed-by tags should be added to each
> individual patch, not cover letter.
> 
> 2. You are using /** ... */ comments, which are considered to be kdoc
> comments and they have some additional formatting, which some of the
> tooling run on patches in Patchworks complains about [0]. Please use
> just /* ... */ style everywhere where it's not actual kdoc (or libbpf
> API documentation).
> 
>   [0] https://patchwork.hopto.org/static/nipa/643335/12856068/kdoc/summary
> 

Apologies, didn't have time to go through the Python changes last week.
This looks all good for me as well! With a few additional nitpicks on
patch 9, see my reply for that one. For all other patches, please feel
free to add:

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

Nice work, thanks a lot Daniel for diving into the Python script!
Daniel Müller May 23, 2022, 8:59 p.m. UTC | #3
On Fri, May 20, 2022 at 04:45:42PM -0700, Andrii Nakryiko wrote:
> On Thu, May 19, 2022 at 2:30 PM Daniel Müller <deso@posteo.net> wrote:
> >
> > This patch set introduces the means for querying a textual representation of
> > the following BPF related enum types:
> > - enum bpf_map_type
> > - enum bpf_prog_type
> > - enum bpf_attach_type
> > - enum bpf_link_type
> >
> > To make that possible, we introduce a new public function for each of the types:
> > libbpf_bpf_<type>_type_str.
> >
> > Having a way to query a textual representation has been asked for in the past
> > (by systemd, among others). Such representations can generally be useful in
> > tracing and logging contexts, among others. At this point, at least one client,
> > bpftool, maintains such a mapping manually, which is prone to get out of date as
> > new enum variants are introduced. libbpf is arguably best situated to keep this
> > list complete and up-to-date. This patch series adds BTF based tests to ensure
> > that exhaustiveness is upheld moving forward.
> >
> > The libbpf provided textual representation can be inferred from the
> > corresponding enum variant name by removing the prefix and lowercasing the
> > remainder. E.g., BPF_PROG_TYPE_SOCKET_FILTER -> socket_filter. Unfortunately,
> > bpftool does not use such a programmatic approach for some of the
> > bpf_attach_type variants. We decided changing its behavior to work with libbpf
> > representations. However, for user inputs, specifically, we do keep support for
> > the traditionally used names around (please see patch "bpftool: Use
> > libbpf_bpf_attach_type_str").
> >
> > The patch series is structured as follows:
> > - for each enumeration type in {bpf_prog_type, bpf_map_type, bpf_attach_type,
> >   bpf_link_type}:
> >   - we first introduce the corresponding public libbpf API function
> >   - we then add BTF based self-tests
> >   - we lastly adjust bpftool to use the libbpf provided functionality
> >
> > Changelog:
> > v2 -> v3:
> > - use LIBBPF_1.0.0 section in libbpf.map for newly added exports
> >
> > v1 -> v2:
> > - adjusted bpftool to work with algorithmically determined attach types as
> >   libbpf now uses (just removed prefix from enum name and lowercased the rest)
> >   - adjusted tests, man page, and completion script to work with the new names
> >   - renamed bpf_attach_type_str -> bpf_attach_type_input_str
> >   - for input: added special cases that accept the traditionally used strings as
> >     well
> > - changed 'char const *' -> 'const char *'
> >
> > Signed-off-by: Daniel Müller <deso@posteo.net>
> > Acked-by: Yonghong Song <yhs@fb.com>
> > Cc: Quentin Monnet <quentin@isovalent.com>
> >
> 
> So this looks good to me for libbpf and selftests/bpf changes. I'll
> wait for Quentin to give his acks at least for bpftool changes.
> Quention, please take a look when you get a chance.
> 
> Few small nits, please accommodate them in next version, if you happen
> to send another one. If not, I'll try to remember to fix it up when
> applying.
> 
> 1. Received Acked-by/Reviewed-by tags should be added to each
> individual patch, not cover letter.

Thanks for pointing that out. Will fix that up.

> 2. You are using /** ... */ comments, which are considered to be kdoc
> comments and they have some additional formatting, which some of the
> tooling run on patches in Patchworks complains about [0]. Please use
> just /* ... */ style everywhere where it's not actual kdoc (or libbpf
> API documentation).

Force of habit. Fixed.

Thanks for taking a look!

Daniel

[...]