netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
       [not found] <CACAyw9-CWRHVH3TJ=Tke2x8YiLsH47sLCijdp=V+5M836R9aAA@mail.gmail.com>
@ 2019-07-18 14:20 ` Ilya Leoshkevich
  2019-07-18 18:51   ` Jakub Kicinski
  2019-07-23 12:59   ` Lorenz Bauer
  0 siblings, 2 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-07-18 14:20 UTC (permalink / raw)
  To: bpf, netdev, lmb; +Cc: gor, heiko.carstens, Ilya Leoshkevich

Hi Lorenz,

I've been using the following patch for quite some time now.
Please let me know if it works for you.

Best regards,
Ilya

---

When OUTPUT is set, bpftool and libbpf put their objects into the same
directory, and since some of them have the same names, the collision
happens.

Fix by invoking libbpf build in a manner similar to $(call descend) -
descend itself cannot be used, since libbpf is a sibling, and not a
child, of bpftool.

Also, don't link bpftool with libbpf.a twice.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/bpf/bpftool/Makefile | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index a7afea4dec47..2cbc3c166f44 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -15,23 +15,18 @@ else
 endif
 
 BPF_DIR = $(srctree)/tools/lib/bpf/
-
-ifneq ($(OUTPUT),)
-  BPF_PATH = $(OUTPUT)
-else
-  BPF_PATH = $(BPF_DIR)
-endif
-
-LIBBPF = $(BPF_PATH)libbpf.a
+BPF_PATH = $(objtree)/tools/lib/bpf
+LIBBPF = $(BPF_PATH)/libbpf.a
 
 BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion)
 
 $(LIBBPF): FORCE
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a
+	$(Q)mkdir -p $(BPF_PATH)
+	$(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) $(LIBBPF)
 
 $(LIBBPF)-clean:
 	$(call QUIET_CLEAN, libbpf)
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) clean >/dev/null
+	$(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) clean >/dev/null
 
 prefix ?= /usr/local
 bash_compdir ?= /usr/share/bash-completion/completions
@@ -112,7 +107,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
 	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
 
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
-	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
 
 $(OUTPUT)%.o: %.c
 	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
-- 
2.21.0


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

* Re: [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
  2019-07-18 14:20 ` [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set Ilya Leoshkevich
@ 2019-07-18 18:51   ` Jakub Kicinski
  2019-07-19 13:12     ` Ilya Leoshkevich
  2019-07-23 12:59   ` Lorenz Bauer
  1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2019-07-18 18:51 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, netdev, lmb, gor, heiko.carstens

On Thu, 18 Jul 2019 16:20:41 +0200, Ilya Leoshkevich wrote:
> Hi Lorenz,
> 
> I've been using the following patch for quite some time now.
> Please let me know if it works for you.
> 
> Best regards,
> Ilya
> 
> ---
> 
> When OUTPUT is set, bpftool and libbpf put their objects into the same
> directory, and since some of them have the same names, the collision
> happens.
> 
> Fix by invoking libbpf build in a manner similar to $(call descend) -
> descend itself cannot be used, since libbpf is a sibling, and not a
> child, of bpftool.
> 
> Also, don't link bpftool with libbpf.a twice.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tools/bpf/bpftool/Makefile | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index a7afea4dec47..2cbc3c166f44 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -15,23 +15,18 @@ else
>  endif
>  
>  BPF_DIR = $(srctree)/tools/lib/bpf/
> -
> -ifneq ($(OUTPUT),)
> -  BPF_PATH = $(OUTPUT)
> -else
> -  BPF_PATH = $(BPF_DIR)
> -endif
> -
> -LIBBPF = $(BPF_PATH)libbpf.a
> +BPF_PATH = $(objtree)/tools/lib/bpf

objtree won't be set for simple make in the directory. Perhaps we
should stick to using OUTPUT and srctree?

We should probably make a script with all the ways of calling make
should work. Otherwise we can lose track too easily.

# thru kbuild
make tools/bpf

T=$(mktemp -d)
make tools/bpf OUTPUT=$T
rm -rf $T

# from kernel source tree
make -C tools/bpf/bpftool

T=$(mktemp -d)
make -C tools/bpf/bpftool OUTPUT=$T
rm -rf $T

# from tools
cd tools/
make bpf

T=$(mktemp -d)
make bpf OUTPUT=$T
rm -rf $T

# from bpftool's dir
cd bpf/bpftool
make

T=$(mktemp -d)
make OUTPUT=$T
rm -rf $T

.. add your own.

> +LIBBPF = $(BPF_PATH)/libbpf.a
>  
>  BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion)
>  
>  $(LIBBPF): FORCE
> -	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a
> +	$(Q)mkdir -p $(BPF_PATH)
> +	$(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) $(LIBBPF)
>  
>  $(LIBBPF)-clean:
>  	$(call QUIET_CLEAN, libbpf)
> -	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) clean >/dev/null
> +	$(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) clean >/dev/null
>  
>  prefix ?= /usr/local
>  bash_compdir ?= /usr/share/bash-completion/completions
> @@ -112,7 +107,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
>  	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
>  
>  $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
> -	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
> +	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
>  
>  $(OUTPUT)%.o: %.c
>  	$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<


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

* Re: [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
  2019-07-18 18:51   ` Jakub Kicinski
@ 2019-07-19 13:12     ` Ilya Leoshkevich
  2019-07-19 18:17       ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-07-19 13:12 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: bpf, netdev, lmb, gor, heiko.carstens

> Am 18.07.2019 um 20:51 schrieb Jakub Kicinski <jakub.kicinski@netronome.com>:
> 
> We should probably make a script with all the ways of calling make
> should work. Otherwise we can lose track too easily.

Thanks for the script!

I’m trying to make it all pass now, and hitting a weird issue in the
Kbuild case. The build prints "No rule to make target
'scripts/Makefile.ubsan.o'" and proceeds with an empty BPFTOOL_VERSION,
which causes problems later on.

I've found that this is caused by sub_make_done=1 environment variable,
and unsetting it indeed fixes the problem, since the root Makefile no
longer uses the implicit %.o rule.

However, I wonder if that would be acceptable in the final version of
the patch, and whether there is a cleaner way to achieve the same
effect?

Best regards,
Ilya

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

* Re: [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
  2019-07-19 13:12     ` Ilya Leoshkevich
@ 2019-07-19 18:17       ` Jakub Kicinski
  2019-07-23 15:14         ` Ilya Leoshkevich
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2019-07-19 18:17 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: bpf, netdev, lmb, gor, heiko.carstens, Arnaldo Carvalho de Melo

On Fri, 19 Jul 2019 15:12:24 +0200, Ilya Leoshkevich wrote:
> > Am 18.07.2019 um 20:51 schrieb Jakub Kicinski <jakub.kicinski@netronome.com>:
> > 
> > We should probably make a script with all the ways of calling make
> > should work. Otherwise we can lose track too easily.  
> 
> Thanks for the script!
> 
> I’m trying to make it all pass now, and hitting a weird issue in the
> Kbuild case. The build prints "No rule to make target
> 'scripts/Makefile.ubsan.o'" and proceeds with an empty BPFTOOL_VERSION,
> which causes problems later on.

Does it only break with UBSAN enabled?

> I've found that this is caused by sub_make_done=1 environment variable,
> and unsetting it indeed fixes the problem, since the root Makefile no
> longer uses the implicit %.o rule.
> 
> However, I wonder if that would be acceptable in the final version of
> the patch, and whether there is a cleaner way to achieve the same
> effect?

I'm not sure to be honest. Did you check how perf deals with that?

My goal was primarily to make sure we don't regress, so maybe if some
corner cases don't work that's not the end of the world. I think a good
rule of the thumb would be "if it works for perf it should work for
bpftool" ;) Perf gets a lot more build testing.

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

* Re: [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
  2019-07-18 14:20 ` [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set Ilya Leoshkevich
  2019-07-18 18:51   ` Jakub Kicinski
@ 2019-07-23 12:59   ` Lorenz Bauer
  1 sibling, 0 replies; 6+ messages in thread
From: Lorenz Bauer @ 2019-07-23 12:59 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, Networking, gor, heiko.carstens

Hi Ilya,

Thanks for your patch! I tried it but had problems with
cross-compilation. Not sure if this is related to the patch or not
though, I haven't had the time to follow up.

Best
Lorenz

On Thu, 18 Jul 2019 at 15:20, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Hi Lorenz,
>
> I've been using the following patch for quite some time now.
> Please let me know if it works for you.
>
> Best regards,
> Ilya
>
> ---
>
> When OUTPUT is set, bpftool and libbpf put their objects into the same
> directory, and since some of them have the same names, the collision
> happens.
>
> Fix by invoking libbpf build in a manner similar to $(call descend) -
> descend itself cannot be used, since libbpf is a sibling, and not a
> child, of bpftool.
>
> Also, don't link bpftool with libbpf.a twice.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tools/bpf/bpftool/Makefile | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index a7afea4dec47..2cbc3c166f44 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -15,23 +15,18 @@ else
>  endif
>
>  BPF_DIR = $(srctree)/tools/lib/bpf/
> -
> -ifneq ($(OUTPUT),)
> -  BPF_PATH = $(OUTPUT)
> -else
> -  BPF_PATH = $(BPF_DIR)
> -endif
> -
> -LIBBPF = $(BPF_PATH)libbpf.a
> +BPF_PATH = $(objtree)/tools/lib/bpf
> +LIBBPF = $(BPF_PATH)/libbpf.a
>
>  BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion)
>
>  $(LIBBPF): FORCE
> -       $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a
> +       $(Q)mkdir -p $(BPF_PATH)
> +       $(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) $(LIBBPF)
>
>  $(LIBBPF)-clean:
>         $(call QUIET_CLEAN, libbpf)
> -       $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) clean >/dev/null
> +       $(Q)$(MAKE) $(COMMAND_O) subdir=tools/lib/bpf -C $(BPF_DIR) clean >/dev/null
>
>  prefix ?= /usr/local
>  bash_compdir ?= /usr/share/bash-completion/completions
> @@ -112,7 +107,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
>         $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
>
>  $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
> -       $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
> +       $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
>
>  $(OUTPUT)%.o: %.c
>         $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
> --
> 2.21.0
>


-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

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

* Re: [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set
  2019-07-19 18:17       ` Jakub Kicinski
@ 2019-07-23 15:14         ` Ilya Leoshkevich
  0 siblings, 0 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2019-07-23 15:14 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: bpf, netdev, lmb, gor, heiko.carstens, Arnaldo Carvalho de Melo

> Am 19.07.2019 um 20:17 schrieb Jakub Kicinski <jakub.kicinski@netronome.com>:
> 
> On Fri, 19 Jul 2019 15:12:24 +0200, Ilya Leoshkevich wrote:
>>> Am 18.07.2019 um 20:51 schrieb Jakub Kicinski <jakub.kicinski@netronome.com>:
>>> 
>>> We should probably make a script with all the ways of calling make
>>> should work. Otherwise we can lose track too easily.  
>> 
>> Thanks for the script!
>> 
>> I’m trying to make it all pass now, and hitting a weird issue in the
>> Kbuild case. The build prints "No rule to make target
>> 'scripts/Makefile.ubsan.o'" and proceeds with an empty BPFTOOL_VERSION,
>> which causes problems later on.
> 
> Does it only break with UBSAN enabled?

No, all the time. I think this is a coincidence - make happens to scan
scripts/Makefile.ubsan first.

> 
>> I've found that this is caused by sub_make_done=1 environment variable,
>> and unsetting it indeed fixes the problem, since the root Makefile no
>> longer uses the implicit %.o rule.
>> 
>> However, I wonder if that would be acceptable in the final version of
>> the patch, and whether there is a cleaner way to achieve the same
>> effect?
> 
> I'm not sure to be honest. Did you check how perf deals with that?

perf obtains the version using "git describe". However, if we are
building it from a tarball, it falls back to "make kernelversion" and
fails in a similar way:

linux-5.3-rc1$ make defconfig
linux-5.3-rc1$ make tools/perf
<snip>
make[6]: Circular scripts/Makefile.ubsan.mod <- scripts/Makefile.ubsan.o dependency dropped.
make[6]: m2c: Command not found
make[6]: *** [<builtin>: scripts/Makefile.ubsan.o] Error 127
make[5]: *** [Makefile:1765: scripts/Makefile.ubsan.o] Error 2
<snip>

The same trick helps:

--- tools/perf/util/PERF-VERSION-GEN.orig	2019-07-23 17:12:07.621123187 +0200
+++ tools/perf/util/PERF-VERSION-GEN	2019-07-23 17:12:33.441133619 +0200
@@ -26,7 +26,7 @@
 fi
 if test -z "$TAG"
 then
-	TAG=$(MAKEFLAGS= make -sC ../.. kernelversion)
+	TAG=$(MAKEFLAGS= sub_make_done= make -sC ../.. kernelversion)
 fi
 VN="$TAG$CID"
 if test -n "$CID"

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

end of thread, other threads:[~2019-07-23 15:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CACAyw9-CWRHVH3TJ=Tke2x8YiLsH47sLCijdp=V+5M836R9aAA@mail.gmail.com>
2019-07-18 14:20 ` [PATCH bpf] tools/bpf: fix bpftool build with OUTPUT set Ilya Leoshkevich
2019-07-18 18:51   ` Jakub Kicinski
2019-07-19 13:12     ` Ilya Leoshkevich
2019-07-19 18:17       ` Jakub Kicinski
2019-07-23 15:14         ` Ilya Leoshkevich
2019-07-23 12:59   ` Lorenz Bauer

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