[v3,bpf-next,11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets
diff mbox series

Message ID 20190916105433.11404-12-ivan.khoronzhuk@linaro.org
State Superseded
Headers show
Series
  • samples: bpf: improve/fix cross-compilation
Related show

Commit Message

Ivan Khoronzhuk Sept. 16, 2019, 10:54 a.m. UTC
In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
correctly to build command, for instance when --sysroot is used or
external libraries are used, like -lelf, wich can be absent in
toolchain. This can be used for samples/bpf cross-compiling allowing
to get elf lib from sysroot.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 tools/lib/bpf/Makefile | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Andrii Nakryiko Sept. 18, 2019, 5:19 a.m. UTC | #1
On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> correctly to build command, for instance when --sysroot is used or
> external libraries are used, like -lelf, wich can be absent in
> toolchain. This can be used for samples/bpf cross-compiling allowing
> to get elf lib from sysroot.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  tools/lib/bpf/Makefile | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index c6f94cffe06e..bccfa556ef4e 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -94,6 +94,10 @@ else
>    CFLAGS := -g -Wall
>  endif
>
> +ifdef EXTRA_CXXFLAGS
> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> +endif
> +
>  ifeq ($(feature-libelf-mmap), 1)
>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>  endif
> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>
>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>         @ln -sf $(@F) $(OUTPUT)libbpf.so
>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>
> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>
>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@

Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
(and is actually recommended my make documentation way to do this).

But actually, there is no need to use C++ compiler here,
test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
using C compiler instead?

>
>  $(OUTPUT)libbpf.pc:
>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> --
> 2.17.1
>
Ivan Khoronzhuk Sept. 18, 2019, 11:05 a.m. UTC | #2
On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
>> correctly to build command, for instance when --sysroot is used or
>> external libraries are used, like -lelf, wich can be absent in
>> toolchain. This can be used for samples/bpf cross-compiling allowing
>> to get elf lib from sysroot.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>  tools/lib/bpf/Makefile | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index c6f94cffe06e..bccfa556ef4e 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -94,6 +94,10 @@ else
>>    CFLAGS := -g -Wall
>>  endif
>>
>> +ifdef EXTRA_CXXFLAGS
>> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
>> +endif
>> +
>>  ifeq ($(feature-libelf-mmap), 1)
>>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>>  endif
>> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>>
>>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
>> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
>> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>>         @ln -sf $(@F) $(OUTPUT)libbpf.so
>>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>>
>> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>>
>>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
>> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
>> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
>
>Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
>both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
>(and is actually recommended my make documentation way to do this).
It's good practice to follow existent style, I've done similar way as for
CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
smth else. And my goal is not to correct everything but embed my
functionality, series tool large w/o it.

>
>But actually, there is no need to use C++ compiler here,
>test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
>using C compiler instead?
Seems like, will try in next v.

>
>>
>>  $(OUTPUT)libbpf.pc:
>>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
>> --
>> 2.17.1
>>
Andrii Nakryiko Sept. 18, 2019, 9:42 p.m. UTC | #3
On Wed, Sep 18, 2019 at 4:05 AM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
> >On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
> ><ivan.khoronzhuk@linaro.org> wrote:
> >>
> >> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> >> correctly to build command, for instance when --sysroot is used or
> >> external libraries are used, like -lelf, wich can be absent in
> >> toolchain. This can be used for samples/bpf cross-compiling allowing
> >> to get elf lib from sysroot.
> >>
> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> ---
> >>  tools/lib/bpf/Makefile | 11 ++++++++---
> >>  1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> >> index c6f94cffe06e..bccfa556ef4e 100644
> >> --- a/tools/lib/bpf/Makefile
> >> +++ b/tools/lib/bpf/Makefile
> >> @@ -94,6 +94,10 @@ else
> >>    CFLAGS := -g -Wall
> >>  endif
> >>
> >> +ifdef EXTRA_CXXFLAGS
> >> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> >> +endif
> >> +
> >>  ifeq ($(feature-libelf-mmap), 1)
> >>    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
> >>  endif
> >> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
> >>  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
> >>
> >>  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> >> -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >> +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
> >> +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> >> +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >>         @ln -sf $(@F) $(OUTPUT)libbpf.so
> >>         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
> >>
> >> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
> >>         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> >>
> >>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> >> -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> >> +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
> >
> >Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
> >both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
> >(and is actually recommended my make documentation way to do this).
> It's good practice to follow existent style, I've done similar way as for
> CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
> smth else. And my goal is not to correct everything but embed my
> functionality, series tool large w/o it.

Alright, we'll have to eventually clean up this Makefile. What we do
with EXTRA_CFLAGS is not exactly correct, as in this Makefile
EXTRA_CFLAGS are overriding CFLAGS, instead of extending them, which
doesn't seem correct to me. BTW, bpftool does += instead of :=. All
this is avoided by just keeping CFLAGS and EXTRA_CFLAGS separate and
specifying both of them in $(CC)/$(CLANG) invocations. But feel free
to ignore this for now.


>
> >
> >But actually, there is no need to use C++ compiler here,
> >test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
> >using C compiler instead?
> Seems like, will try in next v.

Thanks!

>
> >
> >>
> >>  $(OUTPUT)libbpf.pc:
> >>         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> >> --
> >> 2.17.1
> >>
>
> --
> Regards,
> Ivan Khoronzhuk

Patch
diff mbox series

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index c6f94cffe06e..bccfa556ef4e 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -94,6 +94,10 @@  else
   CFLAGS := -g -Wall
 endif
 
+ifdef EXTRA_CXXFLAGS
+  CXXFLAGS := $(EXTRA_CXXFLAGS)
+endif
+
 ifeq ($(feature-libelf-mmap), 1)
   override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
 endif
@@ -176,8 +180,9 @@  $(BPF_IN): force elfdep bpfdep
 $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
 
 $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
-	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
-				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
+	$(QUIET_LINK)$(CC) $(LDFLAGS) \
+		--shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
+		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
 	@ln -sf $(@F) $(OUTPUT)libbpf.so
 	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
 
@@ -185,7 +190,7 @@  $(OUTPUT)libbpf.a: $(BPF_IN)
 	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
 
 $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
-	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
+	$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
 
 $(OUTPUT)libbpf.pc:
 	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \