[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YYUNmAexMZ1RQhWk@kernel.org>
Date: Fri, 5 Nov 2021 07:55:20 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Quentin Monnet <quentin@...valent.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org,
bpf@...r.kernel.org, Song Liu <songliubraving@...com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf] bpftool: Install libbpf headers for the bootstrap
version, too
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 <acme@...hat.com>
Testing now.
- Arnaldo
> Fixes: f012ade10b34 ("bpftool: Install libbpf headers instead of including the dir")
> Signed-off-by: Quentin Monnet <quentin@...valent.com>
> ---
> 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
--
- Arnaldo
Powered by blists - more mailing lists