| Message ID | 20231016130307.35104-1-akihiko.odaki@daynix.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v4] selftests/bpf: Use pkg-config to determine ld flags | expand |
On 10/16/23 3:03 PM, Akihiko Odaki wrote: > When linking statically, libraries may require other dependencies to be > included to ld flags. In particular, libelf may require libzstd. Use > pkg-config to determine such dependencies. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > V3 -> V4: Added "2> /dev/null". > V2 -> V3: Added missing "echo". > V1 -> V2: Implemented fallback, referring to HOSTPKG_CONFIG. > > tools/testing/selftests/bpf/Makefile | 4 +++- > tools/testing/selftests/bpf/README.rst | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index caede9b574cb..009e907a8abe 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -4,6 +4,7 @@ include ../../../scripts/Makefile.arch > include ../../../scripts/Makefile.include > > CXX ?= $(CROSS_COMPILE)g++ > +PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config > > CURDIR := $(abspath .) > TOOLSDIR := $(abspath ../../..) > @@ -31,7 +32,8 @@ CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \ > -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ > -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) > LDFLAGS += $(SAN_LDFLAGS) > -LDLIBS += -lelf -lz -lrt -lpthread > +LDLIBS += $(shell $(PKG_CONFIG) --libs libelf zlib 2> /dev/null || echo -lelf -lz) \ > + -lrt -lpthread > > ifneq ($(LLVM),) > # Silence some warnings when compiled with clang Staring at tools/bpf/resolve_btfids/Makefile, I'm trying to understand why we cannot replicate something similar for BPF selftests? For example, with your patch, why is it necessary to now have PKG_CONFIG and another HOSTPKG_CONFIG var? What about the below? diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 4225f975fce3..62166d2f937d 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -29,13 +29,17 @@ SAN_CFLAGS ?= SAN_LDFLAGS ?= $(SAN_CFLAGS) RELEASE ?= OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0) + +LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null) +LIBELF_LIBS := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf) + CFLAGS += -g $(OPT_FLAGS) -rdynamic \ -Wall -Werror \ - $(GENFLAGS) $(SAN_CFLAGS) \ + $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_FLAGS) \ -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) LDFLAGS += $(SAN_LDFLAGS) -LDLIBS += -lelf -lz -lrt -lpthread +LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread ifneq ($(LLVM),) # Silence some warnings when compiled with clang
On 2023/10/17 23:15, Daniel Borkmann wrote: > On 10/16/23 3:03 PM, Akihiko Odaki wrote: >> When linking statically, libraries may require other dependencies to be >> included to ld flags. In particular, libelf may require libzstd. Use >> pkg-config to determine such dependencies. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> V3 -> V4: Added "2> /dev/null". >> V2 -> V3: Added missing "echo". >> V1 -> V2: Implemented fallback, referring to HOSTPKG_CONFIG. >> >> tools/testing/selftests/bpf/Makefile | 4 +++- >> tools/testing/selftests/bpf/README.rst | 2 +- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/bpf/Makefile >> b/tools/testing/selftests/bpf/Makefile >> index caede9b574cb..009e907a8abe 100644 >> --- a/tools/testing/selftests/bpf/Makefile >> +++ b/tools/testing/selftests/bpf/Makefile >> @@ -4,6 +4,7 @@ include ../../../scripts/Makefile.arch >> include ../../../scripts/Makefile.include >> CXX ?= $(CROSS_COMPILE)g++ >> +PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config >> CURDIR := $(abspath .) >> TOOLSDIR := $(abspath ../../..) >> @@ -31,7 +32,8 @@ CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) >> $(SAN_CFLAGS) \ >> -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ >> -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) >> LDFLAGS += $(SAN_LDFLAGS) >> -LDLIBS += -lelf -lz -lrt -lpthread >> +LDLIBS += $(shell $(PKG_CONFIG) --libs libelf zlib 2> /dev/null || >> echo -lelf -lz) \ >> + -lrt -lpthread >> ifneq ($(LLVM),) >> # Silence some warnings when compiled with clang > > Staring at tools/bpf/resolve_btfids/Makefile, I'm trying to understand > why we > cannot replicate something similar for BPF selftests? > > For example, with your patch, why is it necessary to now have PKG_CONFIG > and > another HOSTPKG_CONFIG var? It's because at least Debian does have wrappers of pkg-config for cross compile targets. You can find them below: https://packages.debian.org/search?searchon=contents&keywords=pkg-config&mode=path&suite=stable&arch=any > > What about the below? > > diff --git a/tools/testing/selftests/bpf/Makefile > b/tools/testing/selftests/bpf/Makefile > index 4225f975fce3..62166d2f937d 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -29,13 +29,17 @@ SAN_CFLAGS ?= > SAN_LDFLAGS ?= $(SAN_CFLAGS) > RELEASE ?= > OPT_FLAGS ?= $(if $(RELEASE),-O2,-O0) > + > +LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null) > +LIBELF_LIBS := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null > || echo -lelf) Having dedicated variables and checking --cflags are a good idea.
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index caede9b574cb..009e907a8abe 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -4,6 +4,7 @@ include ../../../scripts/Makefile.arch include ../../../scripts/Makefile.include CXX ?= $(CROSS_COMPILE)g++ +PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config CURDIR := $(abspath .) TOOLSDIR := $(abspath ../../..) @@ -31,7 +32,8 @@ CFLAGS += -g -O0 -rdynamic -Wall -Werror $(GENFLAGS) $(SAN_CFLAGS) \ -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) LDFLAGS += $(SAN_LDFLAGS) -LDLIBS += -lelf -lz -lrt -lpthread +LDLIBS += $(shell $(PKG_CONFIG) --libs libelf zlib 2> /dev/null || echo -lelf -lz) \ + -lrt -lpthread ifneq ($(LLVM),) # Silence some warnings when compiled with clang diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst index cb9b95702ac6..9af79c7a9b58 100644 --- a/tools/testing/selftests/bpf/README.rst +++ b/tools/testing/selftests/bpf/README.rst @@ -77,7 +77,7 @@ In case of linker errors when running selftests, try using static linking: .. code-block:: console - $ LDLIBS=-static vmtest.sh + $ LDLIBS=-static PKG_CONFIG='pkg-config --static' vmtest.sh .. note:: Some distros may not support static linking.
When linking statically, libraries may require other dependencies to be included to ld flags. In particular, libelf may require libzstd. Use pkg-config to determine such dependencies. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- V3 -> V4: Added "2> /dev/null". V2 -> V3: Added missing "echo". V1 -> V2: Implemented fallback, referring to HOSTPKG_CONFIG. tools/testing/selftests/bpf/Makefile | 4 +++- tools/testing/selftests/bpf/README.rst | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)