mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>, bpf <bpf@vger.kernel.org>,
	Shuah Khan <shuah@kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Mykola Lysenko <mykolal@fb.com>,
	stable <stable@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] selftests: bpf: Fix incorrect kernel headers search path
Date: Tue, 31 Jan 2023 10:17:05 -0500	[thread overview]
Message-ID: <edad4f4d-f19e-34dc-06f5-7d30fd5d058f@efficios.com> (raw)
In-Reply-To: <CAADnVQLve5je3GKesCjn5HhuxgOe2u1OX7GQnx-58dsuUFWbZQ@mail.gmail.com>

On 2023-01-30 16:03, Alexei Starovoitov wrote:
> On Mon, Jan 30, 2023 at 10:36 AM Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> 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 <mathieu.desnoyers@efficios.com>
>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>> Cc: <bpf@vger.kernel.org>
>> Cc: Shuah Khan <shuah@kernel.org>
>> Cc: linux-kselftest@vger.kernel.org
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Andrii Nakryiko <andrii@kernel.org>
>> Cc: Martin KaFai Lau <martin.lau@linux.dev>
>> Cc: Song Liu <song@kernel.org>
>> Cc: Yonghong Song <yhs@fb.com>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Cc: KP Singh <kpsingh@kernel.org>
>> Cc: Stanislav Fomichev <sdf@google.com>
>> Cc: Hao Luo <haoluo@google.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Mykola Lysenko <mykolal@fb.com>
>> Cc: <stable@vger.kernel.org>    [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/37649bee-5eb3-93a2-ac57-56eb375ef8cd@iogearbox.net/

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


      reply	other threads:[~2023-01-31 15:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-30 18:35 Mathieu Desnoyers
2023-01-30 18:35 ` [PATCH bpf-next 2/2] selftests: bpf docs: Use installed " Mathieu Desnoyers
2023-01-30 21:03 ` [PATCH bpf-next 1/2] selftests: bpf: Fix incorrect " Alexei Starovoitov
2023-01-31 15:17   ` Mathieu Desnoyers [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edad4f4d-f19e-34dc-06f5-7d30fd5d058f@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mingo@redhat.com \
    --cc=mykolal@fb.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=song@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®