[PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS

public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
@ 2023-05-30 12:33 Viktor Malik
  2023-05-30 13:29 ` Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Viktor Malik @ 2023-05-30 12:33 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Ian Rogers,
	Shen Jiamin, Viktor Malik

Building BPF selftests with custom HOSTCFLAGS yields an error:

    # make HOSTCFLAGS="-O2"
    [...]
      HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
    main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
       73 | #include <linux/rbtree.h>
          |          ^~~~~~~~~~~~~~~~

The reason is that tools/bpf/resolve_btfids/Makefile passes header
include paths by extending HOSTCFLAGS which is overridden by setting
HOSTCFLAGS in the make command (because of Makefile rules [1]).

This patch fixes the above problem by passing the include paths via
`HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
and can be combined with overridding HOSTCFLAGS.

[1] https://www.gnu.org/software/make/manual/html_node/Overriding.html

Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 tools/bpf/resolve_btfids/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index ac548a7baa73..4b8079f294f6 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
 LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
 LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
 
-HOSTCFLAGS += -g \
+HOSTCFLAGS_resolve_btfids += -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
@@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
 
 LIBS = $(LIBELF_LIBS) -lz
 
-export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
+export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
 include $(srctree)/tools/build/Makefile.include
 
 $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
-- 
2.40.1


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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-30 12:33 [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS Viktor Malik
@ 2023-05-30 13:29 ` Jiri Olsa
  2023-05-31  7:20   ` Viktor Malik
  2023-06-05  9:45 ` Jiri Olsa
  2023-06-05 22:50 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2023-05-30 13:29 UTC (permalink / raw)
  To: Viktor Malik
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Ian Rogers, Shen Jiamin

On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> include paths by extending HOSTCFLAGS which is overridden by setting
> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> 
> This patch fixes the above problem by passing the include paths via
> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> and can be combined with overridding HOSTCFLAGS.
> 
> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> 
> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> ---
>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..4b8079f294f6 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS_resolve_btfids += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR

hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
to the libbpf and libsubcmd dependencies, right?

how about we add the EXTRA_CFLAGS variable like we do in libbpf,
libsubcmd or perf

with the change below you'd need to run:

  $ make EXTRA_CFLAGS="-O2"

I'll dig up the cross build scenarious we broke last time we
touched this stuff, perhaps Ian might remember as well ;-)

jirka


---
diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index ac548a7baa73..58cfedc9c2db 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -18,8 +18,8 @@ else
 endif
 
 # Overrides for the prepare step libraries.
-HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
-		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
+HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
+		  CROSS_COMPILE=""
 
 RM      ?= rm
 HOSTCC  ?= gcc
@@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
 LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
 LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
 
-HOSTCFLAGS += -g \
+HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
@@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
 
 LIBS = $(LIBELF_LIBS) -lz
 
-export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
+export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
 include $(srctree)/tools/build/Makefile.include
 
 $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)

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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-30 13:29 ` Jiri Olsa
@ 2023-05-31  7:20   ` Viktor Malik
  2023-05-31  8:34     ` Jiri Olsa
  0 siblings, 1 reply; 7+ messages in thread
From: Viktor Malik @ 2023-05-31  7:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Ian Rogers, Shen Jiamin

On 5/30/23 15:29, Jiri Olsa wrote:
> On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
>> Building BPF selftests with custom HOSTCFLAGS yields an error:
>>
>>     # make HOSTCFLAGS="-O2"
>>     [...]
>>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>>        73 | #include <linux/rbtree.h>
>>           |          ^~~~~~~~~~~~~~~~
>>
>> The reason is that tools/bpf/resolve_btfids/Makefile passes header
>> include paths by extending HOSTCFLAGS which is overridden by setting
>> HOSTCFLAGS in the make command (because of Makefile rules [1]).
>>
>> This patch fixes the above problem by passing the include paths via
>> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
>> and can be combined with overridding HOSTCFLAGS.
>>
>> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
>>
>> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>> ---
>>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>> index ac548a7baa73..4b8079f294f6 100644
>> --- a/tools/bpf/resolve_btfids/Makefile
>> +++ b/tools/bpf/resolve_btfids/Makefile
>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>  
>> -HOSTCFLAGS += -g \
>> +HOSTCFLAGS_resolve_btfids += -g \
>>            -I$(srctree)/tools/include \
>>            -I$(srctree)/tools/include/uapi \
>>            -I$(LIBBPF_INCLUDE) \
>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>  
>>  LIBS = $(LIBELF_LIBS) -lz
>>  
>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
> 
> hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
> to the libbpf and libsubcmd dependencies, right?

IIUC, it will, b/c we're doing:

    HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"

and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
then pick EXTRA_CFLAGS as a part of their build.

Confirmed for libsubcmd:

    $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
    14
    $ make V=1 | grep libsubcmd | grep O2 | wc -l
    0

Interestingly, I couldn't do the same for libbpf. It looks like libbpf
is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
$(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
libbpf is reused and that one hasn't picked HOSTCFLAGS.

> how about we add the EXTRA_CFLAGS variable like we do in libbpf,
> libsubcmd or perf
> 
> with the change below you'd need to run:
> 
>   $ make EXTRA_CFLAGS="-O2"
> 

I'd like to avoid that b/c then, we would need to issue a different make
command for the BPF selftests than for the rest of the kernel to pass
custom flags to host-built programs.

> I'll dig up the cross build scenarious we broke last time we
> touched this stuff, perhaps Ian might remember as well ;-)

That will be useful, thanks :-)

Viktor

> 
> jirka
> 
> 
> ---
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..58cfedc9c2db 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -18,8 +18,8 @@ else
>  endif
>  
>  # Overrides for the prepare step libraries.
> -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
> +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> +		  CROSS_COMPILE=""
>  
>  RM      ?= rm
>  HOSTCC  ?= gcc
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> 


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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-31  7:20   ` Viktor Malik
@ 2023-05-31  8:34     ` Jiri Olsa
  2023-05-31 11:40       ` Viktor Malik
  0 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2023-05-31  8:34 UTC (permalink / raw)
  To: Viktor Malik
  Cc: Jiri Olsa, bpf, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Ian Rogers,
	Shen Jiamin

On Wed, May 31, 2023 at 09:20:44AM +0200, Viktor Malik wrote:
> On 5/30/23 15:29, Jiri Olsa wrote:
> > On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> >> Building BPF selftests with custom HOSTCFLAGS yields an error:
> >>
> >>     # make HOSTCFLAGS="-O2"
> >>     [...]
> >>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
> >>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
> >>        73 | #include <linux/rbtree.h>
> >>           |          ^~~~~~~~~~~~~~~~
> >>
> >> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> >> include paths by extending HOSTCFLAGS which is overridden by setting
> >> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> >>
> >> This patch fixes the above problem by passing the include paths via
> >> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> >> and can be combined with overridding HOSTCFLAGS.
> >>
> >> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> >>
> >> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> >> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> >> ---
> >>  tools/bpf/resolve_btfids/Makefile | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> >> index ac548a7baa73..4b8079f294f6 100644
> >> --- a/tools/bpf/resolve_btfids/Makefile
> >> +++ b/tools/bpf/resolve_btfids/Makefile
> >> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> >>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> >>  
> >> -HOSTCFLAGS += -g \
> >> +HOSTCFLAGS_resolve_btfids += -g \
> >>            -I$(srctree)/tools/include \
> >>            -I$(srctree)/tools/include/uapi \
> >>            -I$(LIBBPF_INCLUDE) \
> >> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
> >>  
> >>  LIBS = $(LIBELF_LIBS) -lz
> >>  
> >> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> >> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
> > 
> > hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
> > to the libbpf and libsubcmd dependencies, right?
> 
> IIUC, it will, b/c we're doing:
> 
>     HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"
> 
> and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
> then pick EXTRA_CFLAGS as a part of their build.
> 
> Confirmed for libsubcmd:
> 
>     $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
>     14
>     $ make V=1 | grep libsubcmd | grep O2 | wc -l
>     0
> 
> Interestingly, I couldn't do the same for libbpf. It looks like libbpf
> is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
> $(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
> BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
> libbpf is reused and that one hasn't picked HOSTCFLAGS.
> 
> > how about we add the EXTRA_CFLAGS variable like we do in libbpf,
> > libsubcmd or perf
> > 
> > with the change below you'd need to run:
> > 
> >   $ make EXTRA_CFLAGS="-O2"
> > 
> 
> I'd like to avoid that b/c then, we would need to issue a different make
> command for the BPF selftests than for the rest of the kernel to pass
> custom flags to host-built programs.

ok

> 
> > I'll dig up the cross build scenarious we broke last time we
> > touched this stuff, perhaps Ian might remember as well ;-)
> 
> That will be useful, thanks :-)

there's test described by Nathan in here:

https://lore.kernel.org/bpf/Y9mFVNEi5wAINARY@dev-arch.thelio-3990X/

jirka

> 
> Viktor
> 
> > 
> > jirka
> > 
> > 
> > ---
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index ac548a7baa73..58cfedc9c2db 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -18,8 +18,8 @@ else
> >  endif
> >  
> >  # Overrides for the prepare step libraries.
> > -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> > -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
> > +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> > +		  CROSS_COMPILE=""
> >  
> >  RM      ?= rm
> >  HOSTCC  ?= gcc
> > @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> >  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> >  
> > -HOSTCFLAGS += -g \
> > +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
> >            -I$(srctree)/tools/include \
> >            -I$(srctree)/tools/include/uapi \
> >            -I$(LIBBPF_INCLUDE) \
> > @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
> >  
> >  LIBS = $(LIBELF_LIBS) -lz
> >  
> > -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> > +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
> >  include $(srctree)/tools/build/Makefile.include
> >  
> >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > 
> 

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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-31  8:34     ` Jiri Olsa
@ 2023-05-31 11:40       ` Viktor Malik
  0 siblings, 0 replies; 7+ messages in thread
From: Viktor Malik @ 2023-05-31 11:40 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Ian Rogers, Shen Jiamin

On 5/31/23 10:34, Jiri Olsa wrote:
> On Wed, May 31, 2023 at 09:20:44AM +0200, Viktor Malik wrote:
>> On 5/30/23 15:29, Jiri Olsa wrote:
>>> On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
>>>> Building BPF selftests with custom HOSTCFLAGS yields an error:
>>>>
>>>>     # make HOSTCFLAGS="-O2"
>>>>     [...]
>>>>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>>>>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>>>>        73 | #include <linux/rbtree.h>
>>>>           |          ^~~~~~~~~~~~~~~~
>>>>
>>>> The reason is that tools/bpf/resolve_btfids/Makefile passes header
>>>> include paths by extending HOSTCFLAGS which is overridden by setting
>>>> HOSTCFLAGS in the make command (because of Makefile rules [1]).
>>>>
>>>> This patch fixes the above problem by passing the include paths via
>>>> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
>>>> and can be combined with overridding HOSTCFLAGS.
>>>>
>>>> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
>>>>
>>>> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
>>>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>>>> ---
>>>>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>>>> index ac548a7baa73..4b8079f294f6 100644
>>>> --- a/tools/bpf/resolve_btfids/Makefile
>>>> +++ b/tools/bpf/resolve_btfids/Makefile
>>>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>>>  
>>>> -HOSTCFLAGS += -g \
>>>> +HOSTCFLAGS_resolve_btfids += -g \
>>>>            -I$(srctree)/tools/include \
>>>>            -I$(srctree)/tools/include/uapi \
>>>>            -I$(LIBBPF_INCLUDE) \
>>>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>>>  
>>>>  LIBS = $(LIBELF_LIBS) -lz
>>>>  
>>>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>>>> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
>>>
>>> hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
>>> to the libbpf and libsubcmd dependencies, right?
>>
>> IIUC, it will, b/c we're doing:
>>
>>     HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"
>>
>> and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
>> then pick EXTRA_CFLAGS as a part of their build.
>>
>> Confirmed for libsubcmd:
>>
>>     $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
>>     14
>>     $ make V=1 | grep libsubcmd | grep O2 | wc -l
>>     0
>>
>> Interestingly, I couldn't do the same for libbpf. It looks like libbpf
>> is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
>> $(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
>> BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
>> libbpf is reused and that one hasn't picked HOSTCFLAGS.
>>
>>> how about we add the EXTRA_CFLAGS variable like we do in libbpf,
>>> libsubcmd or perf
>>>
>>> with the change below you'd need to run:
>>>
>>>   $ make EXTRA_CFLAGS="-O2"
>>>
>>
>> I'd like to avoid that b/c then, we would need to issue a different make
>> command for the BPF selftests than for the rest of the kernel to pass
>> custom flags to host-built programs.
> 
> ok
> 
>>
>>> I'll dig up the cross build scenarious we broke last time we
>>> touched this stuff, perhaps Ian might remember as well ;-)
>>
>> That will be useful, thanks :-)
> 
> there's test described by Nathan in here:
> 
> https://lore.kernel.org/bpf/Y9mFVNEi5wAINARY@dev-arch.thelio-3990X/

Thanks, I ran the commands and it builds fine.

Running:
    $ make -j4 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 V=1 prepare

Current master:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -D"BUILD_STR(s)=#s"   -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

With this patch:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o  -D"BUILD_STR(s)=#s"  -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

With this patch and HOSTCFLAGS=-O2:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o -O2 -D"BUILD_STR(s)=#s"  -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

The flags in the first two are the same, they are just reordered. The
last one correctly adds -O2. Other than that, this patch only drops
resolve_btfids-specific flags (those -I...) from building
resolve_btfids/fixdep but that's, IIUC, fine.

Viktor

> 
> jirka
> 
>>
>> Viktor
>>
>>>
>>> jirka
>>>
>>>
>>> ---
>>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>>> index ac548a7baa73..58cfedc9c2db 100644
>>> --- a/tools/bpf/resolve_btfids/Makefile
>>> +++ b/tools/bpf/resolve_btfids/Makefile
>>> @@ -18,8 +18,8 @@ else
>>>  endif
>>>  
>>>  # Overrides for the prepare step libraries.
>>> -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
>>> -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
>>> +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
>>> +		  CROSS_COMPILE=""
>>>  
>>>  RM      ?= rm
>>>  HOSTCC  ?= gcc
>>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>>  
>>> -HOSTCFLAGS += -g \
>>> +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
>>>            -I$(srctree)/tools/include \
>>>            -I$(srctree)/tools/include/uapi \
>>>            -I$(LIBBPF_INCLUDE) \
>>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>>  
>>>  LIBS = $(LIBELF_LIBS) -lz
>>>  
>>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>>> +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
>>>  include $(srctree)/tools/build/Makefile.include
>>>  
>>>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
>>>
>>
> 


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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-30 12:33 [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS Viktor Malik
  2023-05-30 13:29 ` Jiri Olsa
@ 2023-06-05  9:45 ` Jiri Olsa
  2023-06-05 22:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2023-06-05  9:45 UTC (permalink / raw)
  To: Viktor Malik
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Ian Rogers, Shen Jiamin

On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> include paths by extending HOSTCFLAGS which is overridden by setting
> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> 
> This patch fixes the above problem by passing the include paths via
> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> and can be combined with overridding HOSTCFLAGS.
> 
> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> 
> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> Signed-off-by: Viktor Malik <vmalik@redhat.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..4b8079f294f6 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS_resolve_btfids += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> -- 
> 2.40.1
> 

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

* Re: [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
  2023-05-30 12:33 [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS Viktor Malik
  2023-05-30 13:29 ` Jiri Olsa
  2023-06-05  9:45 ` Jiri Olsa
@ 2023-06-05 22:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-05 22:50 UTC (permalink / raw)
  To: Viktor Malik
  Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
	kpsingh, sdf, haoluo, jolsa, irogers, shen_jiamin

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue, 30 May 2023 14:33:52 +0200 you wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> [...]

Here is the summary with links:
  - [bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
    https://git.kernel.org/bpf/bpf-next/c/edd75c802855

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2023-06-05 22:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-30 12:33 [PATCH bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS Viktor Malik
2023-05-30 13:29 ` Jiri Olsa
2023-05-31  7:20   ` Viktor Malik
2023-05-31  8:34     ` Jiri Olsa
2023-05-31 11:40       ` Viktor Malik
2023-06-05  9:45 ` Jiri Olsa
2023-06-05 22:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox