From: Björn Töpel bjorn@rivosinc.com
Here's the fourth try. The "make install" target for the BPF selftests are missing a bunch of files, which makes the BPF machine flavor fail (e.g. cpuv4).
This series aims to fix that, but explicitly installing bpftool, all the BPF programs, and the utilities defined by the TRUNNER_EXTRA_PROGS for test_progs.
The fact that this series even have a changelog says a lot, but for those who care: v4: Added bpftool v3: Do not use hardcoded file names (Andrii) v2: Added btf_dump_test_case files
Björn Töpel (3): selftests/bpf: Remove incorrect object path selftests/bpf: Make install target copy test_progs extra files selftests/bpf: Make install target copy bpftool
tools/testing/selftests/bpf/Makefile | 31 +++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-)
base-commit: beb53f32698ff9cd0ca442c1f856ea0ecfb82be3
From: Björn Töpel bjorn@rivosinc.com
The glob path, progs/*.bpf.o, is no longer a correct path. The BPF object files reside in the top directory, or in the per-flavor directory (e.g. cpuv4).
Remove the incorrect object path.
Signed-off-by: Björn Töpel bjorn@rivosinc.com --- tools/testing/selftests/bpf/Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index fd15017ed3b1..830a34f0aa37 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -612,8 +612,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \ $(OUTPUT)/uprobe_multi \ ima_setup.sh \ verify_sig_setup.sh \ - $(wildcard progs/btf_dump_test_case_*.c) \ - $(wildcard progs/*.bpf.o) + $(wildcard progs/btf_dump_test_case_*.c) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS $(eval $(call DEFINE_TEST_RUNNER,test_progs))
From: Björn Töpel bjorn@rivosinc.com
Currently, "make install" does not install the required test_progs "extra files" (e.g. kernel modules, helper shell scripts, etc.) for the BPF machine flavors (e.g. cpuv4).
Add the missing "extra files" dependencies to rsync, called from the install target.
Unfortunately, kselftest does not use bash as the default shell, so the globbering is limited. Blindly enabling "SHELL:=/bin/bash" for the Makefile breaks in other places. Workaround by explicitly call "/bin/bash" to expand the file globbing.
Signed-off-by: Björn Töpel bjorn@rivosinc.com --- tools/testing/selftests/bpf/Makefile | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 830a34f0aa37..c3c5b85f7dae 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -605,14 +605,15 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \ json_writer.c \ flow_dissector_load.h \ ip_check_defrag_frags.h -TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \ - $(OUTPUT)/liburandom_read.so \ - $(OUTPUT)/xdp_synproxy \ - $(OUTPUT)/sign-file \ - $(OUTPUT)/uprobe_multi \ - ima_setup.sh \ - verify_sig_setup.sh \ - $(wildcard progs/btf_dump_test_case_*.c) +TRUNNER_PROGS_EXTRA_FILES:= $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \ + $(OUTPUT)/liburandom_read.so \ + $(OUTPUT)/xdp_synproxy \ + $(OUTPUT)/sign-file \ + $(OUTPUT)/uprobe_multi \ + ima_setup.sh \ + verify_sig_setup.sh \ + $(wildcard progs/btf_dump_test_case_*.c) +TRUNNER_EXTRA_FILES := $(TRUNNER_PROGS_EXTRA_FILES) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS $(eval $(call DEFINE_TEST_RUNNER,test_progs)) @@ -740,11 +741,17 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \ # Delete partially updated (corrupted) files on error .DELETE_ON_ERROR:
+space := $(subst ,, ) +comma := , +EXTRA_FILES_GLOB := {$(subst $(space),$(comma),$(notdir $(TRUNNER_PROGS_EXTRA_FILES)))} DEFAULT_INSTALL_RULE := $(INSTALL_RULE) override define INSTALL_RULE $(DEFAULT_INSTALL_RULE) - @for DIR in $(TEST_INST_SUBDIRS); do \ - mkdir -p $(INSTALL_PATH)/$$DIR; \ - rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR;\ + @for DIR in $(TEST_INST_SUBDIRS); do \ + mkdir -p $(INSTALL_PATH)/$$DIR; \ + rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR; \ + rsync -a --copy-unsafe-links \ + $$(/bin/bash -c "echo $(OUTPUT)/$$DIR/$(EXTRA_FILES_GLOB)") \ + $(INSTALL_PATH)/$$DIR; \ done endef
On Sun, Jan 28, 2024 at 11:09 PM Björn Töpel bjorn@kernel.org wrote:
From: Björn Töpel bjorn@rivosinc.com
Currently, "make install" does not install the required test_progs "extra files" (e.g. kernel modules, helper shell scripts, etc.) for the BPF machine flavors (e.g. cpuv4).
Add the missing "extra files" dependencies to rsync, called from the install target.
Unfortunately, kselftest does not use bash as the default shell, so the globbering is limited. Blindly enabling "SHELL:=/bin/bash" for the Makefile breaks in other places. Workaround by explicitly call "/bin/bash" to expand the file globbing.
Signed-off-by: Björn Töpel bjorn@rivosinc.com
tools/testing/selftests/bpf/Makefile | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 830a34f0aa37..c3c5b85f7dae 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -605,14 +605,15 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \ json_writer.c \ flow_dissector_load.h \ ip_check_defrag_frags.h -TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/liburandom_read.so \$(OUTPUT)/xdp_synproxy \$(OUTPUT)/sign-file \$(OUTPUT)/uprobe_multi \ima_setup.sh \verify_sig_setup.sh \$(wildcard progs/btf_dump_test_case_*.c)+TRUNNER_PROGS_EXTRA_FILES:= $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/liburandom_read.so \$(OUTPUT)/xdp_synproxy \$(OUTPUT)/sign-file \$(OUTPUT)/uprobe_multi \ima_setup.sh \verify_sig_setup.sh \$(wildcard progs/btf_dump_test_case_*.c)+TRUNNER_EXTRA_FILES := $(TRUNNER_PROGS_EXTRA_FILES) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS $(eval $(call DEFINE_TEST_RUNNER,test_progs)) @@ -740,11 +741,17 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \ # Delete partially updated (corrupted) files on error .DELETE_ON_ERROR:
+space := $(subst ,, ) +comma := , +EXTRA_FILES_GLOB := {$(subst $(space),$(comma),$(notdir $(TRUNNER_PROGS_EXTRA_FILES)))} DEFAULT_INSTALL_RULE := $(INSTALL_RULE) override define INSTALL_RULE $(DEFAULT_INSTALL_RULE)
@for DIR in $(TEST_INST_SUBDIRS); do \mkdir -p $(INSTALL_PATH)/$$DIR; \rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR;\
@for DIR in $(TEST_INST_SUBDIRS); do \mkdir -p $(INSTALL_PATH)/$$DIR; \rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR; \rsync -a --copy-unsafe-links \$$(/bin/bash -c "echo $(OUTPUT)/$$DIR/$(EXTRA_FILES_GLOB)") \
this feels quite hacky... have you tried using $(foreach) to go over each element of TRUNNER_PROGS_EXTRA_FILES and append $(OUTPUT)/$$DIR/ to each one? Hopefully that will allow us to get rid of space and comma hacks?
I'm also wondering if it would be ok to just combine two rsync calls into one, so that $(INSTALL_PATH)/$$DIR is specified once?
$(INSTALL_PATH)/$$DIR; \ doneendef
2.40.1
Andrii Nakryiko andrii.nakryiko@gmail.com writes:
On Sun, Jan 28, 2024 at 11:09 PM Björn Töpel bjorn@kernel.org wrote:
From: Björn Töpel bjorn@rivosinc.com
Currently, "make install" does not install the required test_progs "extra files" (e.g. kernel modules, helper shell scripts, etc.) for the BPF machine flavors (e.g. cpuv4).
Add the missing "extra files" dependencies to rsync, called from the install target.
Unfortunately, kselftest does not use bash as the default shell, so the globbering is limited. Blindly enabling "SHELL:=/bin/bash" for the Makefile breaks in other places. Workaround by explicitly call "/bin/bash" to expand the file globbing.
Signed-off-by: Björn Töpel bjorn@rivosinc.com
tools/testing/selftests/bpf/Makefile | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 830a34f0aa37..c3c5b85f7dae 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -605,14 +605,15 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \ json_writer.c \ flow_dissector_load.h \ ip_check_defrag_frags.h -TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/liburandom_read.so \$(OUTPUT)/xdp_synproxy \$(OUTPUT)/sign-file \$(OUTPUT)/uprobe_multi \ima_setup.sh \verify_sig_setup.sh \$(wildcard progs/btf_dump_test_case_*.c)+TRUNNER_PROGS_EXTRA_FILES:= $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/liburandom_read.so \$(OUTPUT)/xdp_synproxy \$(OUTPUT)/sign-file \$(OUTPUT)/uprobe_multi \ima_setup.sh \verify_sig_setup.sh \$(wildcard progs/btf_dump_test_case_*.c)+TRUNNER_EXTRA_FILES := $(TRUNNER_PROGS_EXTRA_FILES) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS $(eval $(call DEFINE_TEST_RUNNER,test_progs)) @@ -740,11 +741,17 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \ # Delete partially updated (corrupted) files on error .DELETE_ON_ERROR:
+space := $(subst ,, ) +comma := , +EXTRA_FILES_GLOB := {$(subst $(space),$(comma),$(notdir $(TRUNNER_PROGS_EXTRA_FILES)))} DEFAULT_INSTALL_RULE := $(INSTALL_RULE) override define INSTALL_RULE $(DEFAULT_INSTALL_RULE)
@for DIR in $(TEST_INST_SUBDIRS); do \mkdir -p $(INSTALL_PATH)/$$DIR; \rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR;\
@for DIR in $(TEST_INST_SUBDIRS); do \mkdir -p $(INSTALL_PATH)/$$DIR; \rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR; \rsync -a --copy-unsafe-links \$$(/bin/bash -c "echo $(OUTPUT)/$$DIR/$(EXTRA_FILES_GLOB)") \this feels quite hacky... have you tried using $(foreach) to go over each element of TRUNNER_PROGS_EXTRA_FILES and append $(OUTPUT)/$$DIR/ to each one? Hopefully that will allow us to get rid of space and comma hacks?
I haven't -- I'll try that out. Another ugliness is that *.bpf.o and bpftool has to be explicitly specified, which hints of unrobustness.
I'm also wondering if it would be ok to just combine two rsync calls into one, so that $(INSTALL_PATH)/$$DIR is specified once?
FWIW, I fold the two calls in patch 3. Regardless; I'll try to make the whole series more palatable.
Björn
From: Björn Töpel bjorn@rivosinc.com
bpftool is required for some test_progs tests. Make sure it is installed as part of the install target.
Signed-off-by: Björn Töpel bjorn@rivosinc.com --- tools/testing/selftests/bpf/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index c3c5b85f7dae..ebbb40ef5702 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -749,8 +749,9 @@ override define INSTALL_RULE $(DEFAULT_INSTALL_RULE) @for DIR in $(TEST_INST_SUBDIRS); do \ mkdir -p $(INSTALL_PATH)/$$DIR; \ - rsync -a $(OUTPUT)/$$DIR/*.bpf.o $(INSTALL_PATH)/$$DIR; \ rsync -a --copy-unsafe-links \ + $(OUTPUT)/$$DIR/*.bpf.o \ + $(OUTPUT)/$$DIR/bpftool \ $$(/bin/bash -c "echo $(OUTPUT)/$$DIR/$(EXTRA_FILES_GLOB)") \ $(INSTALL_PATH)/$$DIR; \ done
linux-kselftest-mirror@lists.linaro.org
-
Andrii Nakryiko -
Björn Töpel