On Sat, Oct 9, 2021 at 2:03 PM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote:
>
> We recently updated bpftool's Makefile to make it install the headers
> from libbpf, instead of pulling them directly from libbpf's directory.
> There is also an additional header, internal to libbpf, that needs be
> installed. The way that bpftool's Makefile installs that particular
> header is currently correct, but would break if we were to modify
> $(LIBBPF_INTERNAL_HDRS) to make it point to more than one header.
>
> Use a static pattern rule instead, so that the Makefile can withstand
> the addition of other headers to install.
>
> The objective is simply to make the Makefile more robust. It should
> _not_ be read as an invitation to import more internal headers from
> libbpf into bpftool.
>
> Fixes: f012ade10b34 ("bpftool: Install libbpf headers instead of including the dir")
> Reported-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Quentin Monnet <quentin@xxxxxxxxxxxxx>
> ---
> tools/bpf/bpftool/Makefile | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 9c2d13c513f0..2c510293f32b 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -14,7 +14,7 @@ else
> Q = @
> endif
>
> -BPF_DIR = $(srctree)/tools/lib/bpf/
> +BPF_DIR = $(srctree)/tools/lib/bpf
>
> ifneq ($(OUTPUT),)
> _OUTPUT := $(OUTPUT)
> @@ -25,6 +25,7 @@ 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_BOOTSTRAP_OUTPUT = $(BOOTSTRAP_OUTPUT)libbpf/
> @@ -32,7 +33,8 @@ LIBBPF_BOOTSTRAP = $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
>
> # We need to copy nlattr.h which is not otherwise exported by libbpf, but still
> # required by bpftool.
> -LIBBPF_INTERNAL_HDRS := nlattr.h
> +_LIBBPF_INTERNAL_HDRS := nlattr.h
Felt weird and ugly to have _LIBBPF_INTERNAL_HDRS, so I just inlined it.
> +LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,$(_LIBBPF_INTERNAL_HDRS))
>
> ifeq ($(BPFTOOL_VERSION),)
> BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
> @@ -45,10 +47,9 @@ $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) \
> DESTDIR=$(LIBBPF_DESTDIR) prefix= $(LIBBPF) install_headers
>
> -$(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS): \
> - $(addprefix $(BPF_DIR),$(LIBBPF_INTERNAL_HDRS)) $(LIBBPF)
> - $(call QUIET_INSTALL, bpf/$(notdir $@))
> - $(Q)install -m 644 -t $(LIBBPF_INCLUDE)/bpf/ $(BPF_DIR)$(notdir $@)
> +$(LIBBPF_INTERNAL_HDRS): $(LIBBPF_HDRS_DIR)/%.h: $(BPF_DIR)/%.h $(LIBBPF)
> + $(call QUIET_INSTALL, $@)
> + $(Q)install -m 644 -t $(LIBBPF_HDRS_DIR) $<
>
> $(LIBBPF_BOOTSTRAP): FORCE | $(LIBBPF_BOOTSTRAP_OUTPUT)
> $(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_BOOTSTRAP_OUTPUT) \
> @@ -150,7 +151,7 @@ BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o g
> $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
>
> OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> -$(OBJS): $(LIBBPF) $(LIBBPF_INCLUDE)/bpf/$(LIBBPF_INTERNAL_HDRS)
> +$(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS)
>
> VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \
> $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \
> --
> 2.30.2
>
