Message-ID: <23c52aaa-c3e2-4d90-611e-515728227e3e@netronome.com>
Date:   Fri, 14 Dec 2018 14:56:06 +0000
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH bpf-next 0/8] tools: bpftool: add probes for system and
 device

Hi Arnaldo,

2018-12-14 11:00 UTC-0300 ~ Arnaldo Carvalho de Melo <acme@...nel.org>
> Em Thu, Dec 13, 2018 at 12:19:14PM +0000, Quentin Monnet escreveu:
>> Hi,
>> This set add a new command to bpftool in order to dump a list of
>> eBPF-related parameters for the system (or for a specific network
>> device) to the console. Once again, this is based on a suggestion from
>> Daniel.
>>
>> At this time, output includes:
>>
>>     - Availability of bpf() system call
>>     - Availability of bpf() system call for unprivileged users
>>     - JIT status (enabled or not, with or without debugging traces)
>>     - JIT hardening status
>>     - JIT kallsyms exports status
>>     - Status of kernel compilation options related to BPF features
>>     - Release number of the running kernel
>>     - Availability of known eBPF program types
>>     - Availability of known eBPF map types
>>     - Availability of known eBPF helper functions
>>
>> There are three different ways to dump this information at this time:
>>
>>     - Plain output dumps probe results in plain text. It is the most
>>       flexible options for providing descriptive output to the user, but
>>       should not be relied upon for parsing the output.
>>     - JSON output is supported.
>>     - A third mode, available through the "macros" keyword appended to
>>       the command line, dumps the parameters as a series of "#define"
>>       directives, that can be included into a C header file for example.
>>
>> If the user does not have root privileges (or more precisely, the
>> CAP_SYS_ADMIN capability) detection will be erroneous for most
>> parameters. Therefore, forbid non-root users to run the command.
> 
> One other thing I noticed is that this has lots of goodies that are not
> bpftool specific, like the json writer, the procfs reading routines,
> etc.
> 
> perf has these and some were even moved to tools/lib/api/, things like
> finding the procfs, debugfs, sysfs mount points, routines to mount then
> automatically when the user can do it, etc.
> 
> Have you considered using them? If so, what prevented you from doing it?
> Licensing?

Some of it is not necessarily relevant. For JSON, for example, it seems
that related parts in perf provide JSON parsing. bpftool does not parse
any JSON, it only needs to print JSON, and I didn't find that in perf.
This being said making both JSON parsing and printing available under
e.g. tools/lib/api (providing licensing details allow it) would make
sense to me, so that it can be shared if e.g. bpftool had to parse JSON
one day (or if other tools were added).

As for BPF, most of the low-level object management directly comes from
libbpf. I see perf uses it as well, but it doesn't really look like the
code of the two tools could be merged easily now.

Regarding procfs, debugfs, sysfs, it's true that there are things that
we could maybe reuse. Here I don't have any argument - mostly I didn't
notice these functions were available here when working on bpftool (I
mostly looked at iproute2 instead).

> Sharing these non-tool specific routines in tools/lib/ is a good thing
> and we should work out details on what prevents that from happening.
> 
> Another thing that came to mind is that the bpf loaded in perf has
> routines for figuring out the kbuild directory, kernel version, etc.
> 
> Please take a look at tools/perf/util/llvm-utils.c and
> tools/perf/util/bpf-loader.c. If we could reuse what is there, working
> out licensing details with Wang, etc, that would be awesome.

At the moment bpftool does not use LLVM stuff. My opinion is that for
loading programs, improvements that could benefit to multiple projects
should probably go to libbpf. For things such as retrieving the kernel
version, we can maybe do something.

Overall I agree it would be nice to share more code between the two,
providing licensing allows it. I'm not sure there's much to share about
BPF itself, but JSON or other FS-related things could maybe be reused.
I'll try to look at it in more details when I have some time, and keep
it in mind for future bpftool work. Thanks!

Powered by blists - more mailing lists