2022-05-20 16:45 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
> 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.
>
> 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@xxxxxxxxxxxxx>
Nice work, thanks a lot Daniel for diving into the Python script!
