Re: [PATCH bpf-next v5 9/9] selftest/bpf: Implement tests for bpftool gen min_core_btf

From: Quentin Monnet <quentin@isovalent.com>
To: "Mauricio Vásquez" <mauricio@kinvolk.io>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Rafael David Tinoco <rafaeldtinoco@gmail.com>,
	Lorenzo Fontana <lorenzo.fontana@elastic.co>,
	Leonardo Di Donato <leonardo.didonato@elastic.co>
Subject: Re: [PATCH bpf-next v5 9/9] selftest/bpf: Implement tests for bpftool gen min_core_btf
Date: Tue, 1 Feb 2022 20:57:52 +0000	[thread overview]
Message-ID: <013a33d5-622e-b503-7a25-c77fbf1be1de@isovalent.com> (raw)
In-Reply-To: <20220128223312.1253169-10-mauricio@kinvolk.io>

2022-01-28 17:33 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> This commit implements some integration tests for "BTFGen". The goal
> of such tests is to verify that the generated BTF file contains the
> expected types.
> 
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
>  tools/testing/selftests/bpf/.gitignore        |   1 +
>  tools/testing/selftests/bpf/Makefile          |   4 +-
>  .../selftests/bpf/progs/btfgen_btf_source.c   |  12 +
>  .../bpf/progs/btfgen_primitives_array.c       |  39 +++
>  .../bpf/progs/btfgen_primitives_struct.c      |  40 +++
>  .../bpf/progs/btfgen_primitives_struct2.c     |  44 ++++
>  .../bpf/progs/btfgen_primitives_union.c       |  32 +++
>  tools/testing/selftests/bpf/test_bpftool.c    | 228 ++++++++++++++++++
>  8 files changed, 399 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/btfgen_btf_source.c
>  create mode 100644 tools/testing/selftests/bpf/progs/btfgen_primitives_array.c
>  create mode 100644 tools/testing/selftests/bpf/progs/btfgen_primitives_struct.c
>  create mode 100644 tools/testing/selftests/bpf/progs/btfgen_primitives_struct2.c
>  create mode 100644 tools/testing/selftests/bpf/progs/btfgen_primitives_union.c
>  create mode 100644 tools/testing/selftests/bpf/test_bpftool.c
> 
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 1dad8d617da8..308cd5b9cfc4 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -41,3 +41,4 @@ test_cpp
>  *.tmp
>  xdpxceiver
>  xdp_redirect_multi
> +test_bpftool
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 945f92d71db3..afc9bff6545d 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -38,7 +38,8 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
>  	test_sock test_sockmap get_cgroup_id_user \
>  	test_cgroup_storage \
>  	test_tcpnotify_user test_sysctl \
> -	test_progs-no_alu32
> +	test_progs-no_alu32 \
> +	test_bpftool

Do you think we could rename this file to something like
“test_bpftool_min_core_btf”? We already have a “test_bpftool.py” and I
fear it might lead to confusion.

>  
>  # Also test bpf-gcc, if present
>  ifneq ($(BPF_GCC),)
> @@ -212,6 +213,7 @@ $(OUTPUT)/xdping: $(TESTING_HELPERS)
>  $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
>  $(OUTPUT)/test_maps: $(TESTING_HELPERS)
>  $(OUTPUT)/test_verifier: $(TESTING_HELPERS)
> +$(OUTPUT)/test_bpftool:
>  
>  BPFTOOL ?= $(DEFAULT_BPFTOOL)
>  $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile)    \

> diff --git a/tools/testing/selftests/bpf/test_bpftool.c b/tools/testing/selftests/bpf/test_bpftool.c
> new file mode 100644
> index 000000000000..ca7facc582d5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_bpftool.c
> @@ -0,0 +1,228 @@
> +// SPDX-License-Identifier: GPL-2.0-only

Not sure if it is intentional to use “GPL-2.0-only” where the other
files in your patch have “GPL-2.0”. But I don't believe it matters much
anyway.