[PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output

Linux Kernel Selftest development
 help / color / mirror / Atom feed

* [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
       [not found] <CAEf4BzarKiUZqNcq1E+6SaeG8oP5+SfSLLoTNKF3_+7MS6CtyQ@mail.gmail.com>
@ 2024-07-19 23:21 ` Tony Ambardar
  2024-07-20  1:23   ` Eduard Zingerman
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Tony Ambardar @ 2024-07-19 23:21 UTC (permalink / raw)
  To: bpf
  Cc: Tony Ambardar, linux-kselftest, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
	Shuah Khan

Make log output incorrectly shows 'test_maps' as the binary name for every
'CLNG-BPF' build step, apparently picking up the last value defined for the
$(TRUNNER_BINARY) variable. Update the 'CLANG_BPF_BUILD_RULE' variants to
fix this confusing output.

Current output:
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h

After fix:
  CLNG-BPF [test_progs] access_map_in_map.bpf.o
  GEN-SKEL [test_progs] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_progs-no_alu32] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_progs-cpuv4] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h

Fixes: a5d0c26a2784 ("selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing")
Fixes: 89ad7420b25c ("selftests/bpf: Drop the need for LLVM's llc")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
 tools/testing/selftests/bpf/Makefile | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 0b4bfbc0ef68..67921e3367dd 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -425,27 +425,28 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
 $(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
 
 # Build BPF object using Clang
-# $1 - input .c file
-# $2 - output .o file
-# $3 - CFLAGS
+# $1 - binary name
+# $2 - input .c file
+# $3 - output .o file
+# $4 - CFLAGS
 define CLANG_BPF_BUILD_RULE
-	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
+	$(call msg,CLNG-BPF,$1,$3)
+	$(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v3 -o $3
 endef
 # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
 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
+	$(call msg,CLNG-BPF,$1,$3)
+	$(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v2 -o $3
 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
+	$(call msg,CLNG-BPF,$1,$3)
+	$(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v4 -o $3
 endef
 # Build BPF object using GCC
 define GCC_BPF_BUILD_RULE
-	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
+	$(call msg,GCC-BPF,$1,$3)
+	$(Q)$(BPF_GCC) $4 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $2 -o $3
 endef
 
 SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
@@ -534,7 +535,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:				\
 		     $(wildcard $(BPFDIR)/bpf_*.h)			\
 		     $(wildcard $(BPFDIR)/*.bpf.h)			\
 		     | $(TRUNNER_OUTPUT) $$(BPFOBJ)
-	$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,			\
+	$$(call $(TRUNNER_BPF_BUILD_RULE),$(TRUNNER_BINARY),$$<,$$@,	\
 					  $(TRUNNER_BPF_CFLAGS)         \
 					  $$($$<-CFLAGS)		\
 					  $$($$<-$2-CFLAGS))
-- 
2.34.1


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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-19 23:21 ` [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output Tony Ambardar
@ 2024-07-20  1:23   ` Eduard Zingerman
  2024-07-20  2:57   ` Andrii Nakryiko
  2024-07-20  5:25   ` [PATCH bpf-next v2] " Tony Ambardar
  2 siblings, 0 replies; 9+ messages in thread
From: Eduard Zingerman @ 2024-07-20  1:23 UTC (permalink / raw)
  To: Tony Ambardar, bpf
  Cc: linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Mykola Lysenko, Shuah Khan

On Fri, 2024-07-19 at 16:21 -0700, Tony Ambardar wrote:

[...]


>  define GCC_BPF_BUILD_RULE
> -	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> -	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> +	$(call msg,GCC-BPF,$1,$3)
> +	$(Q)$(BPF_GCC) $4 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $2 -o $3
>  endef
>  
>  SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
> @@ -534,7 +535,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:				\
>  		     $(wildcard $(BPFDIR)/bpf_*.h)			\
>  		     $(wildcard $(BPFDIR)/*.bpf.h)			\
>  		     | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> -	$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,			\
> +	$$(call $(TRUNNER_BPF_BUILD_RULE),$(TRUNNER_BINARY),$$<,$$@,	\

At first I found it quite confusing that we use TRUNNER_BINARY variable in this define,
but can't use it in the CLANG_BPF_BUILD_RULE and co.
However, it looks like this is because of the eval in the (eval (call ...)) pair,
used to invoke DEFINE_TEST_RUNNER_RULES.

Suggested patch works and is probably the simplest fix.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

>  					  $(TRUNNER_BPF_CFLAGS)         \
>  					  $$($$<-CFLAGS)		\
>  					  $$($$<-$2-CFLAGS))


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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-19 23:21 ` [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output Tony Ambardar
  2024-07-20  1:23   ` Eduard Zingerman
@ 2024-07-20  2:57   ` Andrii Nakryiko
  2024-07-20  5:23     ` Tony Ambardar
  2024-07-20  5:25   ` [PATCH bpf-next v2] " Tony Ambardar
  2 siblings, 1 reply; 9+ messages in thread
From: Andrii Nakryiko @ 2024-07-20  2:57 UTC (permalink / raw)
  To: Tony Ambardar
  Cc: bpf, linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Mykola Lysenko, Shuah Khan

On Fri, Jul 19, 2024 at 4:22 PM Tony Ambardar <tony.ambardar@gmail.com> wrote:
>
> Make log output incorrectly shows 'test_maps' as the binary name for every
> 'CLNG-BPF' build step, apparently picking up the last value defined for the
> $(TRUNNER_BINARY) variable. Update the 'CLANG_BPF_BUILD_RULE' variants to
> fix this confusing output.
>
> Current output:
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h
>
> After fix:
>   CLNG-BPF [test_progs] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_progs-no_alu32] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_progs-cpuv4] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h
>
> Fixes: a5d0c26a2784 ("selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing")
> Fixes: 89ad7420b25c ("selftests/bpf: Drop the need for LLVM's llc")
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 0b4bfbc0ef68..67921e3367dd 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -425,27 +425,28 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
>  $(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
>
>  # Build BPF object using Clang
> -# $1 - input .c file
> -# $2 - output .o file
> -# $3 - CFLAGS
> +# $1 - binary name
> +# $2 - input .c file
> +# $3 - output .o file
> +# $4 - CFLAGS
>  define CLANG_BPF_BUILD_RULE
> -       $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> -       $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
> +       $(call msg,CLNG-BPF,$1,$3)
> +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v3 -o $3

this works, but did you have to renumber all parameters? Let's maybe
pass this binary name as the 4th argument?

pw-bot: cr

>  endef
>  # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
>  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
> +       $(call msg,CLNG-BPF,$1,$3)
> +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v2 -o $3
>  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
> +       $(call msg,CLNG-BPF,$1,$3)
> +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v4 -o $3
>  endef
>  # Build BPF object using GCC
>  define GCC_BPF_BUILD_RULE
> -       $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> -       $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> +       $(call msg,GCC-BPF,$1,$3)
> +       $(Q)$(BPF_GCC) $4 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $2 -o $3
>  endef
>
>  SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
> @@ -534,7 +535,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:                             \
>                      $(wildcard $(BPFDIR)/bpf_*.h)                      \
>                      $(wildcard $(BPFDIR)/*.bpf.h)                      \
>                      | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> -       $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,                      \
> +       $$(call $(TRUNNER_BPF_BUILD_RULE),$(TRUNNER_BINARY),$$<,$$@,    \
>                                           $(TRUNNER_BPF_CFLAGS)         \
>                                           $$($$<-CFLAGS)                \
>                                           $$($$<-$2-CFLAGS))
> --
> 2.34.1
>

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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-20  2:57   ` Andrii Nakryiko
@ 2024-07-20  5:23     ` Tony Ambardar
  2024-07-23  1:35       ` Tony Ambardar
  0 siblings, 1 reply; 9+ messages in thread
From: Tony Ambardar @ 2024-07-20  5:23 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Mykola Lysenko, Shuah Khan

On Fri, Jul 19, 2024 at 07:57:09PM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 19, 2024 at 4:22 PM Tony Ambardar <tony.ambardar@gmail.com> wrote:
> >
> > Make log output incorrectly shows 'test_maps' as the binary name for every
> > 'CLNG-BPF' build step, apparently picking up the last value defined for the
> > $(TRUNNER_BINARY) variable. Update the 'CLANG_BPF_BUILD_RULE' variants to
> > fix this confusing output.
> >
> > Current output:
> >   CLNG-BPF [test_maps] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs] access_map_in_map.skel.h
> >   ...
> >   CLNG-BPF [test_maps] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
> >   ...
> >   CLNG-BPF [test_maps] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h
> >
> > After fix:
> >   CLNG-BPF [test_progs] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs] access_map_in_map.skel.h
> >   ...
> >   CLNG-BPF [test_progs-no_alu32] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
> >   ...
> >   CLNG-BPF [test_progs-cpuv4] access_map_in_map.bpf.o
> >   GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h
> >
> > Fixes: a5d0c26a2784 ("selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing")
> > Fixes: 89ad7420b25c ("selftests/bpf: Drop the need for LLVM's llc")
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile | 25 +++++++++++++------------
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 0b4bfbc0ef68..67921e3367dd 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -425,27 +425,28 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
> >  $(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
> >
> >  # Build BPF object using Clang
> > -# $1 - input .c file
> > -# $2 - output .o file
> > -# $3 - CFLAGS
> > +# $1 - binary name
> > +# $2 - input .c file
> > +# $3 - output .o file
> > +# $4 - CFLAGS
> >  define CLANG_BPF_BUILD_RULE
> > -       $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> > -       $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
> > +       $(call msg,CLNG-BPF,$1,$3)
> > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v3 -o $3
> 
> this works, but did you have to renumber all parameters? Let's maybe
> pass this binary name as the 4th argument?
> 

OK, I'll update as requested and minimize the diff.

> pw-bot: cr
> 
> >  endef
> >  # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
> >  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
> > +       $(call msg,CLNG-BPF,$1,$3)
> > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v2 -o $3
> >  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
> > +       $(call msg,CLNG-BPF,$1,$3)
> > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v4 -o $3
> >  endef
> >  # Build BPF object using GCC
> >  define GCC_BPF_BUILD_RULE
> > -       $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> > -       $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> > +       $(call msg,GCC-BPF,$1,$3)
> > +       $(Q)$(BPF_GCC) $4 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $2 -o $3
> >  endef
> >
> >  SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
> > @@ -534,7 +535,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:                             \
> >                      $(wildcard $(BPFDIR)/bpf_*.h)                      \
> >                      $(wildcard $(BPFDIR)/*.bpf.h)                      \
> >                      | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> > -       $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,                      \
> > +       $$(call $(TRUNNER_BPF_BUILD_RULE),$(TRUNNER_BINARY),$$<,$$@,    \
> >                                           $(TRUNNER_BPF_CFLAGS)         \
> >                                           $$($$<-CFLAGS)                \
> >                                           $$($$<-$2-CFLAGS))
> > --
> > 2.34.1
> >

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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-20  5:23     ` Tony Ambardar
@ 2024-07-23  1:35       ` Tony Ambardar
  2024-07-23  1:37         ` Eduard Zingerman
  0 siblings, 1 reply; 9+ messages in thread
From: Tony Ambardar @ 2024-07-23  1:35 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Mykola Lysenko, Shuah Khan

On Fri, Jul 19, 2024 at 10:23:33PM -0700, Tony Ambardar wrote:
> On Fri, Jul 19, 2024 at 07:57:09PM -0700, Andrii Nakryiko wrote:
> > On Fri, Jul 19, 2024 at 4:22 PM Tony Ambardar <tony.ambardar@gmail.com> wrote:
> > >

[...]

> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -425,27 +425,28 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
> > >  $(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
> > >
> > >  # Build BPF object using Clang
> > > -# $1 - input .c file
> > > -# $2 - output .o file
> > > -# $3 - CFLAGS
> > > +# $1 - binary name
> > > +# $2 - input .c file
> > > +# $3 - output .o file
> > > +# $4 - CFLAGS
> > >  define CLANG_BPF_BUILD_RULE
> > > -       $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> > > -       $(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
> > > +       $(call msg,CLNG-BPF,$1,$3)
> > > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v3 -o $3
> > 
> > this works, but did you have to renumber all parameters? Let's maybe
> > pass this binary name as the 4th argument?
> > 
> 
> OK, I'll update as requested and minimize the diff.
> 
Hi Andrii,

I sent out a v2 based on your suggestions but omitted Eduard's Acked-by:
by mistake. Should I resubmit or is that something you can update?

Thanks,
Tony
> > pw-bot: cr
> > 
> > >  endef
> > >  # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
> > >  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
> > > +       $(call msg,CLNG-BPF,$1,$3)
> > > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v2 -o $3
> > >  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
> > > +       $(call msg,CLNG-BPF,$1,$3)
> > > +       $(Q)$(CLANG) $4 -O2 --target=bpf -c $2 -mcpu=v4 -o $3
> > >  endef
> > >  # Build BPF object using GCC
> > >  define GCC_BPF_BUILD_RULE
> > > -       $(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> > > -       $(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
> > > +       $(call msg,GCC-BPF,$1,$3)
> > > +       $(Q)$(BPF_GCC) $4 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $2 -o $3
> > >  endef
> > >
> > >  SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
> > > @@ -534,7 +535,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:                             \
> > >                      $(wildcard $(BPFDIR)/bpf_*.h)                      \
> > >                      $(wildcard $(BPFDIR)/*.bpf.h)                      \
> > >                      | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> > > -       $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,                      \
> > > +       $$(call $(TRUNNER_BPF_BUILD_RULE),$(TRUNNER_BINARY),$$<,$$@,    \
> > >                                           $(TRUNNER_BPF_CFLAGS)         \
> > >                                           $$($$<-CFLAGS)                \
> > >                                           $$($$<-$2-CFLAGS))
> > > --
> > > 2.34.1
> > >

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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-23  1:35       ` Tony Ambardar
@ 2024-07-23  1:37         ` Eduard Zingerman
  2024-07-23 20:28           ` Andrii Nakryiko
  0 siblings, 1 reply; 9+ messages in thread
From: Eduard Zingerman @ 2024-07-23  1:37 UTC (permalink / raw)
  To: Tony Ambardar, Andrii Nakryiko
  Cc: bpf, linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Mykola Lysenko, Shuah Khan

On Mon, 2024-07-22 at 18:35 -0700, Tony Ambardar wrote:
[...]

> Hi Andrii,
> 
> I sent out a v2 based on your suggestions but omitted Eduard's Acked-by:
> by mistake. Should I resubmit or is that something you can update?

That's fine, don't bother.

[...]


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

* Re: [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-23  1:37         ` Eduard Zingerman
@ 2024-07-23 20:28           ` Andrii Nakryiko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrii Nakryiko @ 2024-07-23 20:28 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Tony Ambardar, bpf, linux-kselftest, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Mykola Lysenko, Shuah Khan

On Mon, Jul 22, 2024 at 6:37 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2024-07-22 at 18:35 -0700, Tony Ambardar wrote:
> [...]
>
> > Hi Andrii,
> >
> > I sent out a v2 based on your suggestions but omitted Eduard's Acked-by:
> > by mistake. Should I resubmit or is that something you can update?
>
> That's fine, don't bother.
>

Added it while applying.

> [...]
>

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

* [PATCH bpf-next v2] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-19 23:21 ` [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output Tony Ambardar
  2024-07-20  1:23   ` Eduard Zingerman
  2024-07-20  2:57   ` Andrii Nakryiko
@ 2024-07-20  5:25   ` Tony Ambardar
  2024-07-23 20:30     ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 9+ messages in thread
From: Tony Ambardar @ 2024-07-20  5:25 UTC (permalink / raw)
  To: bpf
  Cc: Tony Ambardar, linux-kselftest, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
	Shuah Khan

Make log output incorrectly shows 'test_maps' as the binary name for every
'CLNG-BPF' build step, apparently picking up the last value defined for the
$(TRUNNER_BINARY) variable. Update the 'CLANG_BPF_BUILD_RULE' variants to
fix this confusing output.

Current output:
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_maps] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h

After fix:
  CLNG-BPF [test_progs] access_map_in_map.bpf.o
  GEN-SKEL [test_progs] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_progs-no_alu32] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
  ...
  CLNG-BPF [test_progs-cpuv4] access_map_in_map.bpf.o
  GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h

Fixes: a5d0c26a2784 ("selftests/bpf: Add a cpuv4 test runner for cpu=v4 testing")
Fixes: 89ad7420b25c ("selftests/bpf: Drop the need for LLVM's llc")
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
 tools/testing/selftests/bpf/Makefile | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 0b4bfbc0ef68..41acba62b8c4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -428,23 +428,24 @@ $(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
 # $1 - input .c file
 # $2 - output .o file
 # $3 - CFLAGS
+# $4 - binary name
 define CLANG_BPF_BUILD_RULE
-	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
+	$(call msg,CLNG-BPF,$4,$2)
 	$(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v3 -o $2
 endef
 # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
 define CLANG_NOALU32_BPF_BUILD_RULE
-	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
+	$(call msg,CLNG-BPF,$4,$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)
+	$(call msg,CLNG-BPF,$4,$2)
 	$(Q)$(CLANG) $3 -O2 --target=bpf -c $1 -mcpu=v4 -o $2
 endef
 # Build BPF object using GCC
 define GCC_BPF_BUILD_RULE
-	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
+	$(call msg,GCC-BPF,$4,$2)
 	$(Q)$(BPF_GCC) $3 -DBPF_NO_PRESERVE_ACCESS_INDEX -Wno-attributes -O2 -c $1 -o $2
 endef
 
@@ -537,7 +538,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o:				\
 	$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,			\
 					  $(TRUNNER_BPF_CFLAGS)         \
 					  $$($$<-CFLAGS)		\
-					  $$($$<-$2-CFLAGS))
+					  $$($$<-$2-CFLAGS),$(TRUNNER_BINARY))
 
 $(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
 	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
-- 
2.34.1


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

* Re: [PATCH bpf-next v2] selftests/bpf: Fix wrong binary in Makefile log output
  2024-07-20  5:25   ` [PATCH bpf-next v2] " Tony Ambardar
@ 2024-07-23 20:30     ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-07-23 20:30 UTC (permalink / raw)
  To: Tony Ambardar
  Cc: bpf, linux-kselftest, ast, daniel, andrii, martin.lau, eddyz87,
	song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
	mykolal, shuah

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri, 19 Jul 2024 22:25:35 -0700 you wrote:
> Make log output incorrectly shows 'test_maps' as the binary name for every
> 'CLNG-BPF' build step, apparently picking up the last value defined for the
> $(TRUNNER_BINARY) variable. Update the 'CLANG_BPF_BUILD_RULE' variants to
> fix this confusing output.
> 
> Current output:
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-no_alu32] access_map_in_map.skel.h
>   ...
>   CLNG-BPF [test_maps] access_map_in_map.bpf.o
>   GEN-SKEL [test_progs-cpuv4] access_map_in_map.skel.h
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests/bpf: Fix wrong binary in Makefile log output
    https://git.kernel.org/bpf/bpf-next/c/efb1d64030ef

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2024-07-23 20:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAEf4BzarKiUZqNcq1E+6SaeG8oP5+SfSLLoTNKF3_+7MS6CtyQ@mail.gmail.com>
2024-07-19 23:21 ` [PATCH bpf-next v1] selftests/bpf: Fix wrong binary in Makefile log output Tony Ambardar
2024-07-20  1:23   ` Eduard Zingerman
2024-07-20  2:57   ` Andrii Nakryiko
2024-07-20  5:23     ` Tony Ambardar
2024-07-23  1:35       ` Tony Ambardar
2024-07-23  1:37         ` Eduard Zingerman
2024-07-23 20:28           ` Andrii Nakryiko
2024-07-20  5:25   ` [PATCH bpf-next v2] " Tony Ambardar
2024-07-23 20:30     ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox