diff mbox series

tools/build: Use SYSTEM_BPFTOOL for system bpftool

Message ID 20250326004018.248357-1-tglozar@redhat.com (mailing list archive)
State Accepted
Commit 814d051ebed40b27285ab3c5e2454bd01a0f9631
Delegated to: BPF
Headers show
Series tools/build: Use SYSTEM_BPFTOOL for system bpftool | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-PR fail merge-conflict

Commit Message

Tomas Glozar March 26, 2025, 12:40 a.m. UTC
The feature test for system bpftool uses BPFTOOL as the variable to set
its path, defaulting to just "bpftool" if not set by the user.

This conflicts with selftests and a few other utilities, which expect
BPFTOOL to be set to the in-tree bpftool path by default. For example,
bpftool selftests fail to build:

$ make -C tools/testing/selftests/bpf/
make: Entering directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'

make: *** No rule to make target 'bpftool', needed by '/home/tglozar/dev/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h'.  Stop.
make: Leaving directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'

Fix the problem by renaming the variable used for system bpftool from
BPFTOOL to SYSTEM_BPFTOOL, so that the new usage does not conflict with
the existing one of BPFTOOL.

Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Closes: https://lore.kernel.org/linux-kernel/5df6968a-2e5f-468e-b457-fc201535dd4c@linux.ibm.com/
Suggested-by: Quentin Monnet <qmo@qmon.net>
Fixes: 8a635c3856dd ("tools/build: Add bpftool-skeletons feature test")
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
 tools/build/feature/Makefile   | 2 +-
 tools/scripts/Makefile.include | 2 +-
 tools/tracing/rtla/Makefile    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Comments

Quentin Monnet March 26, 2025, 9:59 a.m. UTC | #1
2025-03-26 01:40 UTC+0100 ~ Tomas Glozar <tglozar@redhat.com>
> The feature test for system bpftool uses BPFTOOL as the variable to set
> its path, defaulting to just "bpftool" if not set by the user.
> 
> This conflicts with selftests and a few other utilities, which expect
> BPFTOOL to be set to the in-tree bpftool path by default. For example,
> bpftool selftests fail to build:
> 
> $ make -C tools/testing/selftests/bpf/
> make: Entering directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'
> 
> make: *** No rule to make target 'bpftool', needed by '/home/tglozar/dev/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h'.  Stop.
> make: Leaving directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'
> 
> Fix the problem by renaming the variable used for system bpftool from
> BPFTOOL to SYSTEM_BPFTOOL, so that the new usage does not conflict with
> the existing one of BPFTOOL.
> 
> Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> Closes: https://lore.kernel.org/linux-kernel/5df6968a-2e5f-468e-b457-fc201535dd4c@linux.ibm.com/
> Suggested-by: Quentin Monnet <qmo@qmon.net>


Let's use <qmo@kernel.org> if possible, please.


> Fixes: 8a635c3856dd ("tools/build: Add bpftool-skeletons feature test")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>

Looks good, thanks a lot!

