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 <[email protected]>
---
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 $@
$(OUTPUT)libbpf.pc:
$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
--
2.17.1
On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
<[email protected]> 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 <[email protected]>
> ---
> 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
>
On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
>On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
><[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
--
Regards,
Ivan Khoronzhuk
On Wed, Sep 18, 2019 at 4:05 AM Ivan Khoronzhuk
<[email protected]> 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
> ><[email protected]> 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 <[email protected]>
> >> ---
> >> 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