On 12/02, Eduard Zingerman wrote:
> On Mon, 2024-12-02 at 17:44 -0800, Eduard Zingerman wrote:
> > On Mon, 2024-12-02 at 16:52 -0800, Eduard Zingerman wrote:
> >
> > [...]
> >
> > > > Naive question: why pass EXTRA_CFLAGS to libbpf at all? Can we drop it?
> > >
> > > This was added by the commit [0].
> > > As far as I understand, the idea is to pass the following flags:
> > >
> > > ifeq ($(ARCH), arm)
> > > # Strip all except -D__LINUX_ARM_ARCH__ option needed to handle linux
> > > # headers when arm instruction set identification is requested.
> > > ARM_ARCH_SELECTOR := $(filter -D__LINUX_ARM_ARCH__%, $(KBUILD_CFLAGS))
> > > ...
> > > TPROGS_CFLAGS += $(ARM_ARCH_SELECTOR)
> > > endif
> > >
> > > ifeq ($(ARCH), mips)
> > > TPROGS_CFLAGS += -D__SANE_USERSPACE_TYPES__
> > > ...
> > > endif
> > >
> > > Not sure if these are still necessary.
> > >
> > > [0] commit d8ceae91e9f0 ("samples/bpf: Provide C/LDFLAGS to libbpf")
> > >
> >
> > But this means that I should include sysroot part in the COMMON_CFLAGS.
> > I'll get the arm cross-compilation environment and double check.
> >
>
> So, I tested build as follows:
> - setup a debian chroot for 'testing';
> - added gcc-arm-linux-gnueabihf toolchain and dependencies necessary
> for kernel build (as in [0]) + clang-18 + qemu-system-arm + qemu-user-static;
> - cross-compiled kernel for ARM inside that chroot:
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make olddefconfig
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make -j
> - prepared an ARM sysroot (again, debian 'testing'):
> $ debootstrap --arch armhf --variant=buildd testing \
> /some/dir/trixie-armhf http://deb.debian.org/debian
> (and installed libelf-dev inside chroot)
> - compiled samples with the following command:
> $ CLANG=clang-18 LLC=llc-18 OPT=opt-18 LLVM_DIS=llvm-dis-18 \
> LLVM_OBJCOPY=llvm-objcopy-18 LLVM_READELF=llvm-readelf-18 \
> ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- \
> SYSROOT=/some/dir/trixie-armhf/ \
> make M=samples/bpf
>
> [0] https://docs.kernel.org/bpf/s390.html
>
> The compilation finishes successfully with and without EXTRA_CFLAGS
> passed to libbpf build. When EXTRA_CFLAGS are passed, I don't see any
> -D__LINUX_ARM_ARCH__% flags passed to libbpf build.
>
> Still, I'm hesitant to remove this flag, I'd prefer to post a v3
> covering sysroot flag and be done with this. E.g. as below:
>
> --- 8< ----------------------------------------------------
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index bcf103a4c14f..96a05e70ace3 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -146,13 +146,14 @@ ifeq ($(ARCH), x86)
> BPF_EXTRA_CFLAGS += -fcf-protection
> endif
>
> -TPROGS_CFLAGS += -Wall -O2
> -TPROGS_CFLAGS += -Wmissing-prototypes
> -TPROGS_CFLAGS += -Wstrict-prototypes
> -TPROGS_CFLAGS += $(call try-run,\
> +COMMON_CFLAGS += -Wall -O2
> +COMMON_CFLAGS += -Wmissing-prototypes
> +COMMON_CFLAGS += -Wstrict-prototypes
> +COMMON_CFLAGS += $(call try-run,\
> printf "int main() { return 0; }" |\
> $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,)
>
> +TPROGS_CFLAGS += $(COMMON_CFLAGS)
> TPROGS_CFLAGS += -I$(objtree)/usr/include
> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> TPROGS_CFLAGS += -I$(LIBBPF_INCLUDE)
> @@ -162,7 +163,7 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib
> TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
>
> ifdef SYSROOT
> -TPROGS_CFLAGS += --sysroot=$(SYSROOT)
> +COMMON_CFLAGS += --sysroot=$(SYSROOT)
> TPROGS_LDFLAGS := -L$(SYSROOT)/usr/lib
> endif
>
> @@ -229,7 +230,7 @@ clean:
>
> $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUTPUT)
> # Fix up variables inherited from Kbuild that tools/ build system won't like
> - $(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
> + $(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(COMMON_CFLAGS)" \
> LDFLAGS="$(TPROGS_LDFLAGS)" srctree=$(BPF_SAMPLES_PATH)/../../ \
> O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> $@ install_headers
> ---------------------------------------------------- >8 ---
>
> (and maybe peek a better name for COMMON_CFLAGS).
>
Agreed, let's go with what you have (especially since you've tested it).
The samples are mostly deprecated / in maintenance mode anyway.
Feel free to slap:
Acked-by: Stanislav Fomichev <sdf@xxxxxxxxxxx>
