On Fri, Aug 25, 2023 at 6:01 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Fri, 2023-08-25 at 10:08 +0200, Benjamin Tissoires wrote: > > > > On Tue, Aug 22, 2023 at 11:42 PM Justin Stitt <justinstitt@xxxxxxxxxx> wrote: > > > > > > > Which kernel are you trying to test? > > > > > > > I tested your 2 commands on v6.5-rc7 and it just works. > > > > > > > > > > > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef) > > > > > > > > > > > > I ran these exact commands: > > > > > > > $ make mrproper > > > > > > > $ make LLVM=1 ARCH=x86_64 headers > > > > > > > $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests > > > > > > TARGETS=hid &> out > > > > > > > > > > > > and here's the contents of `out` (still warnings/errors): > > > > > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d > > > > > > > > > > > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas? > > > > > > > > > > Sigh... there is a high chance my Makefile is not correct and uses the > > > > > installed headers (I was running the exact same commands, but on a > > > > > v6.4-rc7+ kernel). > > > > > > > > > > But sorry, it will have to wait for tomorrow if you want me to have a > > > > > look at it. It's 11:35 PM here, and I need to go to bed > > > > Take it easy. Thanks for the prompt responses here! I'd like to get > > > > the entire kselftest make target building with Clang so that we can > > > > close [1]. > > > > Sorry I got urgent matters to tackle yesterday. > > > > It took me a while to understand what was going on, and I finally found > > it. > > > > struct hid_bpf_ctx is internal to the kernel, and so is declared in > > vmlinux.h, that we generate through BTF. But to generate the vmlinux.h > > with the correct symbols, these need to be present in the running > > kernel. > > And that's where we had a fundamental difference: I was running my > > compilations on a kernel v6.3+ (6.4.11) with that symbol available, and > > you are probably not. > > > > The bpf folks are using a clever trick to force the compilation[2]. And > > I think the following patch would work for you: > > Hi Benjamin, Justin, > > You might want to take a look at these two links: > [1] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes > [2] https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html#dealing-with-kernel-version-and-configuration-differences > > The short version is: CO-RE relocation handling logic in libbpf > ignores suffixes of form '___something' for type and field names. > > So, the following should accomplish the same as the trick with > #define/#undef: > > #include "vmlinux.h" > ... > struct hid_bpf_ctx___local { > __u32 index; > const struct hid_device *hid; > __u32 allocated_size; > enum hid_report_type report_type; > union { > __s32 retval; > __s32 size; > }; > > }; > ... > extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx, > unsigned int offset, ...) > > However, if the kernel does not have `hid_bpf_ctx` definition would > the test `progs/hid.c` still make sense? > > When I tried to build hid tests locally I run into similar errors: > > ... > CLNG-BPF hid.bpf.o > In file included from progs/hid.c:6: > progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct hid_bpf_ctx' \ > will not be visible outside of this function [-Werror,-Wvisibility] > extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, > ... > > And there is indeed no `hid_bpf_ctx` in my vmlinux.h. > However, after enabling CONFIG_HID_BPF in kernel config the > `hid_bpf_ctx` appears in vmlinux.h, and I can compile HID selftests > w/o issues. Even with enabling this configuration option I was unable to get clean builds of the HID selftests. I proposed a 4th patch on top of Benjamin's n=3 patch series here [1] using the #def/#undef pattern. > > > > > --- > > From bb9eccb7a896ba4b3a35ed12a248e6d6cfed2df6 Mon Sep 17 00:00:00 2001 > > From: Benjamin Tissoires <bentiss@xxxxxxxxxx> > > Date: Fri, 25 Aug 2023 10:02:32 +0200 > > Subject: [PATCH] selftests/hid: ensure we can compile the tests on kernels > > pre-6.3 > > > > For the hid-bpf tests to compile, we need to have the definition of > > struct hid_bpf_ctx. This definition is an internal one from the kernel > > and it is supposed to be defined in the generated vmlinux.h. > > > > This vmlinux.h header is generated based on the currently running kernel > > or if the kernel was already compiled in the tree. If you just compile > > the selftests without compiling the kernel beforehand and you are running > > on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx > > definition. > > > > Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h > > to force the definition of that symbol in case we don't find it in the > > BTF. > > > > Signed-off-by: Benjamin Tissoires <bentiss@xxxxxxxxxx> > > --- > > tools/testing/selftests/hid/progs/hid.c | 3 --- > > .../selftests/hid/progs/hid_bpf_helpers.h | 20 +++++++++++++++++++ > > 2 files changed, 20 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c > > index 88c593f753b5..1e558826b809 100644 > > --- a/tools/testing/selftests/hid/progs/hid.c > > +++ b/tools/testing/selftests/hid/progs/hid.c > > @@ -1,8 +1,5 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* Copyright (c) 2022 Red hat */ > > -#include "vmlinux.h" > > -#include <bpf/bpf_helpers.h> > > -#include <bpf/bpf_tracing.h> > > #include "hid_bpf_helpers.h" > > > > char _license[] SEC("license") = "GPL"; > > diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h > > index 4fff31dbe0e7..749097f8f4d9 100644 > > --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h > > +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h > > @@ -5,6 +5,26 @@ > > #ifndef __HID_BPF_HELPERS_H > > #define __HID_BPF_HELPERS_H > > > > +/* "undefine" structs in vmlinux.h, because we "override" them below */ > > +#define hid_bpf_ctx hid_bpf_ctx___not_used > > +#include "vmlinux.h" > > +#undef hid_bpf_ctx > > + > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > + > > + > > +struct hid_bpf_ctx { > > + __u32 index; > > + const struct hid_device *hid; > > + __u32 allocated_size; > > + enum hid_report_type report_type; > > + union { > > + __s32 retval; > > + __s32 size; > > + }; > > +}; > > + > > /* following are kfuncs exported by HID for HID-BPF */ > > extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, > > unsigned int offset, > [1]: https://lore.kernel.org/all/20230825182316.m2ksjoxe4s7dsapn@xxxxxxxxxx/ Thanks Justin
