| Message ID | 20211110114632.24537-4-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | BPF |
| Headers | show |
| Series | bpftool: miscellaneous fixes | expand |
On Wed, Nov 10, 2021 at 3:46 AM Quentin Monnet <quentin@isovalent.com> wrote: > > The Makefile for bpftool relies on $(OUTPUT), and not on $(O), for > passing the output directory. So $(VMLINUX_BTF_PATHS), used for > searching for kernel BTF info, should use the same variable. > > Fixes: 05aca6da3b5a ("tools/bpftool: Generalize BPF skeleton support and generate vmlinux.h") > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > tools/bpf/bpftool/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > index 2a846cb92120..40abf50b59d4 100644 > --- a/tools/bpf/bpftool/Makefile > +++ b/tools/bpf/bpftool/Makefile > @@ -150,7 +150,7 @@ $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP) > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o > $(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS) > > -VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ > +VMLINUX_BTF_PATHS ?= $(if $(OUTPUT),$(OUTPUT)/vmlinux) \ > $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \ But you still check KBUILD_OUTPUT? O overrides KBUILD_OUTPUT as far as kernel build goes. So if you still support KBUILD_OUTPUT, you should support O. And the $(OUTPUT) seems to be completely unrelated, as that defines the output of bpftool build files, not the vmlinux image. Or am I missing something? > ../../../vmlinux \ > /sys/kernel/btf/vmlinux \ > -- > 2.32.0 >
On Thu, 11 Nov 2021 at 18:59, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Nov 10, 2021 at 3:46 AM Quentin Monnet <quentin@isovalent.com> wrote: > > > > The Makefile for bpftool relies on $(OUTPUT), and not on $(O), for > > passing the output directory. So $(VMLINUX_BTF_PATHS), used for > > searching for kernel BTF info, should use the same variable. > > > > Fixes: 05aca6da3b5a ("tools/bpftool: Generalize BPF skeleton support and generate vmlinux.h") > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > --- > > tools/bpf/bpftool/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > > index 2a846cb92120..40abf50b59d4 100644 > > --- a/tools/bpf/bpftool/Makefile > > +++ b/tools/bpf/bpftool/Makefile > > @@ -150,7 +150,7 @@ $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP) > > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o > > $(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS) > > > > -VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ > > +VMLINUX_BTF_PATHS ?= $(if $(OUTPUT),$(OUTPUT)/vmlinux) \ > > $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \ > > But you still check KBUILD_OUTPUT? O overrides KBUILD_OUTPUT as far as > kernel build goes. So if you still support KBUILD_OUTPUT, you should > support O. And the $(OUTPUT) seems to be completely unrelated, as that > defines the output of bpftool build files, not the vmlinux image. Or > am I missing something? OK, I think I'm the one who missed the point. I simply figured we meant to search the output directory, and that it should be $(OUTPUT) like everywhere else in the Makefile. But from what I understand now, it's not the case. Let's drop this patch. If the rest of the set looks good to you, can you just skip this patch, or do you prefer me to send a v2? Quentin
On Thu, Nov 11, 2021 at 3:39 PM Quentin Monnet <quentin@isovalent.com> wrote: > > On Thu, 11 Nov 2021 at 18:59, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, Nov 10, 2021 at 3:46 AM Quentin Monnet <quentin@isovalent.com> wrote: > > > > > > The Makefile for bpftool relies on $(OUTPUT), and not on $(O), for > > > passing the output directory. So $(VMLINUX_BTF_PATHS), used for > > > searching for kernel BTF info, should use the same variable. > > > > > > Fixes: 05aca6da3b5a ("tools/bpftool: Generalize BPF skeleton support and generate vmlinux.h") > > > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > > > --- > > > tools/bpf/bpftool/Makefile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > > > index 2a846cb92120..40abf50b59d4 100644 > > > --- a/tools/bpf/bpftool/Makefile > > > +++ b/tools/bpf/bpftool/Makefile > > > @@ -150,7 +150,7 @@ $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP) > > > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o > > > $(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS) > > > > > > -VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ > > > +VMLINUX_BTF_PATHS ?= $(if $(OUTPUT),$(OUTPUT)/vmlinux) \ > > > $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \ > > > > But you still check KBUILD_OUTPUT? O overrides KBUILD_OUTPUT as far as > > kernel build goes. So if you still support KBUILD_OUTPUT, you should > > support O. And the $(OUTPUT) seems to be completely unrelated, as that > > defines the output of bpftool build files, not the vmlinux image. Or > > am I missing something? > > OK, I think I'm the one who missed the point. I simply figured we > meant to search the output directory, and that it should be $(OUTPUT) > like everywhere else in the Makefile. But from what I understand now, > it's not the case. Let's drop this patch. > > If the rest of the set looks good to you, can you just skip this > patch, or do you prefer me to send a v2? > I can just drop it when applying. > Quentin
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 2a846cb92120..40abf50b59d4 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -150,7 +150,7 @@ $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP) OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o $(OBJS): $(LIBBPF) $(LIBBPF_INTERNAL_HDRS) -VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux) \ +VMLINUX_BTF_PATHS ?= $(if $(OUTPUT),$(OUTPUT)/vmlinux) \ $(if $(KBUILD_OUTPUT),$(KBUILD_OUTPUT)/vmlinux) \ ../../../vmlinux \ /sys/kernel/btf/vmlinux \
The Makefile for bpftool relies on $(OUTPUT), and not on $(O), for passing the output directory. So $(VMLINUX_BTF_PATHS), used for searching for kernel BTF info, should use the same variable. Fixes: 05aca6da3b5a ("tools/bpftool: Generalize BPF skeleton support and generate vmlinux.h") Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- tools/bpf/bpftool/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)