[PATCH bpf] bpftool: Install libbpf headers for the bootstrap version, too

We recently changed bpftool's Makefile to make it install libbpf's
headers locally instead of pulling them from the source directory of the
library. Although bpftool needs two versions of libbpf, a "regular" one
and a "bootstrap" version, we would only install headers for the regular
libbpf build. Given that this build always occurs before the bootstrap
build when building bpftool, this is enough to ensure that the bootstrap
bpftool will have access to the headers exported through the regular
libbpf build.

However, this did not account for the case when we only want the
bootstrap version of bpftool, through the "bootstrap" target. For
example, perf needs the bootstrap version only, to generate BPF
skeletons. In that case, when are the headers installed? For some time,
the issue has been masked, because we had a step (the installation of
headers internal to libbpf) which would depend on the regular build of
libbpf and hence trigger the export of the headers, just for the sake of
creating a directory. But this changed with commit 8b6c46241c77
("bpftool: Remove Makefile dep. on $(LIBBPF) for
$(LIBBPF_INTERNAL_HDRS)"), where we cleaned up that stage and removed
the dependency on the regular libbpf build. As a result, when we only
want the bootstrap bpftool version, the regular libbpf is no longer
built. The bootstrap libbpf version is built, but headers are not
exported, and the bootstrap bpftool build fails because of the missing
headers.

To fix this, we also install the library headers for the bootstrap
version of libbpf, to use them for the bootstrap bpftool and for
generating the skeletons.

Fixes: f012ade10b34 ("bpftool: Install libbpf headers instead of including the dir")
Signed-off-by: Quentin Monnet <redacted>
---
 tools/bpf/bpftool/Makefile | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c0c30e56988f..7cfba11c3014 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile

@@ -22,24 +22,29 @@ else
   _OUTPUT := $(CURDIR)
 endif
 BOOTSTRAP_OUTPUT := $(_OUTPUT)/bootstrap/
+
 LIBBPF_OUTPUT := $(_OUTPUT)/libbpf/
 LIBBPF_DESTDIR := $(LIBBPF_OUTPUT)
 LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)/include
 LIBBPF_HDRS_DIR := $(LIBBPF_INCLUDE)/bpf
+LIBBPF := $(LIBBPF_OUTPUT)libbpf.a
 
-LIBBPF = $(LIBBPF_OUTPUT)libbpf.a
-LIBBPF_BOOTSTRAP_OUTPUT = $(BOOTSTRAP_OUTPUT)libbpf/
-LIBBPF_BOOTSTRAP = $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
+LIBBPF_BOOTSTRAP_OUTPUT := $(BOOTSTRAP_OUTPUT)libbpf/
+LIBBPF_BOOTSTRAP_DESTDIR := $(LIBBPF_BOOTSTRAP_OUTPUT)
+LIBBPF_BOOTSTRAP_INCLUDE := $(LIBBPF_BOOTSTRAP_DESTDIR)/include
+LIBBPF_BOOTSTRAP_HDRS_DIR := $(LIBBPF_BOOTSTRAP_INCLUDE)/bpf
+LIBBPF_BOOTSTRAP := $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
 
 # We need to copy hashmap.h and nlattr.h which is not otherwise exported by
 # libbpf, but still required by bpftool.
 LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h)
+LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h)
 
 ifeq ($(BPFTOOL_VERSION),)
 BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 endif
 
