On 12/17/2018 11:44 AM, Quentin Monnet wrote:
> 2018-12-16 01:14 UTC+0100 ~ Daniel Borkmann <dan...@iogearbox.net>
>> On 12/15/2018 04:31 AM, Quentin Monnet wrote:
>>> 2018-12-15 00:40 UTC+0100 ~ Daniel Borkmann <dan...@iogearbox.net>
>>>> On 12/13/2018 01:19 PM, Quentin Monnet wrote:
>>>>> Add a set of probes to dump the eBPF-related parameters available from
>>>>> /proc/: availability of bpf() syscall for unprivileged users,
>>>>> JIT compiler status and hardening status, kallsyms exports status.
>>>>>
>>>>> Sample output:
>>>>>
>>>>> # bpftool feature probe kernel
>>>>> Scanning system configuration...
>>>>> bpf() syscall for unprivileged users is enabled
>>>>> JIT compiler is disabled
>>>>> JIT compiler hardening is disabled
>>>>> JIT compiler kallsyms exports are disabled
>>>>> ...
>>>>>
>>>>> # bpftool --json --pretty feature probe kernel
>>>>> {
>>>>> "system_config": {
>>>>> "unprivileged_bpf_disabled": 0,
>>>>> "bpf_jit_enable": 0,
>>>>> "bpf_jit_harden": 0,
>>>>> "bpf_jit_kallsyms": 0
>>>>> },
>>>>> ...
>>>>> }
>>>>>
>>>>> # bpftool feature probe kernel macros prefix BPFTOOL_
>>>>> #define UNPRIVILEGED_BPF_DISABLED UNPRIVILEGED_BPF_DISABLED_OFF
>>>>> #define UNPRIVILEGED_BPF_DISABLED_OFF 0
>>>>> #define UNPRIVILEGED_BPF_DISABLED_ON 1
>>>>> #define UNPRIVILEGED_BPF_DISABLED_UNKNOWN -1
>>>>> #define JIT_COMPILER_ENABLE JIT_COMPILER_ENABLE_OFF
>>>>> #define JIT_COMPILER_ENABLE_OFF 0
>>>>> #define JIT_COMPILER_ENABLE_ON 1
>>>>> #define JIT_COMPILER_ENABLE_ON_WITH_DEBUG 2
>>>>> #define JIT_COMPILER_ENABLE_UNKNOWN -1
>>>>> #define JIT_COMPILER_HARDEN JIT_COMPILER_HARDEN_OFF
>>>>> #define JIT_COMPILER_HARDEN_OFF 0
>>>>> #define JIT_COMPILER_HARDEN_FOR_UNPRIVILEGED 1
>>>>> #define JIT_COMPILER_HARDEN_FOR_ALL_USERS 2
>>>>> #define JIT_COMPILER_HARDEN_UNKNOWN -1
>>>>> #define JIT_COMPILER_KALLSYMS JIT_COMPILER_KALLSYMS_OFF
>>>>> #define JIT_COMPILER_KALLSYMS_OFF 0
>>>>> #define JIT_COMPILER_KALLSYMS_FOR_ROOT 1
>>>>> #define JIT_COMPILER_KALLSYMS_UNKNOWN -1
>>>>> ...
>>>>
>>>> Hm, given these knobs may change at any point in time, what would
>>>> be a use case in an application for these if they cannot be relied
>>>> upon? (At least the jit_enable and jit_harden are transparent to
>>>> the user.)
>>>
>>> Granted, for those parameters it's a snapshot of the system at the time
>>> the probes are run. It can be useful, I suppose, if a server is not
>>> expected to change them often... And the plain output might be useful to
>>> a sysadmin who wants to have a quick look at BPF-related parameters, maybe?
>>
>> Hmm, but wouldn't the main purpose of this header file be to include it
>> into a BPF program to selectively enable / disable features (e.g. LPM
>> map vs hashtab when kernel does not support LPM type as one example)?
>> What would a use-case be for the above defines used inside such BPF prog?
>> (Similarly for the kernel config defines in the other patch, how would
>> a BPF prog use them?)
>>
>> I think perhaps the 'issue' is that the C-style header generation and json
>> dump are dumping the /exact/ same information. Is this a requirement?
>> Wouldn't it be better to evolve the two /independently/?
>>
>> E.g. the system_config bits from the json dump and BPF-related kernel
>> config, perhaps also a listing of available maps, progs with supported
>> helpers for a prog would be useful for the json dump for an admin or
>> orchestration daemon to adapt to the underlying kernel where it could
>> just parse the json and doesn't have to do the queries by itself.
>>
>> But for the header generation, I would only place defines in there that
>> are strictly relevant for the BPF program author. Available maps, progs
>> and helpers is a good start there, later we could also put others in there
>> such as [0] and similar specifics or quirks to verifier behavior that
>> would be relevant in terms of work-arounds for supporting different kernel
>> versions; but on a case by case basis. There things might potentially be
>> less interesting for a json dump (though the json dump could overall be
>> a superset of the info from the header file).
>>
>> [0]
>> https://github.com/cilium/cilium/blob/master/bpf/probes/raw_mark_map_val.t
>
> For the use case about kernel config options, I was thinking about Cilium
> which collects some of them as well [0], but maybe it's not worth having it
> in the C-style header for now. For procfs parameters, maybe it's not so
> relevant indeed to have them at all in this output.
>
> So you have a point, I suppose. I do not have any hard requirement about
> having the #define and the JSON similar; I argued with Stanislav that I
> didn't want to introduce small losses of information between the two, but if
> we consider them entirely different from the start it is not the same
> thing... So maybe I should just stick to the basics for the #define output,
> as you suggest.
>
> I've seen the other probes used by Cilium, but I intentionally left the most
> specific one aside for now, there's enough to do with the current probes :).
> But yeah, it would make sense to have them added in the future. And for the
> record, I like the idea of keeping JSON a superset of the available
> information indeed.
Sounds good to me, I also think it's better to keep the json output a superset;
definitely allows for more flexibility and if there's a real world use case to
have some of that additional information also in the header output as defines,
we can add that at a later point in time.
> I'll go with just prog/map types and helpers for the #define in my next
> version. This should also settle the discussion on the format of the macros
> used in this first version for the procfs parameters.
Yes makes sense, this would be great as a start, and we can extend it as needed.
Thanks,
Daniel
