All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
@ 2022-11-28 15:06 Lang Daniel via buildroot
  2023-02-07 20:10 ` Francis Laniel
  0 siblings, 1 reply; 6+ messages in thread
From: Lang Daniel via buildroot @ 2022-11-28 15:06 UTC (permalink / raw)
  To: buildroot; +Cc: Francis Laniel, Romain Naour

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/d126a4b6eca786402dc362c86f8df3addec3d217/

Signed-off-by: Daniel Lang <d.lang@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))
-- 
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
  2022-11-28 15:06 [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host Lang Daniel via buildroot
@ 2023-02-07 20:10 ` Francis Laniel
  2023-02-08 15:47   ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: Francis Laniel @ 2023-02-07 20:10 UTC (permalink / raw)
  To: buildroot, Lang Daniel; +Cc: Romain Naour

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

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?


Best regards and thank you in advance.


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
  2023-02-07 20:10 ` Francis Laniel
@ 2023-02-08 15:47   ` Arnout Vandecappelle
  2023-02-08 22:22     ` Francis Laniel
  0 siblings, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2023-02-08 15:47 UTC (permalink / raw)
  To: Francis Laniel, buildroot, Lang Daniel; +Cc: Romain Naour

  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@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@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@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?

  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?
> 
> 
> Best regards and thank you in advance.
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
  2023-02-08 15:47   ` Arnout Vandecappelle
@ 2023-02-08 22:22     ` Francis Laniel
  2023-02-13 12:24       ` Lang Daniel via buildroot
  0 siblings, 1 reply; 6+ messages in thread
From: Francis Laniel @ 2023-02-08 22:22 UTC (permalink / raw)
  To: buildroot, Lang Daniel, Arnout Vandecappelle; +Cc: Romain Naour

Hi.


Le mercredi 8 février 2023, 16:47:58 CET Arnout Vandecappelle a écrit :
>   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/d126a4b6eca786402dc362c86f8df3adde
> >> c3
> >> d217/
> >> 
> >> Signed-off-by: Daniel Lang <d.lang@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)))" \

Thank you for all the details! Using -isystem sounds like a really good idea!

> 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@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@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 will sure take a look and will come back here with my findings!

>   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?
> > 
> > 
> > Best regards and thank you in advance.
> > 
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot


Best regards.


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
  2023-02-08 22:22     ` Francis Laniel
@ 2023-02-13 12:24       ` Lang Daniel via buildroot
  2023-02-14 21:16         ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: Lang Daniel via buildroot @ 2023-02-13 12:24 UTC (permalink / raw)
  To: buildroot; +Cc: Francis Laniel, Romain Naour

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@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@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@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@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host
  2023-02-13 12:24       ` Lang Daniel via buildroot
@ 2023-02-14 21:16         ` Arnout Vandecappelle
  0 siblings, 0 replies; 6+ messages in thread
From: Arnout Vandecappelle @ 2023-02-14 21:16 UTC (permalink / raw)
  To: Lang Daniel, buildroot; +Cc: Francis Laniel, Romain Naour



On 13/02/2023 13:24, Lang Daniel wrote:
> Hi Arnout, Francis,
[snip]
>> 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@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@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.

  Yeah, that's the thing that turned out not to work before.

> 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)" \

  That was indeed what I meant.

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

  Since you said "non-LTS versions", I guess it's an upstream issue that got 
fixed in 6.1 and applied to 6.0.x and 5.15.x?


> Should I create a patch to change HOSTCC or is additional testing required?

  Yes please! And include your extensive test coverage in the commit message.

  Regards,
  Arnout

> 
>>
>>    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@buildroot.org
>>> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-14 21:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 15:06 [Buildroot] [PATCH] package/libbpf: Don't remove bpf.h on host Lang Daniel via buildroot
2023-02-07 20:10 ` Francis Laniel
2023-02-08 15:47   ` Arnout Vandecappelle
2023-02-08 22:22     ` Francis Laniel
2023-02-13 12:24       ` Lang Daniel via buildroot
2023-02-14 21:16         ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.