[bpf-next,1/2] selftests: bpf: Fix incorrect kernel headers search path

Commit Message

Mathieu Desnoyers Jan. 30, 2023, 6:35 p.m. UTC

Use $(KHDR_INCLUDES) as lookup path for kernel headers. This prevents
building against kernel headers from the build environment in scenarios
where kernel headers are installed into a specific output directory
(O=...).

Signed-off-by: Mathieu Desnoyers <[e-mail]>
Acked-by: Shuah Khan <[e-mail]>
Cc: <[e-mail]>
Cc: Shuah Khan <[e-mail]>
Cc: [e-mail]
Cc: Ingo Molnar <[e-mail]>
Cc: Alexei Starovoitov <[e-mail]>
Cc: Daniel Borkmann <[e-mail]>
Cc: Andrii Nakryiko <[e-mail]>
Cc: Martin KaFai Lau <[e-mail]>
Cc: Song Liu <[e-mail]>
Cc: Yonghong Song <[e-mail]>
Cc: John Fastabend <[e-mail]>
Cc: KP Singh <[e-mail]>
Cc: Stanislav Fomichev <[e-mail]>
Cc: Hao Luo <[e-mail]>
Cc: Jiri Olsa <[e-mail]>
Cc: Mykola Lysenko <[e-mail]>
Cc: <[e-mail]>    [5.18+]
---
 tools/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alexei Starovoitov Jan. 30, 2023, 9:03 p.m. UTC | #1

On Mon, Jan 30, 2023 at 10:36 AM Mathieu Desnoyers
<[e-mail]> wrote:
>
> Use $(KHDR_INCLUDES) as lookup path for kernel headers. This prevents
> building against kernel headers from the build environment in scenarios
> where kernel headers are installed into a specific output directory
> (O=...).
>
> Signed-off-by: Mathieu Desnoyers <[e-mail]>
> Acked-by: Shuah Khan <[e-mail]>
> Cc: <[e-mail]>
> Cc: Shuah Khan <[e-mail]>
> Cc: [e-mail]
> Cc: Ingo Molnar <[e-mail]>
> Cc: Alexei Starovoitov <[e-mail]>
> Cc: Daniel Borkmann <[e-mail]>
> Cc: Andrii Nakryiko <[e-mail]>
> Cc: Martin KaFai Lau <[e-mail]>
> Cc: Song Liu <[e-mail]>
> Cc: Yonghong Song <[e-mail]>
> Cc: John Fastabend <[e-mail]>
> Cc: KP Singh <[e-mail]>
> Cc: Stanislav Fomichev <[e-mail]>
> Cc: Hao Luo <[e-mail]>
> Cc: Jiri Olsa <[e-mail]>
> Cc: Mykola Lysenko <[e-mail]>
> Cc: <[e-mail]>    [5.18+]
> ---
>  tools/testing/selftests/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index c22c43bbee19..6998c816afef 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -327,7 +327,7 @@ endif
>  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
>  BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)          \
>              -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR)                   \
> -            -I$(abspath $(OUTPUT)/../usr/include)
> +            $(KHDR_INCLUDES)

It breaks the build:
https://github.com/kernel-patches/bpf/actions/runs/4047075637/jobs/6960655246

make[1]: *** No rule to make target '/linux/bpf.h', needed by
'/tmp/work/bpf/bpf/tools/testing/selftests/bpf/bpf-helpers.rst'. Stop.

Mathieu Desnoyers Jan. 31, 2023, 3:17 p.m. UTC | #2

On 2023-01-30 16:03, Alexei Starovoitov wrote:
> On Mon, Jan 30, 2023 at 10:36 AM Mathieu Desnoyers
> <[e-mail]> wrote:
>>
>> Use $(KHDR_INCLUDES) as lookup path for kernel headers. This prevents
>> building against kernel headers from the build environment in scenarios
>> where kernel headers are installed into a specific output directory
>> (O=...).
>>
>> Signed-off-by: Mathieu Desnoyers <[e-mail]>
>> Acked-by: Shuah Khan <[e-mail]>
>> Cc: <[e-mail]>
>> Cc: Shuah Khan <[e-mail]>
>> Cc: [e-mail]
>> Cc: Ingo Molnar <[e-mail]>
>> Cc: Alexei Starovoitov <[e-mail]>
>> Cc: Daniel Borkmann <[e-mail]>
>> Cc: Andrii Nakryiko <[e-mail]>
>> Cc: Martin KaFai Lau <[e-mail]>
>> Cc: Song Liu <[e-mail]>
>> Cc: Yonghong Song <[e-mail]>
>> Cc: John Fastabend <[e-mail]>
>> Cc: KP Singh <[e-mail]>
>> Cc: Stanislav Fomichev <[e-mail]>
>> Cc: Hao Luo <[e-mail]>
>> Cc: Jiri Olsa <[e-mail]>
>> Cc: Mykola Lysenko <[e-mail]>
>> Cc: <[e-mail]>    [5.18+]
>> ---
>>   tools/testing/selftests/bpf/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index c22c43bbee19..6998c816afef 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -327,7 +327,7 @@ endif
>>   CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
>>   BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)          \
>>               -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR)                   \
>> -            -I$(abspath $(OUTPUT)/../usr/include)
>> +            $(KHDR_INCLUDES)
> 
> It breaks the build:
> https://github.com/kernel-patches/bpf/actions/runs/4047075637/jobs/6960655246
> 
> make[1]: *** No rule to make target '/linux/bpf.h', needed by
> '/tmp/work/bpf/bpf/tools/testing/selftests/bpf/bpf-helpers.rst'. Stop.

