| Message ID | 20190718142041.83342-1-iii@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf] tools/bpf: fix bpftool build with OUTPUT set | expand |
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 $@ $<
> 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
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.
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 >
> 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"
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 $@ $<
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(-)