[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fq7iecmqkibtexsbgvv5gyu5nva6ig7gcww56oskk2rvmoxfno@uohfy2x57tch>
Date: Fri, 2 Feb 2024 18:34:40 -0700
From: Daniel Xu <dxu@...uu.xyz>
To: Manu Bretelle <chantr4@...il.com>
Cc: linux-trace-kernel@...r.kernel.org, coreteam@...filter.org,
bpf@...r.kernel.org, linux-input@...r.kernel.org, cgroups@...r.kernel.org,
netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org, fsverity@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
netfilter-devel@...r.kernel.org, alexei.starovoitov@...il.com, olsajiri@...il.com,
quentin@...valent.com, alan.maguire@...cle.com, memxor@...il.com, vmalik@...hat.com
Subject: Re: [PATCH bpf-next v4 0/3] Annotate kfuncs in .BTF_ids section
Hi Manu,
On Fri, Feb 02, 2024 at 03:09:05PM -0800, Manu Bretelle wrote:
> On Sun, Jan 28, 2024 at 06:24:05PM -0700, Daniel Xu wrote:
> > === Description ===
> >
> > This is a bpf-treewide change that annotates all kfuncs as such inside
> > .BTF_ids. This annotation eventually allows us to automatically generate
> > kfunc prototypes from bpftool.
> >
> > We store this metadata inside a yet-unused flags field inside struct
> > btf_id_set8 (thanks Kumar!). pahole will be taught where to look.
> >
> > More details about the full chain of events are available in commit 3's
> > description.
> >
> > The accompanying pahole and bpftool changes can be viewed
> > here on these "frozen" branches [0][1].
> >
> > [0]: https://github.com/danobi/pahole/tree/kfunc_btf-v3-mailed
> > [1]: https://github.com/danobi/linux/tree/kfunc_bpftool-mailed
>
>
> I hit a similar issue to [0] on master
> 943b043aeecc ("selftests/bpf: Fix bench runner SIGSEGV")
> when cross-compiling on x86_64 (LE) to s390x (BE).
> I do have CONFIG_DEBUG_INFO_BTF enable and the issue would not trigger if
> I disabled CONFIG_DEBUG_INFO_BTF (and with the fix mentioned in [0]).
>
> What seems to happen is that `tools/resolve_btfids` is ran in the context of the
> host endianess and if I printk before the WARN_ON:
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index ef380e546952..a9ed7a1a4936 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8128,6 +8128,7 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
> * WARN() for initcall registrations that do not check errors.
> */
> if (!(kset->set->flags & BTF_SET8_KFUNCS)) {
> + printk("Flag 0x%08X, expected 0x%08X\n", kset->set->flags, BTF_SET8_KFUNCS);
> WARN_ON(!kset->owner);
> return -EINVAL;
> }
>
> the boot logs would show:
> Flag 0x01000000, expected 0x00000001
>
> The issue did not happen prior to
> 6f3189f38a3e ("bpf: treewide: Annotate BPF kfuncs in BTF")
> has only 0 was written before.
>
> It seems [1] will be addressing cross-compilation, but it did not fix it as is
> by just applying on top of master, so probably some of the changes will also need
> to be ported to `tools/include/linux/btf_ids.h`?
>
> A hacky workaround to cross-compilation I have is to apply:
>
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index 4b8079f294f6..b706e7ab066f 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -22,10 +22,10 @@ HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)
> CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
> RM ?= rm
> -HOSTCC ?= gcc
> -HOSTLD ?= ld
> -HOSTAR ?= ar
> -CROSS_COMPILE =
> +HOSTCC = $(CC)
> +HOSTLD = $(LD)
> +HOSTAR = $(AR)
> +#CROSS_COMPILE =
> OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
> @@ -56,16 +56,16 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> - DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> + DESTDIR=$(SUBCMD_DESTDIR) prefix= subdir= \
> $(abspath $@) install_headers
> $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT) \
> - DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
> + DESTDIR=$(LIBBPF_DESTDIR) prefix= subdir= \
> $(abspath $@) install_headers
> -LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> -LIBELF_LIBS := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> +LIBELF_FLAGS := $(shell $(PKG_CONFIG) libelf --cflags 2>/dev/null)
> +LIBELF_LIBS := $(shell $(PKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> HOSTCFLAGS_resolve_btfids += -g \
> -I$(srctree)/tools/include \
> @@ -84,7 +84,7 @@ $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> $(call msg,LINK,$@)
> - $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> + $(Q)$(CC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
> clean_objects := $(wildcard $(OUTPUT)/*.o \
> $(OUTPUT)/.*.o.cmd \
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index a38a3001527c..5cd193c04448 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -171,7 +171,7 @@ INCLUDE_DIR := $(SCRATCH_DIR)/include
> BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
> ifneq ($(CROSS_COMPILE),)
> HOST_BUILD_DIR := $(BUILD_DIR)/host
> -HOST_SCRATCH_DIR := $(OUTPUT)/host-tools
> +HOST_SCRATCH_DIR := $(SCRATCH_DIR)
> HOST_INCLUDE_DIR := $(HOST_SCRATCH_DIR)/include
> else
> HOST_BUILD_DIR := $(BUILD_DIR)
>
> This causes `resolve_btfids` to be compiled in the target endianess and gets
> magically run provided that the hosts has `qemu-s390x-static` and a functional
> binfmt_misc [2] on the host, but having this using host architecture per [1]
> is likely better.
This is kinda surprising to me. I don't recall seeing any code inside
resolve_btfids that touches the set8 flags field -- only the ids in the
flexible array member. Would be interested to see what the value of the
set8 flags field is before resolve_btfids is run.
I'm a bit busy until Sunday afternoon but I'll try to take a look around
then. Might be a good opportunity to play with poke [0].
Thanks,
Daniel
[0]: http://www.jemarch.net/poke
Powered by blists - more mailing lists