| Message ID | 20190916105433.11404-12-ivan.khoronzhuk@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | samples: bpf: improve/fix cross-compilation | expand |
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 >
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 >>
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
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)|" \
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(-)