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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Acked-by: Yonghong Song <yhs@xxxxxx>
> > Cc: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> >
> 
> 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

[...]



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux