diff mbox series

[bpf-next] tools/bpftool: Fix cross-build

Message ID 20210603170515.1854642-1-jean-philippe@linaro.org (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series [bpf-next] tools/bpftool: Fix cross-build | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 4 maintainers not CCed: brouer@redhat.com netdev@vger.kernel.org quentin@isovalent.com tony.ambardar@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 17 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Jean-Philippe Brucker June 3, 2021, 5:05 p.m. UTC
When the bootstrap and final bpftool have different architectures, we
need to build two distinct disasm.o objects. Add a recipe for the
bootstrap disasm.o

Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tools/bpf/bpftool/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko June 3, 2021, 5:50 p.m. UTC | #1
On Thu, Jun 3, 2021 at 10:06 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> When the bootstrap and final bpftool have different architectures, we
> need to build two distinct disasm.o objects. Add a recipe for the
> bootstrap disasm.o
>
> Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")

Did this commit break something specifically?

> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  tools/bpf/bpftool/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index d16d289ade7a..d73232be1e99 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -136,7 +136,7 @@ endif
>
>  BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
>
> -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o) $(OUTPUT)disasm.o
> +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o disasm.o)
>  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
>
>  VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)                           \
> @@ -180,6 +180,9 @@ endif
>
>  CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
>
> +$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> +       $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
> +
>  $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c

maybe just do

$(BOOTSTRAP_OUTPUT)disasm.o $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c

?

>         $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<
>
> --
> 2.31.1
>
Jean-Philippe Brucker June 4, 2021, 7:50 a.m. UTC | #2
On Thu, Jun 03, 2021 at 10:50:08AM -0700, Andrii Nakryiko wrote:
> On Thu, Jun 3, 2021 at 10:06 AM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> >
> > When the bootstrap and final bpftool have different architectures, we
> > need to build two distinct disasm.o objects. Add a recipe for the
> > bootstrap disasm.o
> >
> > Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
> 
> Did this commit break something specifically?

Yes, cross-building bpftool doesn't work anymore, because the bootstrap
bpftool is linked using objects from different architectures:

$ make O=/tmp/bpftool ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -C tools/bpf/bpftool/ V=1

  aarch64-linux-gnu-gcc ... -c -MMD -o /tmp/bpftool/disasm.o /home/z/src/linux/kernel/bpf/disasm.c
  gcc ... -c -MMD -o /tmp/bpftool//bootstrap/main.o main.c
  gcc ... -o /tmp/bpftool//bootstrap/bpftool /tmp/bpftool//bootstrap/main.o ... /tmp/bpftool/disasm.o
/usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
/usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
/usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
/usr/bin/ld: /tmp/bpftool/disasm.o: error adding symbols: file in wrong format
collect2: error: ld returned 1 exit status

The final bpftool is built for arm64, while the bootstrap bpftool,
executed on the host, is built for x86. The problem here is that disasm.o
linked into the bootstrap bpftool is arm64 rather than x86. With my fix we
build two disasm.o, one for the target bpftool in arm64, and one for the
bootstrap bpftool in x86.

> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >  tools/bpf/bpftool/Makefile | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index d16d289ade7a..d73232be1e99 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -136,7 +136,7 @@ endif
> >
> >  BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
> >
> > -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o) $(OUTPUT)disasm.o
> > +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o disasm.o)
> >  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> >
> >  VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)                           \
> > @@ -180,6 +180,9 @@ endif
> >
> >  CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
> >
> > +$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> > +       $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
> > +
> >  $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> 
> maybe just do
> 
> $(BOOTSTRAP_OUTPUT)disasm.o $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c

They need two different recipes. The bootstrap disasm.o needs to be built
with $(HOSTCC) rather than $(CC) (CC=aarch64-linux-gnu-gcc and HOSTCC=gcc)

Thanks,
Jean

