[Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
Lang Daniel
d.lang at abatec.at
Mon Feb 13 12:24:18 UTC 2023
Hi Arnout, Francis,
> Hi Daniel, Francis,
>
> On 07/02/2023 21:10, Francis Laniel wrote:
>> Hi.
>>
>> Le lundi 28 novembre 2022, 16:06:20 CET Lang Daniel a écrit :
>>> libbpf >1.0.0 defines libbpf_bpf_link_type_str(enum bpf_link_type) in
>>> src/libbpf.h, which is included by host-pahole.
>>> bpf_link_type is defined in linux/bpf.h, therefore the comment stating
>>> that pahole doesn't need bpf.h is no longer valid.
>>>
>>> Fixes:
>>> -
>>> http://autobuild.buildroot.net/results/d126a4b6eca786402dc362c86f8df3addec3
>>> d217/
>>>
>>> Signed-off-by: Daniel Lang <d.lang at abatec.at>
>>> ---
>>> I wasn't able to reproduce the mentioned compile error in the kernel.
>>> The mentioned file (tools/lib/bpf/strset.c) shouldn't be compiled when
>>> compiling the kernel as it would recompile libbpf.
>>> ---
>>> package/libbpf/libbpf.mk | 17 +----------------
>>> 1 file changed, 1 insertion(+), 16 deletions(-)
>>>
>>> diff --git a/package/libbpf/libbpf.mk b/package/libbpf/libbpf.mk
>>> index 820f1dc4bf..0381fd833a 100644
>>> --- a/package/libbpf/libbpf.mk
>>> +++ b/package/libbpf/libbpf.mk
>>> @@ -39,26 +39,11 @@ define LIBBPF_INSTALL_TARGET_CMDS
>>> -C $(@D)/src install DESTDIR=$(TARGET_DIR)
>>> endef
>>>
>>> -# We need to install_uapi_headers so we have btf.h to compile
>>> -# host-pahole.
>>> -# Nonetheless, this target adds bpf.h which generates a conflict when
>>> -# building the kernel:
>>> -# In file included from libbpf_internal.h:17:0, from strset.c:9:
>>> -# relo_core.h:10:6: error: nested redefinition of 'enum bpf_core_relo_kind'
>>> -# enum bpf_core_relo_kind {
>>> -# ^~~~~~~~~~~~~~~~~~
>>> -# relo_core.h:10:6: error: redeclaration of 'enum bpf_core_relo_kind'
>>> -# In file included from libbpf_legacy.h:13:0,
>>> -# from libbpf_internal.h:16,
>>> -# from strset.c:9:
>>> -# /home/francis/buildroot/output/host/include/linux/bpf.h:6497:6: note:
>>> originally defined here -# enum bpf_core_relo_kind {
>>> -# So, better to remove remove it now since we do not need it to build
>>> +# We need to install_uapi_headers so we have bpf.h and btf.h to compile
>>> # host-pahole, the only user of host-libbpf.
>>> define HOST_LIBBPF_INSTALL_CMDS
>>> $(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) \
>>> -C $(@D)/src install install_uapi_headers DESTDIR=$(HOST_DIR)
>>> - rm $(HOST_DIR)/include/linux/bpf.h
>>> endef
>>>
>>> $(eval $(generic-package))
>>
>> Thank you for this patch and sorry for my very late reply.
>> Sadly, I do not think the patch addresses the underlying issue.
>> Indeed, this is not pahole which does not need this file, as pahole relies on
>> <linux/bpf.h> to be built.
>> So, the whole point of this comment and the rm is to avoid a problem when
>> building the kernel when pahole was built before.
>
> The current solution, however, also doesn't work, which is why Franci's patch
> [1] is needed to fix it again.
>
> However, we believe this is not the proper fix. The real issue is the following:
>
> - libbpf installs headers which are possibly incompatible with the kernel's
> headers. Normally this is not a problem, because you have either the kernel's or
> libbpf's headers installed, not both.
>
> - However, when we install libbpf headers in $(HOST_DIR)/include, the kernel
> build itself picks up those headers instead of its own internal headers. That is
> why bpf.h is removed here to begin with. If bpf.h is not removed (and the kernel
> headers are indeed incompatible with the libbpf ones), then the kernel build
> will fail.
>
> - There is however no reason why the kernel should pick up $(HOST_DIR)/include
> instead of its own internal headers. Normally it doesn't do that, because it
> supplies -I options to its own headers, and these have precedence over the
> system included /usr/include etc. However, we force HOSTCC="$(HOSTCC)
> $(HOST_CFLAGS)" and HOST_CFLAGS has -I$(HOST_DIR)/include. This one will come
> before all of the ones that the kernel adds and will have precedence.
>
> Therefore, we believe that the solution is to use -isystem instead of -I in the
> HOSTCC setting of the kernel. This is indeed what is already done for uboot:
>
> HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem
> /,$(HOST_CFLAGS)))" \
>
> Note that there is a risk that this by itself goes wrong as well. We tried at
> some point to use -isystem instead of -I in HOST_CFLAGS:
>
> commit 6f8162cf8c1abef7e0a4771fe0d6b26a28f5c2b6
> Author: David Raeman <draeman at bbn.com>
> Date: Mon Jul 25 21:52:26 2016
>
> package/Makefile.in should grab HOST_DIR headers using -isystem instead of -I.
>
>
> but this was reverted a copule of days later:
>
> commit 255b6f80d395ef048f46cfcf75dba690c56af657
> Author: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
> Date: Sat Jul 30 18:10:18 2016
>
> Revert "package/Makefile.in should grab HOST_DIR headers using -isystem
> instead of -I."
>
> (Unfortunately, the commit has no further explanation of what went wrong).
>
> So, could you (Daniel and Francis) try to apply this patch and replace -I with
> -isystem like is done for U-Boot, and do a bunch of kernel builds to see if it
> breaks anything?
I didn't apply the patch for package/Makefile.in as that would change the behavour
for all packages. I did however copy the logic used in the U-Boot.
-HOSTCC="$(HOSTCC) $(HOST_CFLAGS) $(HOST_LDFLAGS)" \
+HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS))) $(HOST_LDFLAGS)" \
And applied this patch to keep bpf.h
I tested the following config:
BR2_aarch64=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN_AARCH64_GLIBC_STABLE=y
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_CUSTOM_VERSION=y
BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="{VERSION}"
BR2_LINUX_KERNEL_USE_ARCH_DEFAULT_CONFIG=y
BR2_LINUX_KERNEL_CONFIG_FRAGMENT_FILES="pahole-kernel.config"
BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
BR2_LINUX_KERNEL_NEEDS_HOST_PAHOLE=y
and with a fragment for linux:
CONFIG_BPF_SYSCALL=y
CONFIG_BPF_UNPRIV_DEFAULT_OFF=n
CONFIG_DEBUG_INFO_REDUCED=n
CONFIG_DEBUG_INFO_COMPRESSED=n
CONFIG_DEBUG_INFO_BTF=y
CONFIG_SYSTEM_TRUSTED_KEYRING=y
where VERSION is one of:
5.2.21 5.3.18 5.4.231 5.5.19 5.6.19 5.7.19 5.8.18 5.9.16 5.10.167 5.11.22
5.12.19 5.13.19 5.14.21 5.15.93 5.16.20 5.17.15 5.18.19 5.19.17 6.0.19 6.1.11
Version 5.2, as far as I could work it out, is the version that introduced
the pahole dependency when CONFIG_DEBUG_INFO_BTF is set.
None-LTS versions after 5.10 and before 6.0 fail with:
LD vmlinux.o
MODPOST vmlinux.symvers
MODINFO modules.builtin.modinfo
GEN modules.builtin
LD .tmp_vmlinux.btf
BTF .btf.vmlinux.bin.o
LD .tmp_vmlinux.kallsyms1
KSYMS .tmp_vmlinux.kallsyms1.S
AS .tmp_vmlinux.kallsyms1.S
LD .tmp_vmlinux.kallsyms2
KSYMS .tmp_vmlinux.kallsyms2.S
AS .tmp_vmlinux.kallsyms2.S
LD vmlinux
BTFIDS vmlinux
FAILED: load BTF from vmlinux: Invalid argument
make[2]: *** [Makefile:1177: vmlinux] Error 255
make[1]: *** [package/pkg-generic.mk:293: /home/d.lang/ws/other/buildroot/output/build/linux-5.11.22/.stamp_built] Error 2
make: *** [Makefile:82: _all] Error 2
But that should be unrelated to the change in package/libbpf and HOSTCC.
I will try to work out the reason behind that error.
Should I create a patch to change HOSTCC or is additional testing required?
>
> For now, I've marked this patch as Changes Requested (it can only be applied
> if the bpf.h problem is solved in some other way), and marked Francis's patch
> [1] as Superseded.
>
> Regards,
> Arnout
>
>
> [1]
> https://patchwork.ozlabs.org/project/buildroot/patch/20220610165441.84812-2-flaniel@linux.microsoft.com/
>
>
>>
>> Sadly, I tested your patch and I hit the problem described in the comment
>> regarding redefinition of bpf_core_relo_kind.
>>
>> Did you also hit it or you were able to build the kernel with
>> BR2_LINUX_KERNEL_NEEDS_HOST_PAHOLE?
It seems like I was tested with a too recent kernel and originally preparing
this patch and therefore didn't hit this error.
>>
>>
>> Best regards and thank you in advance.
>>
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
More information about the buildroot
mailing list