Re: [PATCH bpf-next v2 08/15] selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing

From: Fangrui Song <maskray@google.com>
To: Yonghong Song <yhs@fb.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	 Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com
Subject: Re: [PATCH bpf-next v2 08/15] selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing
Date: Wed, 12 Jul 2023 23:18:12 -0700	[thread overview]
Message-ID: <CAFP8O3L04X_c2wD0pg7Av24K107SFeCUm+-xtCs30EbQag5xOQ@mail.gmail.com> (raw)
In-Reply-To: <20230713060800.392500-1-yhs@fb.com>

On Wed, Jul 12, 2023 at 11:11 PM Yonghong Song <yhs@fb.com> wrote:
>
> Similar to no-alu32 runner, a cpuv4 runner is created to test
> bpf programs compiled with -mcpu=v4.
>
> The following are some num-of-insn statistics for each newer
> instructions, excluding naked asm (verifier_*) tests:
>    insn pattern                # of instructions
>    reg = (s8)reg               4
>    reg = (s16)reg              2
>    reg = (s32)reg              26
>    reg = *(s8 *)(reg + off)    11
>    reg = *(s16 *)(reg + off)   14
>    reg = *(s32 *)(reg + off)   15214
>    reg = bswap16 reg           133
>    reg = bswap32 reg           38
>    reg = bswap64 reg           14
>    reg s/= reg                 0
>    reg s%= reg                 0
>    gotol <offset>              58
>
> Note that in llvm -mcpu=v4 implementation, the compiler is a little
> bit conservative about generating 'gotol' insn (32-bit branch offset)
> as it didn't precise count the number of insns (e.g., some insns are
> debug insns, etc.). Compared to old 'goto' insn, newer 'gotol' insn
> should have comparable verification states to 'goto' insn.
>
> I did not collect verifier stats now since I have not really
> started to do proper range bound estimation with these
> instructions.
>
> With current patch set, all selftests passed with -mcpu=v4
> when running test_progs-cpuv4 binary.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/.gitignore |  2 ++
>  tools/testing/selftests/bpf/Makefile   | 18 ++++++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 116fecf80ca1..110518ba4804 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -13,6 +13,7 @@ test_dev_cgroup
>  /test_progs
>  /test_progs-no_alu32
>  /test_progs-bpf_gcc
> +/test_progs-cpuv4
>  test_verifier_log
>  feature
>  test_sock
> @@ -36,6 +37,7 @@ test_cpp
>  *.lskel.h
>  /no_alu32
>  /bpf_gcc
> +/cpuv4
>  /host-tools
>  /tools
>  /runqslower
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 882be03b179f..4b2cf5d40120 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -44,7 +44,7 @@ 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_progs-cpuv4
>
>  # Also test bpf-gcc, if present
>  ifneq ($(BPF_GCC),)
> @@ -383,6 +383,11 @@ define CLANG_NOALU32_BPF_BUILD_RULE
>         $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
>         $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v2 -o $2
>  endef
> +# Similar to CLANG_BPF_BUILD_RULE, but with cpu-v4
> +define CLANG_CPUV4_BPF_BUILD_RULE
> +       $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> +       $(Q)$(CLANG) $3 -O2 -target bpf -c $1 -mcpu=v4 -o $2
> +endef

Use --target= (Clang 3.4 preferred form) for new code :)

>  # Build BPF object using GCC
>  define GCC_BPF_BUILD_RULE
>         $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> @@ -425,7 +430,7 @@ LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.c,$(foreach skel,$(LINKED_SKELS),$($(ske
>  # $eval()) and pass control to DEFINE_TEST_RUNNER_RULES.
>  # Parameters:
>  # $1 - test runner base binary name (e.g., test_progs)
> -# $2 - test runner extra "flavor" (e.g., no_alu32, gcc-bpf, etc)
> +# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, gcc-bpf, etc)
>  define DEFINE_TEST_RUNNER
>
>  TRUNNER_OUTPUT := $(OUTPUT)$(if $2,/)$2
> @@ -453,7 +458,7 @@ endef
>  # Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and
>  # set up by DEFINE_TEST_RUNNER itself, create test runner build rules with:
>  # $1 - test runner base binary name (e.g., test_progs)
> -# $2 - test runner extra "flavor" (e.g., no_alu32, gcc-bpf, etc)
> +# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, gcc-bpf, etc)
>  define DEFINE_TEST_RUNNER_RULES
>
>  ifeq ($($(TRUNNER_OUTPUT)-dir),)
> @@ -584,6 +589,11 @@ TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE
>  TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
>  $(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32))
>
> +# Define test_progs-cpuv4 test runner.
> +TRUNNER_BPF_BUILD_RULE := CLANG_CPUV4_BPF_BUILD_RULE
> +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
> +$(eval $(call DEFINE_TEST_RUNNER,test_progs,cpuv4))
> +
>  # Define test_progs BPF-GCC-flavored test runner.
>  ifneq ($(BPF_GCC),)
>  TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE
> @@ -681,7 +691,7 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)      \
>         prog_tests/tests.h map_tests/tests.h verifier/tests.h           \
>         feature bpftool                                                 \
>         $(addprefix $(OUTPUT)/,*.o *.skel.h *.lskel.h *.subskel.h       \
> -                              no_alu32 bpf_gcc bpf_testmod.ko          \
> +                              no_alu32 cpuv4 bpf_gcc bpf_testmod.ko    \
>                                liburandom_read.so)
>
>  .PHONY: docs docs-clean
> --
> 2.34.1
>


-- 
宋方睿