> ?
> 
> >         $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<
> >
> > --
> > 2.31.1
> >
Andrii Nakryiko June 4, 2021, 5:02 p.m. UTC | #3
On Fri, Jun 4, 2021 at 12:51 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> On Thu, Jun 03, 2021 at 10:50:08AM -0700, Andrii Nakryiko wrote:
> > On Thu, Jun 3, 2021 at 10:06 AM Jean-Philippe Brucker
> > <jean-philippe@linaro.org> wrote:
> > >
> > > When the bootstrap and final bpftool have different architectures, we
> > > need to build two distinct disasm.o objects. Add a recipe for the
> > > bootstrap disasm.o
> > >
> > > Fixes: d510296d331a ("bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command.")
> >
> > Did this commit break something specifically?
>
> Yes, cross-building bpftool doesn't work anymore, because the bootstrap
> bpftool is linked using objects from different architectures:
>
> $ make O=/tmp/bpftool ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -C tools/bpf/bpftool/ V=1
>
>   aarch64-linux-gnu-gcc ... -c -MMD -o /tmp/bpftool/disasm.o /home/z/src/linux/kernel/bpf/disasm.c
>   gcc ... -c -MMD -o /tmp/bpftool//bootstrap/main.o main.c
>   gcc ... -o /tmp/bpftool//bootstrap/bpftool /tmp/bpftool//bootstrap/main.o ... /tmp/bpftool/disasm.o
> /usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
> /usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
> /usr/bin/ld: /tmp/bpftool/disasm.o: Relocations in generic ELF (EM: 183)
> /usr/bin/ld: /tmp/bpftool/disasm.o: error adding symbols: file in wrong format
> collect2: error: ld returned 1 exit status
>
> The final bpftool is built for arm64, while the bootstrap bpftool,
> executed on the host, is built for x86. The problem here is that disasm.o
> linked into the bootstrap bpftool is arm64 rather than x86. With my fix we
> build two disasm.o, one for the target bpftool in arm64, and one for the
> bootstrap bpftool in x86.

Oh, ok, that commit added disasm.o into bootstrap version of bpftool, thanks.

>
> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > ---
> > >  tools/bpf/bpftool/Makefile | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > > index d16d289ade7a..d73232be1e99 100644
> > > --- a/tools/bpf/bpftool/Makefile
> > > +++ b/tools/bpf/bpftool/Makefile
> > > @@ -136,7 +136,7 @@ endif
> > >
> > >  BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
> > >
> > > -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o) $(OUTPUT)disasm.o
> > > +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o disasm.o)
> > >  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> > >
> > >  VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)                           \
> > > @@ -180,6 +180,9 @@ endif
> > >
> > >  CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
> > >
> > > +$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> > > +       $(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
> > > +
> > >  $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> >
> > maybe just do
> >
> > $(BOOTSTRAP_OUTPUT)disasm.o $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
>
> They need two different recipes. The bootstrap disasm.o needs to be built
> with $(HOSTCC) rather than $(CC) (CC=aarch64-linux-gnu-gcc and HOSTCC=gcc)
>

My bad, missed HOSTCC vs CC in the rule. At a first glance they
appeared to be identical.

Your patch makes sense:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> Thanks,
> Jean
>
> > ?
> >
> > >         $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<
> > >
> > > --
> > > 2.31.1
> > >
diff mbox series

Patch

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index d16d289ade7a..d73232be1e99 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -136,7 +136,7 @@  endif
 
 BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
 
-BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o) $(OUTPUT)disasm.o
+BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o disasm.o)
 OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
 
 VMLINUX_BTF_PATHS ?= $(if $(O),$(O)/vmlinux)				\
@@ -180,6 +180,9 @@  endif
 
 CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
 
+$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
+	$(QUIET_CC)$(HOSTCC) $(CFLAGS) -c -MMD -o $@ $<
+
 $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
 	$(QUIET_CC)$(CC) $(CFLAGS) -c -MMD -o $@ $<