netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Alexei Starovoitov" <alexei.starovoitov@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Michael Petlan <mpetlan@redhat.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	kubakici@wp.pl
Subject: Re: [PATCH bpf v3] bpftool: Allow to link libbpf dynamically
Date: Fri, 29 Nov 2019 11:24:20 +0100	[thread overview]
Message-ID: <f6e8f6d2-6155-3b20-9975-81e29e460915@iogearbox.net> (raw)
In-Reply-To: <20191128160712.1048793-1-toke@redhat.com>

On 11/28/19 5:07 PM, Toke Høiland-Jørgensen wrote:
> From: Jiri Olsa <jolsa@kernel.org>
> 
> Currently we support only static linking with kernel's libbpf
> (tools/lib/bpf). This patch adds LIBBPF_DYNAMIC compile variable
> that triggers libbpf detection and bpf dynamic linking:
> 
>    $ make -C tools/bpf/bpftool make LIBBPF_DYNAMIC=1
> 
> If libbpf is not installed, build (with LIBBPF_DYNAMIC=1) stops with:
> 
>    $ make -C tools/bpf/bpftool LIBBPF_DYNAMIC=1
>      Auto-detecting system features:
>      ...                        libbfd: [ on  ]
>      ...        disassembler-four-args: [ on  ]
>      ...                          zlib: [ on  ]
>      ...                        libbpf: [ OFF ]
> 
>    Makefile:102: *** Error: No libbpf devel library found, please install libbpf-devel or libbpf-dev.
> 
> Adding LIBBPF_DIR compile variable to allow linking with
> libbpf installed into specific directory:
> 
>    $ make -C tools/lib/bpf/ prefix=/tmp/libbpf/ install_lib install_headers
>    $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
> 
> It might be needed to clean build tree first because features
> framework does not detect the change properly:
> 
>    $ make -C tools/build/feature clean
>    $ make -C tools/bpf/bpftool/ clean
> 
> Since bpftool uses bits of libbpf that are not exported as public API in
> the .so version, we also pass in libbpf.a to the linker, which allows it to
> pick up the private functions from the static library without having to
> expose them as ABI.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> v3:
>    - Keep $(LIBBPF) in $LIBS, and just add -lbpf on top
>    - Fix typo in error message
> v2:
>    - Pass .a file to linker when dynamically linking, so bpftool can use
>      private functions from libbpf without exposing them as API.
> 
>   tools/bpf/bpftool/Makefile | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 39bc6f0f4f0b..15052dcaa39b 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -1,6 +1,15 @@
>   # SPDX-License-Identifier: GPL-2.0-only
> +# LIBBPF_DYNAMIC to enable libbpf dynamic linking.
> +
>   include ../../scripts/Makefile.include
>   include ../../scripts/utilities.mak
> +include ../../scripts/Makefile.arch
> +
> +ifeq ($(LP64), 1)
> +  libdir_relative = lib64
> +else
> +  libdir_relative = lib
> +endif
>   
>   ifeq ($(srctree),)
>   srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> @@ -63,6 +72,19 @@ RM ?= rm -f
>   FEATURE_USER = .bpftool
>   FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
>   FEATURE_DISPLAY = libbfd disassembler-four-args zlib
> +ifdef LIBBPF_DYNAMIC
> +  FEATURE_TESTS   += libbpf
> +  FEATURE_DISPLAY += libbpf
> +
> +  # for linking with debug library run:
> +  # make LIBBPF_DYNAMIC=1 LIBBPF_DIR=/opt/libbpf

The Makefile already has BPF_DIR which points right now to '$(srctree)/tools/lib/bpf/'
and LIBBPF_PATH for the final one and where $(LIBBPF_PATH)libbpf.a is expected to reside.
Can't we improve the Makefile to reuse and work with these instead of adding yet another
LIBBPF_DIR var which makes future changes in this area more confusing? The libbpf build
spills out libbpf.{a,so*} by default anyway.

Was wondering whether we could drop LIBBPF_DYNAMIC altogether and have some sort of auto
detection, but given for perf the `make LIBBPF_DYNAMIC=1` option was just applied to perf
tree it's probably better to stay consistent plus static linking would stay as-is for
preferred method for bpftool, so that part seems fine.

