| Message ID | 20211009210341.6291-2-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | BPF |
| Headers | show |
| Series | fixes for bpftool's Makefile | expand |
On Sat, Oct 9, 2021 at 2:03 PM Quentin Monnet <quentin@isovalent.com> 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@kernel.org> > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > 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 >
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 +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) \
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@kernel.org> Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- tools/bpf/bpftool/Makefile | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)