Re: [PATCH v7 bpf-next 8/8] selftests/bpf: add test for bpf_seq_printf_btf helper

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andriin@fb.com>, Yonghong Song <yhs@fb.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	andriy.shevchenko@linux.intel.com, Petr Mladek <pmladek@suse.com>,
	Martin Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	john fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>, Shuah Khan <shuah@kernel.org>,
	Andrey Ignatov <rdna@fb.com>,
	scott.branden@broadcom.com,
	Quentin Monnet <quentin@isovalent.com>,
	Carlos Neira <cneirabustos@gmail.com>,
	Jakub Sitnicki <jakub@cloudflare.com>,
	Ingo Molnar <mingo@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	bpf <bpf@vger.kernel.org>, Networking <netdev@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v7 bpf-next 8/8] selftests/bpf: add test for bpf_seq_printf_btf helper
Date: Mon, 28 Sep 2020 21:42:22 -0700	[thread overview]
Message-ID: <CAEf4Bzb+V-EKgqMFWUGoQLiC_560mCUCdjD4UTPuqnN26-qnvA@mail.gmail.com> (raw)
In-Reply-To: <1601292670-1616-9-git-send-email-alan.maguire@oracle.com>

On Mon, Sep 28, 2020 at 4:36 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Add a test verifying iterating over tasks and displaying BTF
> representation of task_struct succeeds.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---

Hey Alan,

These selftests rely on having struct btf_ptr and BTF_F_COMPACT (and
other) flags to be present in vmlinux.h. While there is nothing wrong
with that per se, it does break selftests builds on older kernels,
because there struct btf_ptr isn't yet present in kernel:

progs/netif_receive_skb.c:131:21: error: use of undeclared identifier
'BTF_F_NONAME'
        TEST_BTF(str, int, BTF_F_NONAME, "0", 0);
                           ^
progs/netif_receive_skb.c:131:2: error: use of undeclared identifier
'BTF_F_COMPACT'; did you mean 'TT_COMPAT'?
        TEST_BTF(str, int, BTF_F_NONAME, "0", 0);
        ^
progs/netif_receive_skb.c:50:28: note: expanded from macro 'TEST_BTF'
                __u64 _hflags = _flags | BTF_F_COMPACT;                 \
                                         ^
/data/users/andriin/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h:330:2:
note: 'TT_COMPAT' declared here
        TT_COMPAT = 2,
        ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]

progs/bpf_iter_task_btf.c:21:24: error: variable has incomplete type
'struct btf_ptr'
        static struct btf_ptr ptr = { };
                              ^
/data/users/andriin/linux/tools/testing/selftests/bpf/tools/include/bpf/bpf_helper_defs.h:33:8:
note: forward declaration of 'struct btf_ptr'
struct btf_ptr;

We actually do build the very latest selftests against old kernels
(4.9 and 5.5 at the moment) as part of libbpf CI, so it would be nice
to fix this problem and keep selftests compilable.

Do you mind following up with a change to define struct btf_ptr and
those BTF_F_xxx flags explicitly for selftests only, similarly to how
we do it for bpf_iter context structs? See progs/bpf_iter.h for
examples. Thanks.

>  tools/testing/selftests/bpf/prog_tests/bpf_iter.c  | 74 ++++++++++++++++++++++
>  .../selftests/bpf/progs/bpf_iter_task_btf.c        | 50 +++++++++++++++
>  2 files changed, 124 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_task_btf.c
>

[...]