Acked-by: Quentin Monnet <qmo@kernel.org>
Venkat Rao Bagalkote March 26, 2025, 1:10 p.m. UTC | #2
On 26/03/25 6:10 am, Tomas Glozar wrote:
> The feature test for system bpftool uses BPFTOOL as the variable to set
> its path, defaulting to just "bpftool" if not set by the user.
>
> This conflicts with selftests and a few other utilities, which expect
> BPFTOOL to be set to the in-tree bpftool path by default. For example,
> bpftool selftests fail to build:
>
> $ make -C tools/testing/selftests/bpf/
> make: Entering directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'
>
> make: *** No rule to make target 'bpftool', needed by '/home/tglozar/dev/linux/tools/testing/selftests/bpf/tools/include/vmlinux.h'.  Stop.
> make: Leaving directory '/home/tglozar/dev/linux/tools/testing/selftests/bpf'
>
> Fix the problem by renaming the variable used for system bpftool from
> BPFTOOL to SYSTEM_BPFTOOL, so that the new usage does not conflict with
> the existing one of BPFTOOL.
>
> Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> Closes: https://lore.kernel.org/linux-kernel/5df6968a-2e5f-468e-b457-fc201535dd4c@linux.ibm.com/
> Suggested-by: Quentin Monnet <qmo@qmon.net>
> Fixes: 8a635c3856dd ("tools/build: Add bpftool-skeletons feature test")
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>   tools/build/feature/Makefile   | 2 +-
>   tools/scripts/Makefile.include | 2 +-
>   tools/tracing/rtla/Makefile    | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 4f9c1d950f5d..b8b5fb183dd4 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -419,7 +419,7 @@ $(OUTPUT)test-libpfm4.bin:
>   	$(BUILD) -lpfm
>   
>   $(OUTPUT)test-bpftool-skeletons.bin:
> -	$(BPFTOOL) version | grep '^features:.*skeletons' \
> +	$(SYSTEM_BPFTOOL) version | grep '^features:.*skeletons' \
>   		> $(@:.bin=.make.output) 2>&1
>   ###############################
>   
> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> index 6268534309aa..5158250988ce 100644
> --- a/tools/scripts/Makefile.include
> +++ b/tools/scripts/Makefile.include
> @@ -92,7 +92,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
>   LLVM_STRIP	?= llvm-strip
>   
>   # Some tools require bpftool
> -BPFTOOL		?= bpftool
> +SYSTEM_BPFTOOL	?= bpftool
>   
>   ifeq ($(CC_NO_CLANG), 1)
>   EXTRA_WARNINGS += -Wstrict-aliasing=3
> diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
> index 4cc3017dccaa..746ccf2f5808 100644
> --- a/tools/tracing/rtla/Makefile
> +++ b/tools/tracing/rtla/Makefile
> @@ -72,7 +72,7 @@ src/timerlat.bpf.o: src/timerlat.bpf.c
>   	$(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@
>   
>   src/timerlat.skel.h: src/timerlat.bpf.o
> -	$(QUIET_GENSKEL)$(BPFTOOL) gen skeleton $< > $@
> +	$(QUIET_GENSKEL)$(SYSTEM_BPFTOOL) gen skeleton $< > $@
>   else
>   src/timerlat.skel.h:
>   	$(Q)echo '/* BPF skeleton is disabled */' > src/timerlat.skel.h

Tested this patch by applying on linux-next20250326 and this patch fixes 
the reported issue.

Please add below tag.

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>


Regards,

Venkat.
Steven Rostedt March 26, 2025, 2:34 p.m. UTC | #3
On Wed, 26 Mar 2025 09:59:12 +0000
Quentin Monnet <qmo@kernel.org> wrote:

> > Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
> > Closes: https://lore.kernel.org/linux-kernel/5df6968a-2e5f-468e-b457-fc201535dd4c@linux.ibm.com/
> > Suggested-by: Quentin Monnet <qmo@qmon.net>  
> 
> 
> Let's use <qmo@kernel.org> if possible, please.

Updated.

> 
> 
> > Fixes: 8a635c3856dd ("tools/build: Add bpftool-skeletons feature test")
> > Signed-off-by: Tomas Glozar <tglozar@redhat.com>  
> 
> Looks good, thanks a lot!
> 
> Acked-by: Quentin Monnet <qmo@kernel.org>

I added this to my queue.

Thanks everyone.

-- Steve
patchwork-bot+netdevbpf@kernel.org March 30, 2025, 10:21 p.m. UTC | #4
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Steven Rostedt (Google) <rostedt@goodmis.org>:

On Wed, 26 Mar 2025 01:40:18 +0100 you wrote:
> The feature test for system bpftool uses BPFTOOL as the variable to set
> its path, defaulting to just "bpftool" if not set by the user.
> 
> This conflicts with selftests and a few other utilities, which expect
> BPFTOOL to be set to the in-tree bpftool path by default. For example,
> bpftool selftests fail to build:
> 
> [...]

Here is the summary with links:
  - tools/build: Use SYSTEM_BPFTOOL for system bpftool
    https://git.kernel.org/bpf/bpf-next/c/814d051ebed4

You are awesome, thank you!
diff mbox series

Patch

diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 4f9c1d950f5d..b8b5fb183dd4 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -419,7 +419,7 @@  $(OUTPUT)test-libpfm4.bin:
 	$(BUILD) -lpfm
 
 $(OUTPUT)test-bpftool-skeletons.bin:
-	$(BPFTOOL) version | grep '^features:.*skeletons' \
+	$(SYSTEM_BPFTOOL) version | grep '^features:.*skeletons' \
 		> $(@:.bin=.make.output) 2>&1
 ###############################
 
diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
index 6268534309aa..5158250988ce 100644
--- a/tools/scripts/Makefile.include
+++ b/tools/scripts/Makefile.include
@@ -92,7 +92,7 @@  LLVM_OBJCOPY	?= llvm-objcopy
 LLVM_STRIP	?= llvm-strip
 
 # Some tools require bpftool
-BPFTOOL		?= bpftool
+SYSTEM_BPFTOOL	?= bpftool
 
 ifeq ($(CC_NO_CLANG), 1)
 EXTRA_WARNINGS += -Wstrict-aliasing=3
diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
index 4cc3017dccaa..746ccf2f5808 100644
--- a/tools/tracing/rtla/Makefile
+++ b/tools/tracing/rtla/Makefile
@@ -72,7 +72,7 @@  src/timerlat.bpf.o: src/timerlat.bpf.c
 	$(QUIET_CLANG)$(CLANG) -g -O2 -target bpf -c $(filter %.c,$^) -o $@
 
 src/timerlat.skel.h: src/timerlat.bpf.o
-	$(QUIET_GENSKEL)$(BPFTOOL) gen skeleton $< > $@
+	$(QUIET_GENSKEL)$(SYSTEM_BPFTOOL) gen skeleton $< > $@
 else
 src/timerlat.skel.h:
 	$(Q)echo '/* BPF skeleton is disabled */' > src/timerlat.skel.h