linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 0/3] Fix make install target for BPF selftests
@ 2024-01-29  7:09 Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 1/3] selftests/bpf: Remove incorrect object path Björn Töpel
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Björn Töpel @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev
  Cc: Björn Töpel, linux-kselftest, linux-kernel

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
-- 
2.40.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 1/3] selftests/bpf: Remove incorrect object path
  2024-01-29  7:09 [PATCH bpf-next v4 0/3] Fix make install target for BPF selftests Björn Töpel
@ 2024-01-29  7:09 ` Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 3/3] selftests/bpf: Make install target copy bpftool Björn Töpel
  2 siblings, 0 replies; 6+ messages in thread
From: Björn Töpel @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev
  Cc: Björn Töpel, linux-kselftest, linux-kernel

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))
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files
  2024-01-29  7:09 [PATCH bpf-next v4 0/3] Fix make install target for BPF selftests Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 1/3] selftests/bpf: Remove incorrect object path Björn Töpel
@ 2024-01-29  7:09 ` Björn Töpel
  2024-01-30  0:59   ` Andrii Nakryiko
  2024-01-29  7:09 ` [PATCH bpf-next v4 3/3] selftests/bpf: Make install target copy bpftool Björn Töpel
  2 siblings, 1 reply; 6+ messages in thread
From: Björn Töpel @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev
  Cc: Björn Töpel, linux-kselftest, linux-kernel

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
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next v4 3/3] selftests/bpf: Make install target copy bpftool
  2024-01-29  7:09 [PATCH bpf-next v4 0/3] Fix make install target for BPF selftests Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 1/3] selftests/bpf: Remove incorrect object path Björn Töpel
  2024-01-29  7:09 ` [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files Björn Töpel
@ 2024-01-29  7:09 ` Björn Töpel
  2 siblings, 0 replies; 6+ messages in thread
From: Björn Töpel @ 2024-01-29  7:09 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev
  Cc: Björn Töpel, linux-kselftest, linux-kernel

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
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files
  2024-01-29  7:09 ` [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files Björn Töpel
@ 2024-01-30  0:59   ` Andrii Nakryiko
  2024-01-30  6:24     ` Björn Töpel
  0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2024-01-30  0:59 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev, Björn Töpel,
	linux-kselftest, linux-kernel

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;                                          \
>         done
>  endef
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files
  2024-01-30  0:59   ` Andrii Nakryiko
@ 2024-01-30  6:24     ` Björn Töpel
  0 siblings, 0 replies; 6+ messages in thread
From: Björn Töpel @ 2024-01-30  6:24 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Mykola Lysenko, bpf, netdev, Björn Töpel,
	linux-kselftest, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-30  6:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-29  7:09 [PATCH bpf-next v4 0/3] Fix make install target for BPF selftests Björn Töpel
2024-01-29  7:09 ` [PATCH bpf-next v4 1/3] selftests/bpf: Remove incorrect object path Björn Töpel
2024-01-29  7:09 ` [PATCH bpf-next v4 2/3] selftests/bpf: Make install target copy test_progs extra files Björn Töpel
2024-01-30  0:59   ` Andrii Nakryiko
2024-01-30  6:24     ` Björn Töpel
2024-01-29  7:09 ` [PATCH bpf-next v4 3/3] selftests/bpf: Make install target copy bpftool Björn Töpel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).