-$(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR):
+$(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR) $(LIBBPF_BOOTSTRAP_HDRS_DIR):
 	$(QUIET_MKDIR)mkdir -p $@
 
 $(LIBBPF): $(wildcard $(BPF_DIR)/*.[ch] $(BPF_DIR)/Makefile) | $(LIBBPF_OUTPUT)

@@ -52,7 +57,12 @@ $(LIBBPF_INTERNAL_HDRS): $(LIBBPF_HDRS_DIR)/%.h: $(BPF_DIR)/%.h | $(LIBBPF_HDRS_
 
 $(LIBBPF_BOOTSTRAP): $(wildcard $(BPF_DIR)/*.[ch] $(BPF_DIR)/Makefile) | $(LIBBPF_BOOTSTRAP_OUTPUT)
 	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
-		ARCH= CC=$(HOSTCC) LD=$(HOSTLD) $@
+		DESTDIR=$(LIBBPF_BOOTSTRAP_DESTDIR) prefix= \
+		ARCH= CC=$(HOSTCC) LD=$(HOSTLD) $@ install_headers
+
+$(LIBBPF_BOOTSTRAP_INTERNAL_HDRS): $(LIBBPF_BOOTSTRAP_HDRS_DIR)/%.h: $(BPF_DIR)/%.h | $(LIBBPF_BOOTSTRAP_HDRS_DIR)
+	$(call QUIET_INSTALL, $@)
+	$(Q)install -m 644 -t $(LIBBPF_BOOTSTRAP_HDRS_DIR) $<
 
 $(LIBBPF)-clean: FORCE | $(LIBBPF_OUTPUT)
 	$(call QUIET_CLEAN, libbpf)

@@ -172,11 +182,11 @@ else
 	$(Q)cp "$(VMLINUX_H)" $@
 endif
 
-$(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF)
+$(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
 	$(QUIET_CLANG)$(CLANG) \
 		-I$(if $(OUTPUT),$(OUTPUT),.) \
 		-I$(srctree)/tools/include/uapi/ \
-		-I$(LIBBPF_INCLUDE) \
+		-I$(LIBBPF_BOOTSTRAP_INCLUDE) \
 		-g -O2 -Wall -target bpf -c $< -o $@ && $(LLVM_STRIP) -g $@
 
 $(OUTPUT)%.skel.h: $(OUTPUT)%.bpf.o $(BPFTOOL_BOOTSTRAP)

@@ -209,8 +219,10 @@ $(BPFTOOL_BOOTSTRAP): $(BOOTSTRAP_OBJS) $(LIBBPF_BOOTSTRAP)
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
 
-$(BOOTSTRAP_OUTPUT)%.o: %.c $(LIBBPF_INTERNAL_HDRS) | $(BOOTSTRAP_OUTPUT)
-	$(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
+$(BOOTSTRAP_OUTPUT)%.o: %.c $(LIBBPF_BOOTSTRAP_INTERNAL_HDRS) | $(BOOTSTRAP_OUTPUT)
+	$(QUIET_CC)$(HOSTCC) \
+		$(subst -I$(LIBBPF_INCLUDE),-I$(LIBBPF_BOOTSTRAP_INCLUDE),$(CFLAGS)) \
+		-c -MMD -o $@ $<
 
 $(OUTPUT)%.o: %.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<

@@ -257,6 +269,6 @@ doc-uninstall:
 FORCE:
 
 .SECONDARY:
-.PHONY: all FORCE clean install-bin install uninstall
+.PHONY: all FORCE bootstrap clean install-bin install uninstall
 .PHONY: doc doc-clean doc-install doc-uninstall
 .DEFAULT_GOAL := all

-- 
2.32.0

Em Fri, Nov 05, 2021 at 01:58:13AM +0000, Quentin Monnet escreveu:

We recently changed bpftool's Makefile to make it install libbpf's
headers locally instead of pulling them from the source directory of the
library. Although bpftool needs two versions of libbpf, a "regular" one
and a "bootstrap" version, we would only install headers for the regular
libbpf build. Given that this build always occurs before the bootstrap
build when building bpftool, this is enough to ensure that the bootstrap
bpftool will have access to the headers exported through the regular
libbpf build.

However, this did not account for the case when we only want the
bootstrap version of bpftool, through the "bootstrap" target. For
example, perf needs the bootstrap version only, to generate BPF
skeletons. In that case, when are the headers installed? For some time,
the issue has been masked, because we had a step (the installation of
headers internal to libbpf) which would depend on the regular build of
libbpf and hence trigger the export of the headers, just for the sake of
creating a directory. But this changed with commit 8b6c46241c77
("bpftool: Remove Makefile dep. on $(LIBBPF) for
$(LIBBPF_INTERNAL_HDRS)"), where we cleaned up that stage and removed
the dependency on the regular libbpf build. As a result, when we only
want the bootstrap bpftool version, the regular libbpf is no longer
built. The bootstrap libbpf version is built, but headers are not
exported, and the bootstrap bpftool build fails because of the missing
headers.

To fix this, we also install the library headers for the bootstrap
version of libbpf, to use them for the bootstrap bpftool and for
generating the skeletons.

Reported-by: Arnaldo Carvalho de Melo <redacted>

Testing now.

- Arnaldo
 

-- 

- Arnaldo

Em Fri, Nov 05, 2021 at 07:55:20AM -0300, Arnaldo Carvalho de Melo escreveu:

Em Fri, Nov 05, 2021 at 01:58:13AM +0000, Quentin Monnet escreveu:

quoted

We recently changed bpftool's Makefile to make it install libbpf's
headers locally instead of pulling them from the source directory of the
library. Although bpftool needs two versions of libbpf, a "regular" one
and a "bootstrap" version, we would only install headers for the regular
libbpf build. Given that this build always occurs before the bootstrap
build when building bpftool, this is enough to ensure that the bootstrap
bpftool will have access to the headers exported through the regular
libbpf build.

quoted

However, this did not account for the case when we only want the
bootstrap version of bpftool, through the "bootstrap" target. For
example, perf needs the bootstrap version only, to generate BPF
skeletons. In that case, when are the headers installed? For some time,
the issue has been masked, because we had a step (the installation of
headers internal to libbpf) which would depend on the regular build of
libbpf and hence trigger the export of the headers, just for the sake of
creating a directory. But this changed with commit 8b6c46241c77
("bpftool: Remove Makefile dep. on $(LIBBPF) for
$(LIBBPF_INTERNAL_HDRS)"), where we cleaned up that stage and removed
the dependency on the regular libbpf build. As a result, when we only
want the bootstrap bpftool version, the regular libbpf is no longer
built. The bootstrap libbpf version is built, but headers are not
exported, and the bootstrap bpftool build fails because of the missing
headers.

quoted

To fix this, we also install the library headers for the bootstrap
version of libbpf, to use them for the bootstrap bpftool and for
generating the skeletons.

Reported-by: Arnaldo Carvalho de Melo <redacted>

Testing now.

Tested-by: Arnaldo Carvalho de Melo <redacted>

With just this patch (and removing the revert I did), it works, thanks!

- Arnaldo
 

quoted

Fixes: f012ade10b34 ("bpftool: Install libbpf headers instead of including the dir")
Signed-off-by: Quentin Monnet <redacted>
---
 tools/bpf/bpftool/Makefile | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c0c30e56988f..7cfba11c3014 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile

@@ -22,24 +22,29 @@ else
   _OUTPUT := $(CURDIR)
 endif
 BOOTSTRAP_OUTPUT := $(_OUTPUT)/bootstrap/
+
 LIBBPF_OUTPUT := $(_OUTPUT)/libbpf/
 LIBBPF_DESTDIR := $(LIBBPF_OUTPUT)
 LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)/include
 LIBBPF_HDRS_DIR := $(LIBBPF_INCLUDE)/bpf
+LIBBPF := $(LIBBPF_OUTPUT)libbpf.a
 
-LIBBPF = $(LIBBPF_OUTPUT)libbpf.a
-LIBBPF_BOOTSTRAP_OUTPUT = $(BOOTSTRAP_OUTPUT)libbpf/
-LIBBPF_BOOTSTRAP = $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
+LIBBPF_BOOTSTRAP_OUTPUT := $(BOOTSTRAP_OUTPUT)libbpf/
+LIBBPF_BOOTSTRAP_DESTDIR := $(LIBBPF_BOOTSTRAP_OUTPUT)
+LIBBPF_BOOTSTRAP_INCLUDE := $(LIBBPF_BOOTSTRAP_DESTDIR)/include
+LIBBPF_BOOTSTRAP_HDRS_DIR := $(LIBBPF_BOOTSTRAP_INCLUDE)/bpf
+LIBBPF_BOOTSTRAP := $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
 
 # We need to copy hashmap.h and nlattr.h which is not otherwise exported by
 # libbpf, but still required by bpftool.
 LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h)
+LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h)
 
 ifeq ($(BPFTOOL_VERSION),)
 BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 endif
 
-$(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR):
+$(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR) $(LIBBPF_BOOTSTRAP_HDRS_DIR):
 	$(QUIET_MKDIR)mkdir -p $@
 
 $(LIBBPF): $(wildcard $(BPF_DIR)/*.[ch] $(BPF_DIR)/Makefile) | $(LIBBPF_OUTPUT)

@@ -52,7 +57,12 @@ $(LIBBPF_INTERNAL_HDRS): $(LIBBPF_HDRS_DIR)/%.h: $(BPF_DIR)/%.h | $(LIBBPF_HDRS_
 
 $(LIBBPF_BOOTSTRAP): $(wildcard $(BPF_DIR)/*.[ch] $(BPF_DIR)/Makefile) | $(LIBBPF_BOOTSTRAP_OUTPUT)
 	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
-		ARCH= CC=$(HOSTCC) LD=$(HOSTLD) $@
+		DESTDIR=$(LIBBPF_BOOTSTRAP_DESTDIR) prefix= \
+		ARCH= CC=$(HOSTCC) LD=$(HOSTLD) $@ install_headers
+
+$(LIBBPF_BOOTSTRAP_INTERNAL_HDRS): $(LIBBPF_BOOTSTRAP_HDRS_DIR)/%.h: $(BPF_DIR)/%.h | $(LIBBPF_BOOTSTRAP_HDRS_DIR)
+	$(call QUIET_INSTALL, $@)
+	$(Q)install -m 644 -t $(LIBBPF_BOOTSTRAP_HDRS_DIR) $<
 
 $(LIBBPF)-clean: FORCE | $(LIBBPF_OUTPUT)
 	$(call QUIET_CLEAN, libbpf)

@@ -172,11 +182,11 @@ else
 	$(Q)cp "$(VMLINUX_H)" $@
 endif
 
-$(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF)
+$(OUTPUT)%.bpf.o: skeleton/%.bpf.c $(OUTPUT)vmlinux.h $(LIBBPF_BOOTSTRAP)
 	$(QUIET_CLANG)$(CLANG) \
 		-I$(if $(OUTPUT),$(OUTPUT),.) \
 		-I$(srctree)/tools/include/uapi/ \
-		-I$(LIBBPF_INCLUDE) \
+		-I$(LIBBPF_BOOTSTRAP_INCLUDE) \
 		-g -O2 -Wall -target bpf -c $< -o $@ && $(LLVM_STRIP) -g $@
 
 $(OUTPUT)%.skel.h: $(OUTPUT)%.bpf.o $(BPFTOOL_BOOTSTRAP)

@@ -209,8 +219,10 @@ $(BPFTOOL_BOOTSTRAP): $(BOOTSTRAP_OBJS) $(LIBBPF_BOOTSTRAP)
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(OBJS) $(LIBS)
 
-$(BOOTSTRAP_OUTPUT)%.o: %.c $(LIBBPF_INTERNAL_HDRS) | $(BOOTSTRAP_OUTPUT)
-	$(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
+$(BOOTSTRAP_OUTPUT)%.o: %.c $(LIBBPF_BOOTSTRAP_INTERNAL_HDRS) | $(BOOTSTRAP_OUTPUT)
+	$(QUIET_CC)$(HOSTCC) \
+		$(subst -I$(LIBBPF_INCLUDE),-I$(LIBBPF_BOOTSTRAP_INCLUDE),$(CFLAGS)) \
+		-c -MMD -o $@ $<
 
 $(OUTPUT)%.o: %.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<

@@ -257,6 +269,6 @@ doc-uninstall:
 FORCE:
 
 .SECONDARY:
-.PHONY: all FORCE clean install-bin install uninstall
+.PHONY: all FORCE bootstrap clean install-bin install uninstall
 .PHONY: doc doc-clean doc-install doc-uninstall
 .DEFAULT_GOAL := all

-- 
2.32.0

Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann [off-list ref]:

On Fri,  5 Nov 2021 01:58:13 +0000 you wrote:

We recently changed bpftool's Makefile to make it install libbpf's
headers locally instead of pulling them from the source directory of the
library. Although bpftool needs two versions of libbpf, a "regular" one
and a "bootstrap" version, we would only install headers for the regular
libbpf build. Given that this build always occurs before the bootstrap
build when building bpftool, this is enough to ensure that the bootstrap
bpftool will have access to the headers exported through the regular
libbpf build.

[...]

Here is the summary with links:
  - [bpf] bpftool: Install libbpf headers for the bootstrap version, too
    https://git.kernel.org/bpf/bpf/c/e41ac2020bca

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