linux-kernel - Re: [PATCH bpf-next v4] selftests/bpf: Use pkg-config to determine ld flags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4037a83a-c6b6-6eab-1cb1-93339686c4e5@iogearbox.net>
Date:   Tue, 17 Oct 2023 16:15:57 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Akihiko Odaki <akihiko.odaki@...nix.com>
Cc:     Andrii Nakryiko <andrii@...nel.org>,
        Mykola Lysenko <mykolal@...com>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>,
        Yonghong Song <yonghong.song@...ux.dev>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Shuah Khan <shuah@...nel.org>, Nick Terrell <terrelln@...com>,
        bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org, bjorn@...nel.org
Subject: Re: [PATCH bpf-next v4] selftests/bpf: Use pkg-config to determine ld
 flags

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@...nix.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