Re: [PATCH bpf v2] samples/bpf: remove unnecessary -I flags from libbpf EXTRA_CFLAGS

[Eduard Zingerman <eddyz87@gmail.com>]

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).