| Message ID | 20220404083816.1560501-1-nborisov@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add btf__field_exists | expand |
On Mon, Apr 4, 2022 at 1:38 AM Nikolay Borisov <nborisov@suse.com> wrote: > > Hello, > > Here are 2 patches with which I want to probe what's the sentiments towards 2 > changes: > > 1. Introduction of libbpf APIs similar to the bpf counterparts: bpf_core_field_exists, > bpf_core_type_exists and bpf_core_enum_value_exists. Of those I've implemented only > the first one and the reasoning behind this is in the patch itself. However, the > TLDR is that there can be cases where based on the kernel version we have to make a > decision in userspace what set of kprobes to use. There are currently no convenince > api's to do this so one has to essentially open code the checks that can be provided > by the aforementioned APIs. > The problem is that what you've implemented is not a user-space equivalent of bpf_core_xxx() macros. CO-RE has extra logic around ___<flavor> suffixes, extra type checks, etc, etc. Helper you are adding does a very straightforward strings check, which isn't hard to implement and it doesn't have to be a set in stone API. So I'm a bit hesitant to add this. But I can share what I did in similar situations where I had to do some CO-RE check both on BPF side and know its result in user-space. I built a separate very simple BPF skeleton and all it did was perform various feature checks (including those that require CO-RE) and then returned the result through global variables. You can then trigger such BPF feature-checking program either through bpf_prog_test_run or through whatever other means (I actually did a simple sys_enter program in my case). See [0] for BPF program side and [1] for user-space activation/consumption of that. The benefit of this approach is that there is no way BPF and user-space sides can get "out of sync" in terms of their feature checking. With skeleton it's also extremely simple to do all this. [0] https://github.com/anakryiko/retsnoop/blob/master/src/calib_feat.bpf.c [1] https://github.com/anakryiko/retsnoop/blob/master/src/mass_attacher.c#L483-L529 > 2. The kernel has for_each_member macro but the libbpf library doesn't provide it, > this results in having to open code members enumeration in various places such as > in find_member_by_name/find_struct_ops_kern_types/bpf_map__init_kern_struct_ops/ > parse_btf_map_def and in the newly introduced btf__field_exists. So how about > bringing the convenience macro to libbpf? see my comment, not sure it's worth it > > The reason why this series is RFC is if people agree with the proposed changed > I'd be happy to extend it to add more *exists* APIs and do the conversion to > using the for_each_member macro. > > Nikolay Borisov (2): > libbpf: Add userspace version of for_each_member macro > libbpf: Add btf__field_exists > > tools/lib/bpf/btf.c | 28 ++++++++++++++++++++++++++++ > tools/lib/bpf/btf.h | 8 ++++++++ > tools/lib/bpf/libbpf.map | 1 + > 3 files changed, 37 insertions(+) > > -- > 2.25.1 >
On 6.04.22 г. 2:37 ч., Andrii Nakryiko wrote: > The problem is that what you've implemented is not a user-space > equivalent of bpf_core_xxx() macros. CO-RE has extra logic around > ___<flavor> suffixes, extra type checks, etc, etc. Helper you are > adding does a very straightforward strings check, which isn't hard to > implement and it doesn't have to be a set in stone API. So I'm a bit > hesitant to add this. > > But I can share what I did in similar situations where I had to do > some CO-RE check both on BPF side and know its result in user-space. I > built a separate very simple BPF skeleton and all it did was perform > various feature checks (including those that require CO-RE) and then > returned the result through global variables. You can then trigger > such BPF feature-checking program either through bpf_prog_test_run or > through whatever other means (I actually did a simple sys_enter > program in my case). See [0] for BPF program side and [1] for > user-space activation/consumption of that. > > The benefit of this approach is that there is no way BPF and > user-space sides can get "out of sync" in terms of their feature > checking. With skeleton it's also extremely simple to do all this. > > [0]https://github.com/anakryiko/retsnoop/blob/master/src/calib_feat.bpf.c > [1]https://github.com/anakryiko/retsnoop/blob/master/src/mass_attacher.c#L483-L529 > That's indeed neat, however what is the minimum kernel version required to have global variables work ? AFAIU one requirement is to use a recent-enough libbpf which supports the skeleton functionality which is fine, userspace components can be updated somewhat easily than target kernels.
On Tue, Apr 5, 2022 at 11:41 PM Nikolay Borisov <nborisov@suse.com> wrote: > > > > On 6.04.22 г. 2:37 ч., Andrii Nakryiko wrote: > > The problem is that what you've implemented is not a user-space > > equivalent of bpf_core_xxx() macros. CO-RE has extra logic around > > ___<flavor> suffixes, extra type checks, etc, etc. Helper you are > > adding does a very straightforward strings check, which isn't hard to > > implement and it doesn't have to be a set in stone API. So I'm a bit > > hesitant to add this. > > > > But I can share what I did in similar situations where I had to do > > some CO-RE check both on BPF side and know its result in user-space. I > > built a separate very simple BPF skeleton and all it did was perform > > various feature checks (including those that require CO-RE) and then > > returned the result through global variables. You can then trigger > > such BPF feature-checking program either through bpf_prog_test_run or > > through whatever other means (I actually did a simple sys_enter > > program in my case). See [0] for BPF program side and [1] for > > user-space activation/consumption of that. > > > > The benefit of this approach is that there is no way BPF and > > user-space sides can get "out of sync" in terms of their feature > > checking. With skeleton it's also extremely simple to do all this. > > > > [0]https://github.com/anakryiko/retsnoop/blob/master/src/calib_feat.bpf.c > > [1]https://github.com/anakryiko/retsnoop/blob/master/src/mass_attacher.c#L483-L529 > > > > > That's indeed neat, however what is the minimum kernel version required > to have global variables work ? AFAIU one requirement is to use a > recent-enough libbpf which supports the skeleton functionality which is > fine, userspace components can be updated somewhat easily than target > kernels. You need Linux 5.5 for global variables mapping into user-space (added in [0]). But you don't need to use global variables to pass information back to user-space. You can just do a trivial BPF_MAP_TYPE_ARRAY for this. It's a bit more verbose to work with from BPF and user-space side, but effectively is the same for this case. Then you basically have no extra Linux version constraints just because of this feature probing. Only the need to have vmlinux BTF. [0] fc9702273e2e ("bpf: Add mmap() support for BPF_MAP_TYPE_ARRAY")