2023-01-30 18:36:13

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH bpf-next 1/2] selftests: bpf: Fix incorrect kernel headers search path

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 <[email protected]>
Acked-by: Shuah Khan <[email protected]>
Cc: <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: [email protected]
Cc: Ingo Molnar <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: Martin KaFai Lau <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Yonghong Song <[email protected]>
Cc: John Fastabend <[email protected]>
Cc: KP Singh <[email protected]>
Cc: Stanislav Fomichev <[email protected]>
Cc: Hao Luo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mykola Lysenko <[email protected]>
Cc: <[email protected]> [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)

CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
-Wno-compare-distinct-pointer-types
--
2.25.1



2023-01-30 18:36:19

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH bpf-next 2/2] selftests: bpf docs: Use installed kernel headers search path

Use $(KHDR_INCLUDES) as lookup path for installed kernel headers rather
than using kernel headers in include/uapi from the source kernel tree
kernel headers.

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

diff --git a/tools/testing/selftests/bpf/Makefile.docs b/tools/testing/selftests/bpf/Makefile.docs
index eb6a4fea8c79..0a538d873def 100644
--- a/tools/testing/selftests/bpf/Makefile.docs
+++ b/tools/testing/selftests/bpf/Makefile.docs
@@ -44,7 +44,7 @@ RST2MAN_DEP := $(shell command -v rst2man 2>/dev/null)
# $1 - target for scripts/bpf_doc.py
# $2 - man page section to generate the troff file
define DOCS_RULES =
-$(OUTPUT)bpf-$1.rst: ../../../../include/uapi/linux/bpf.h
+$(OUTPUT)bpf-$1.rst: $(KHDR_INCLUDES)/linux/bpf.h
$$(QUIET_GEN)../../../../scripts/bpf_doc.py $1 \
--filename $$< > $$@

--
2.25.1


2023-01-30 21:04:39

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] selftests: bpf: Fix incorrect kernel headers search path

On Mon, Jan 30, 2023 at 10:36 AM Mathieu Desnoyers
<[email protected]> 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 <[email protected]>
> Acked-by: Shuah Khan <[email protected]>
> Cc: <[email protected]>
> Cc: Shuah Khan <[email protected]>
> Cc: [email protected]
> Cc: Ingo Molnar <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Martin KaFai Lau <[email protected]>
> Cc: Song Liu <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: John Fastabend <[email protected]>
> Cc: KP Singh <[email protected]>
> Cc: Stanislav Fomichev <[email protected]>
> Cc: Hao Luo <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Mykola Lysenko <[email protected]>
> Cc: <[email protected]> [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.

2023-01-31 15:19:22

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH bpf-next 1/2] selftests: bpf: Fix incorrect kernel headers search path

On 2023-01-30 16:03, Alexei Starovoitov wrote:
> On Mon, Jan 30, 2023 at 10:36 AM Mathieu Desnoyers
> <[email protected]> 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 <[email protected]>
>> Acked-by: Shuah Khan <[email protected]>
>> Cc: <[email protected]>
>> Cc: Shuah Khan <[email protected]>
>> Cc: [email protected]
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Alexei Starovoitov <[email protected]>
>> Cc: Daniel Borkmann <[email protected]>
>> Cc: Andrii Nakryiko <[email protected]>
>> Cc: Martin KaFai Lau <[email protected]>
>> Cc: Song Liu <[email protected]>
>> Cc: Yonghong Song <[email protected]>
>> Cc: John Fastabend <[email protected]>
>> Cc: KP Singh <[email protected]>
>> Cc: Stanislav Fomichev <[email protected]>
>> Cc: Hao Luo <[email protected]>
>> Cc: Jiri Olsa <[email protected]>
>> Cc: Mykola Lysenko <[email protected]>
>> Cc: <[email protected]> [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/[email protected]/

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

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com