I finally figured out why I did not catch it in my own testing: it appears
that the bpf selftest is special: it's not built when issuing "make" from
tools/testing/selftests/. Perhaps that's also why Shuah did not see any
issues with my bpf patches:

tools/testing/selftests/Makefile:

# User can optionally provide a TARGETS skiplist.  By default we skip
# BPF since it has cutting edge build time dependencies which require
# more effort to install.
SKIP_TARGETS ?= bpf

It appears that the baseline bpf selftest on v6.2-rc6 does not build on
my system:

   CLNG-BPF [test_maps] test_bpf_nf.bpf.o
progs/test_bpf_nf.c:156:7: error: no member named 'mark' in 'struct nf_conn'
                 ct->mark = 77;
                 ~~  ^
progs/test_bpf_nf.c:160:42: error: use of undeclared identifier 'NF_NAT_MANIP_SRC'
                 bpf_ct_set_nat_info(ct, &saddr, sport, NF_NAT_MANIP_SRC);
                                                        ^
progs/test_bpf_nf.c:163:42: error: use of undeclared identifier 'NF_NAT_MANIP_DST'
                 bpf_ct_set_nat_info(ct, &daddr, dport, NF_NAT_MANIP_DST);
                                                        ^
progs/test_bpf_nf.c:187:38: error: no member named 'mark' in 'struct nf_conn'
                                 test_insert_lookup_mark = ct_lk->mark;
                                                           ~~~~~  ^
progs/test_bpf_nf.c:189:12: error: use of undeclared identifier 'IPS_CONFIRMED'
                                                      IPS_CONFIRMED | IPS_SEEN_REPLY);
                                                      ^
progs/test_bpf_nf.c:189:28: error: use of undeclared identifier 'IPS_SEEN_REPLY'
                                                      IPS_CONFIRMED | IPS_SEEN_REPLY);
                                                                      ^
progs/test_bpf_nf.c:209:11: error: no member named 'mark' in 'struct nf_conn'
                 if (ct->mark == 42) {
                     ~~  ^
progs/test_bpf_nf.c:210:8: error: no member named 'mark' in 'struct nf_conn'
                         ct->mark++;
                         ~~  ^
progs/test_bpf_nf.c:211:33: error: no member named 'mark' in 'struct nf_conn'
                         test_exist_lookup_mark = ct->mark;
                                                  ~~  ^
9 errors generated.

Based on this:
https://lore.kernel.org/bpf/[e-mail]/

It appears that NF_CONNTRACK_MARK should be set to =y. Perhaps it should be
detected at selftest build time and skipped rather than failing to build ? It
appears to be a case where a user-space selftest depends on internal kernel data
structures:

Ref. include/net/netfilter/nf_conntrack.h:struct nf_conn

After setting this config option, it fails at:

   GEN-SKEL [test_progs] test_kfunc_dynptr_param.skel.h
libbpf: sec '?lsm.s/bpf': corrupted program 'not_ptr_to_stack', offset 48, size 0
Error: failed to open BPF object file: Invalid argument
make: *** [Makefile:546: /home/efficios/git/linux/tools/testing/selftests/bpf/test_kfunc_dynptr_param.skel.h] Error 234
make: *** Deleting file '/home/efficios/git/linux/tools/testing/selftests/bpf/test_kfunc_dynptr_param.skel.h'

Based on this issue: https://github.com/libbpf/libbpf-bootstrap/issues/12 it appears
that bpf selftests are only meant to be used against a set of pre-specified kernel configurations.
README.rst confirms that. So let's use tools/testing/selftests/bpf/config.x86_64
with make olddefconfig for my kernel build then. It would have been less unexpected for the
bpf selftests to report incorrect or missing kernel config options rather than limiting the
supported set to specific configuration files provided by the bpf selftests. This limits
the testing coverage to very few kernel configurations.

It still does not work. I need to explicitly enable BPF_SYSCALL=y which is not present in
the bpf selftests config.x86_64. Then I can explicitly reenable DEBUG_INFO_BTF=y. It appears
that config.x86_64 is outdated in the bpf selftests. It also misses NF_CONNTRACK_MARK=y.
So let's assume that config.x86_64 is outdated and broken and go with
tools/testing/selftests/bpf/config followed by make olddefconfig.

No more luck, this time the bpf kernel config misses the DEBUG_KERNEL=y, which is needed
for DEBUG_INFO and DEBUG_INFO_BTF.

Now it's:

   CLANG   /home/efficios/git/linux/tools/testing/selftests/bpf/tools/build/bpftool/profiler.bpf.o
skeleton/profiler.bpf.c:18:21: error: invalid application of 'sizeof' to an incomplete type 'struct bpf_perf_event_value'
         __uint(value_size, sizeof(struct bpf_perf_event_value));
[...]

And that's where I give up. Has anyone else succeeded in building bpf selftests locally ?

Thanks,

Mathieu

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index c22c43bbee19..6998c816afef 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -327,7 +327,7 @@  endif
 CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
 BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) 		\
 	     -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR)			\
-	     -I$(abspath $(OUTPUT)/../usr/include)
+	     $(KHDR_INCLUDES)
 
 CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
 	       -Wno-compare-distinct-pointer-types