> +  ifdef LIBBPF_DIR
> +    LIBBPF_CFLAGS  := -I$(LIBBPF_DIR)/include
> +    LIBBPF_LDFLAGS := -L$(LIBBPF_DIR)/$(libdir_relative)
> +    FEATURE_CHECK_CFLAGS-libbpf  := $(LIBBPF_CFLAGS)
> +    FEATURE_CHECK_LDFLAGS-libbpf := $(LIBBPF_LDFLAGS)
> +  endif
> +endif
>   
>   check_feat := 1
>   NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
> @@ -88,6 +110,18 @@ ifeq ($(feature-reallocarray), 0)
>   CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
>   endif
>   
> +ifdef LIBBPF_DYNAMIC
> +  ifeq ($(feature-libbpf), 1)
> +    # bpftool uses non-exported functions from libbpf, so just add the dynamic
> +    # version of libbpf and let the linker figure it out
> +    LIBS    := -lbpf $(LIBS)

Seems okay as a workaround for bpftool and avoids getting into the realm of declaring
libbpf as another half-baked netlink library if we'd have exposed these. Ideally the
netlink symbols shouldn't be needed at all from libbpf, but I presume the rationale
back then was that given it's used internally in libbpf for some of the APIs and was
needed in bpftool's net subcommand as well later on, it avoided duplicating the code
given statically linked and both are in-tree anyway.

> +    CFLAGS  += $(LIBBPF_CFLAGS)
> +    LDFLAGS += $(LIBBPF_LDFLAGS)
> +  else
> +    dummy := $(error Error: No libbpf devel library found, please install libbpf-devel or libbpf-dev.)
> +  endif
> +endif
> +
>   include $(wildcard $(OUTPUT)*.d)
>   
>   all: $(OUTPUT)bpftool
> 

Thanks,
Daniel

  parent reply	other threads:[~2019-11-29 10:53 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-27  9:48 [PATCH 0/3] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
2019-11-27  9:48 ` [PATCH 1/3] perf tools: Allow to specify libbpf install directory Jiri Olsa
2019-11-27  9:48 ` [PATCH 2/3] libbpf: Export netlink functions used by bpftool Jiri Olsa
2019-11-27  9:48 ` [PATCH 3/3] bpftool: Allow to link libbpf dynamically Jiri Olsa
2019-11-27 13:38   ` Quentin Monnet
2019-11-27 14:15     ` Jiri Olsa
2019-11-27 14:24       ` Arnaldo Carvalho de Melo
2019-11-27 14:31         ` Quentin Monnet
2019-11-27 15:48           ` Arnaldo Carvalho de Melo
2019-11-27 15:52             ` Quentin Monnet
2019-11-27 15:59               ` Arnaldo Carvalho de Melo
2019-11-27 14:29       ` Quentin Monnet
2019-11-27 16:41   ` Alexei Starovoitov
2019-11-27 16:37 ` [PATCH 0/3] perf/bpftool: " Alexei Starovoitov
2019-11-27 18:44   ` Arnaldo Carvalho de Melo
2019-11-27 20:24   ` Andrii Nakryiko
2019-11-27 21:22     ` Daniel Borkmann
2019-11-27 22:47       ` Jiri Olsa
2019-11-28  9:06   ` Toke Høiland-Jørgensen
2019-11-28 14:53 ` [PATCH bpf v2] bpftool: " Toke Høiland-Jørgensen
2019-11-28 15:32   ` Quentin Monnet
2019-11-28 15:52     ` Toke Høiland-Jørgensen
2019-11-28 16:07   ` [PATCH bpf v3] " Toke Høiland-Jørgensen
2019-11-28 17:30     ` Quentin Monnet
2019-11-29  8:12     ` Jiri Olsa
2019-11-29  8:24       ` Toke Høiland-Jørgensen
2019-11-29 10:24     ` Daniel Borkmann [this message]
2019-11-29 14:00       ` Toke Høiland-Jørgensen
2019-11-29 23:56         ` Daniel Borkmann
2019-12-02  8:59           ` Jiri Olsa
2019-12-02 17:09 ` [PATCH 0/3] perf/bpftool: " Andrii Nakryiko
2019-12-02 18:08   ` Toke Høiland-Jørgensen
2019-12-02 18:42     ` Andrii Nakryiko
2019-12-02 18:54       ` Arnaldo Carvalho de Melo
2019-12-02 19:21       ` Jiri Olsa
2019-12-02 19:54         ` Andrii Nakryiko
2019-12-02 20:02           ` Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f6e8f6d2-6155-3b20-9975-81e29e460915@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andriin@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kubakici@wp.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpetlan@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).