hi,
adding support to link bpftool with libbpf dynamically,
and config change for perf.
It's now possible to use:
$ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
which will detect libbpf devel package with needed version,
and if found, link it with bpftool.
It's possible to use arbitrary installed libbpf:
$ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
I based this change on top of Arnaldo's perf/core, because
it contains libbpf feature detection code as dependency.
It's now also synced with latest bpf-next, so Toke's change
applies correctly.
Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
libbpf/dyn
thanks,
jirka
---
Jiri Olsa (2):
perf tools: Allow to specify libbpf install directory
bpftool: Allow to link libbpf dynamically
Toke Høiland-Jørgensen (1):
libbpf: Export netlink functions used by bpftool
tools/bpf/bpftool/Makefile | 40 +++++++++++++++++++++++++++++++++++++++-
tools/build/feature/test-libbpf.c | 9 +++++++++
tools/lib/bpf/libbpf.h | 22 +++++++++++++---------
tools/lib/bpf/libbpf.map | 7 +++++++
tools/lib/bpf/nlattr.h | 15 ++++++++++-----
tools/perf/Makefile.config | 27 ++++++++++++++++++++-------
6 files changed, 98 insertions(+), 22 deletions(-)
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/perf/ 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/perf/ clean
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Makefile.config | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index c90f4146e5a2..eb9d9b70b7d3 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -22,6 +22,14 @@ include $(srctree)/tools/scripts/Makefile.arch
$(call detected_var,SRCARCH)
+ifndef lib
+ ifeq ($(SRCARCH)$(IS_64_BIT), x861)
+ lib = lib64
+ else
+ lib = lib
+ endif
+endif # lib
+
NO_PERF_REGS := 1
NO_SYSCALL_TABLE := 1
@@ -484,11 +492,22 @@ ifndef NO_LIBELF
CFLAGS += -DHAVE_LIBBPF_SUPPORT
$(call detected,CONFIG_LIBBPF)
+ # for linking with debug library run:
+ # make DEBUG=1 LIBBPF_DIR=/opt/libbpf
+ ifdef LIBBPF_DIR
+ LIBBPF_CFLAGS := -I$(LIBBPF_DIR)/include
+ LIBBPF_LDFLAGS := -L$(LIBBPF_DIR)/$(lib)
+ FEATURE_CHECK_CFLAGS-libbpf := $(LIBBPF_CFLAGS)
+ FEATURE_CHECK_LDFLAGS-libbpf := $(LIBBPF_LDFLAGS)
+ endif
+
# detecting libbpf without LIBBPF_DYNAMIC, so make VF=1 shows libbpf detection status
$(call feature_check,libbpf)
ifdef LIBBPF_DYNAMIC
ifeq ($(feature-libbpf), 1)
EXTLIBS += -lbpf
+ CFLAGS += $(LIBBPF_CFLAGS)
+ LDFLAGS += $(LIBBPF_LDFLAGS)
else
dummy := $(error Error: No libbpf devel library found, please install libbpf-devel);
endif
@@ -1037,13 +1056,6 @@ else
sysconfdir = $(prefix)/etc
ETC_PERFCONFIG = etc/perfconfig
endif
-ifndef lib
-ifeq ($(SRCARCH)$(IS_64_BIT), x861)
-lib = lib64
-else
-lib = lib
-endif
-endif # lib
libdir = $(prefix)/$(lib)
# Shell quote (do not use $(call) to accommodate ancient setups);
@@ -1108,6 +1120,7 @@ ifeq ($(VF),1)
$(call print_var,LIBUNWIND_DIR)
$(call print_var,LIBDW_DIR)
$(call print_var,JDIR)
+ $(call print_var,LIBBPF_DIR)
ifeq ($(dwarf-post-unwind),1)
$(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text))
--
2.21.0
From: Toke Høiland-Jørgensen <[email protected]>
There are a couple of netlink-related symbols in libbpf that is used by
bpftool, but not exported in the .so version of libbpf. This makes it
impossible to link bpftool dynamically against libbpf. Fix this by adding
the missing function exports.
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
tools/lib/bpf/libbpf.h | 22 +++++++++++++---------
tools/lib/bpf/libbpf.map | 7 +++++++
tools/lib/bpf/nlattr.h | 15 ++++++++++-----
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 0dbf4bfba0c4..ba32eb0b2b99 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -514,15 +514,19 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
struct nlattr;
typedef int (*libbpf_dump_nlmsg_t)(void *cookie, void *msg, struct nlattr **tb);
-int libbpf_netlink_open(unsigned int *nl_pid);
-int libbpf_nl_get_link(int sock, unsigned int nl_pid,
- libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie);
-int libbpf_nl_get_class(int sock, unsigned int nl_pid, int ifindex,
- libbpf_dump_nlmsg_t dump_class_nlmsg, void *cookie);
-int libbpf_nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
- libbpf_dump_nlmsg_t dump_qdisc_nlmsg, void *cookie);
-int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
- libbpf_dump_nlmsg_t dump_filter_nlmsg, void *cookie);
+LIBBPF_API int libbpf_netlink_open(unsigned int *nl_pid);
+LIBBPF_API int libbpf_nl_get_link(int sock, unsigned int nl_pid,
+ libbpf_dump_nlmsg_t dump_link_nlmsg,
+ void *cookie);
+LIBBPF_API int libbpf_nl_get_class(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_class_nlmsg,
+ void *cookie);
+LIBBPF_API int libbpf_nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_qdisc_nlmsg,
+ void *cookie);
+LIBBPF_API int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex,
+ int handle, libbpf_dump_nlmsg_t dump_filter_nlmsg,
+ void *cookie);
struct bpf_prog_linfo;
struct bpf_prog_info;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 8ddc2c40e482..fbd08911ec9e 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -207,4 +207,11 @@ LIBBPF_0.0.6 {
bpf_program__size;
btf__find_by_name_kind;
libbpf_find_vmlinux_btf_id;
+ libbpf_netlink_open;
+ libbpf_nl_get_class;
+ libbpf_nl_get_filter;
+ libbpf_nl_get_link;
+ libbpf_nl_get_qdisc;
+ libbpf_nla_parse;
+ libbpf_nla_parse_nested;
} LIBBPF_0.0.5;
diff --git a/tools/lib/bpf/nlattr.h b/tools/lib/bpf/nlattr.h
index 6cc3ac91690f..91119ff5701a 100644
--- a/tools/lib/bpf/nlattr.h
+++ b/tools/lib/bpf/nlattr.h
@@ -14,6 +14,10 @@
/* avoid multiple definition of netlink features */
#define __LINUX_NETLINK_H
+#ifndef LIBBPF_API
+#define LIBBPF_API __attribute__((visibility("default")))
+#endif
+
/**
* Standard attribute types to specify validation policy
*/
@@ -95,11 +99,12 @@ static inline int libbpf_nla_len(const struct nlattr *nla)
return nla->nla_len - NLA_HDRLEN;
}
-int libbpf_nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head,
- int len, struct libbpf_nla_policy *policy);
-int libbpf_nla_parse_nested(struct nlattr *tb[], int maxtype,
- struct nlattr *nla,
- struct libbpf_nla_policy *policy);
+LIBBPF_API int libbpf_nla_parse(struct nlattr *tb[], int maxtype,
+ struct nlattr *head, int len,
+ struct libbpf_nla_policy *policy);
+LIBBPF_API int libbpf_nla_parse_nested(struct nlattr *tb[], int maxtype,
+ struct nlattr *nla,
+ struct libbpf_nla_policy *policy);
int libbpf_nla_dump_errormsg(struct nlmsghdr *nlh);
--
2.21.0
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: libbpf-devel is missing, please install it. Stop.
Adding specific bpftool's libbpf check for libbpf_netlink_open (LIBBPF_0.0.6)
which is the latest we need for bpftool at the moment.
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
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/bpf/bpftool/Makefile | 40 ++++++++++++++++++++++++++++++-
tools/build/feature/test-libbpf.c | 9 +++++++
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 39bc6f0f4f0b..2b6ed08cb31e 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)))
@@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
LDFLAGS += $(EXTRA_LDFLAGS)
endif
-LIBS = $(LIBBPF) -lelf -lz
+LIBS = -lelf -lz
INSTALL ?= install
RM ?= rm -f
@@ -64,6 +73,23 @@ FEATURE_USER = .bpftool
FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
FEATURE_DISPLAY = libbfd disassembler-four-args zlib
+ifdef LIBBPF_DYNAMIC
+ # Add libbpf check with the flags to ensure bpftool
+ # specific version is detected.
+ FEATURE_CHECK_CFLAGS-libbpf := -DBPFTOOL
+ FEATURE_TESTS += libbpf
+ FEATURE_DISPLAY += libbpf
+
+ # for linking with debug library run:
+ # make LIBBPF_DYNAMIC=1 LIBBPF_DIR=/opt/libbpf
+ 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
ifdef MAKECMDGOALS
@@ -88,6 +114,18 @@ ifeq ($(feature-reallocarray), 0)
CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
endif
+ifdef LIBBPF_DYNAMIC
+ ifeq ($(feature-libbpf), 1)
+ LIBS += -lbpf
+ CFLAGS += $(LIBBPF_CFLAGS)
+ LDFLAGS += $(LIBBPF_LDFLAGS)
+ else
+ dummy := $(error Error: No libbpf devel library found, please install libbpf-devel)
+ endif
+else
+ LIBS += $(LIBBPF)
+endif
+
include $(wildcard $(OUTPUT)*.d)
all: $(OUTPUT)bpftool
diff --git a/tools/build/feature/test-libbpf.c b/tools/build/feature/test-libbpf.c
index a508756cf4cc..93566d105a64 100644
--- a/tools/build/feature/test-libbpf.c
+++ b/tools/build/feature/test-libbpf.c
@@ -3,5 +3,14 @@
int main(void)
{
+#ifdef BPFTOOL
+ /*
+ * libbpf_netlink_open (LIBBPF_0.0.6) is the latest
+ * we need for bpftool at the moment
+ */
+ libbpf_netlink_open(NULL);
+ return 0;
+#else
return bpf_object__open("test") ? 0 : -1;
+#endif
}
--
2.21.0
2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
> 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: libbpf-devel is missing, please install it. Stop.
>
> Adding specific bpftool's libbpf check for libbpf_netlink_open (LIBBPF_0.0.6)
> which is the latest we need for bpftool at the moment.
>
> 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
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> tools/bpf/bpftool/Makefile | 40 ++++++++++++++++++++++++++++++-
> tools/build/feature/test-libbpf.c | 9 +++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 39bc6f0f4f0b..2b6ed08cb31e 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> LDFLAGS += $(EXTRA_LDFLAGS)
> endif
>
> -LIBS = $(LIBBPF) -lelf -lz
> +LIBS = -lelf -lz
Hi Jiri,
This change seems to be breaking the build with the static library for
me. I know you add back $(LIBBPF) later in the Makefile, see at the end
of this email...
>
> INSTALL ?= install
> RM ?= rm -f
> @@ -64,6 +73,23 @@ FEATURE_USER = .bpftool
> FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
> FEATURE_DISPLAY = libbfd disassembler-four-args zlib
>
> +ifdef LIBBPF_DYNAMIC
> + # Add libbpf check with the flags to ensure bpftool
> + # specific version is detected.
Nit: We do not check for a specific bpftool version, we check for a
recent enough libbpf version?
> + FEATURE_CHECK_CFLAGS-libbpf := -DBPFTOOL
> + FEATURE_TESTS += libbpf
> + FEATURE_DISPLAY += libbpf
> +
> + # for linking with debug library run:
> + # make LIBBPF_DYNAMIC=1 LIBBPF_DIR=/opt/libbpf
> + 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
> ifdef MAKECMDGOALS
> @@ -88,6 +114,18 @@ ifeq ($(feature-reallocarray), 0)
> CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> endif
>
> +ifdef LIBBPF_DYNAMIC
> + ifeq ($(feature-libbpf), 1)
> + LIBS += -lbpf
> + CFLAGS += $(LIBBPF_CFLAGS)
> + LDFLAGS += $(LIBBPF_LDFLAGS)
> + else
> + dummy := $(error Error: No libbpf devel library found, please install libbpf-devel)
libbpf-devel sounds like a RH/Fedora package name, but other
distributions might have different names (Debian/Ubuntu would go by
libbpf-dev I suppose, although I don't believe such package exists at
the moment). Maybe use a more generic message?
> + endif
> +else
> + LIBS += $(LIBBPF)
... I believe the order of the libraries is relevant, and it seems the
static libbpf should be passed before the dynamic libs. Here I could fix
the build with the static library on my setup by prepending the library
path instead, like this:
LIBS := $(LIBBPF) $(LIBS)
On the plus side, all build attempts from
tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
my setup with dynamic linking from your branch.
> +endif
> +
> include $(wildcard $(OUTPUT)*.d)
>
> all: $(OUTPUT)bpftool
Thanks,
Quentin
On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
> > 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: libbpf-devel is missing, please install it. Stop.
> >
> > Adding specific bpftool's libbpf check for libbpf_netlink_open (LIBBPF_0.0.6)
> > which is the latest we need for bpftool at the moment.
> >
> > 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
> >
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
> > tools/bpf/bpftool/Makefile | 40 ++++++++++++++++++++++++++++++-
> > tools/build/feature/test-libbpf.c | 9 +++++++
> > 2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 39bc6f0f4f0b..2b6ed08cb31e 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
>
> > @@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> > LDFLAGS += $(EXTRA_LDFLAGS)
> > endif
> >
> > -LIBS = $(LIBBPF) -lelf -lz
> > +LIBS = -lelf -lz
>
> Hi Jiri,
>
> This change seems to be breaking the build with the static library for
> me. I know you add back $(LIBBPF) later in the Makefile, see at the end
> of this email...
>
> >
> > INSTALL ?= install
> > RM ?= rm -f
> > @@ -64,6 +73,23 @@ FEATURE_USER = .bpftool
> > FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
> > FEATURE_DISPLAY = libbfd disassembler-four-args zlib
> >
> > +ifdef LIBBPF_DYNAMIC
> > + # Add libbpf check with the flags to ensure bpftool
> > + # specific version is detected.
>
> Nit: We do not check for a specific bpftool version, we check for a
> recent enough libbpf version?
hi,
we check for a version that has the latest exported function
that bpftool needs, which is currently libbpf_netlink_open
please check the '#ifdef BPFTOOL' in feature/test-libbpf.c
it's like that because there's currently no support to check
for particular library version in the build/features framework
I'll make that comment more clear
>
> > + FEATURE_CHECK_CFLAGS-libbpf := -DBPFTOOL
> > + FEATURE_TESTS += libbpf
> > + FEATURE_DISPLAY += libbpf
> > +
> > + # for linking with debug library run:
> > + # make LIBBPF_DYNAMIC=1 LIBBPF_DIR=/opt/libbpf
> > + 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
> > ifdef MAKECMDGOALS
> > @@ -88,6 +114,18 @@ ifeq ($(feature-reallocarray), 0)
> > CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> > endif
> >
> > +ifdef LIBBPF_DYNAMIC
> > + ifeq ($(feature-libbpf), 1)
> > + LIBS += -lbpf
> > + CFLAGS += $(LIBBPF_CFLAGS)
> > + LDFLAGS += $(LIBBPF_LDFLAGS)
> > + else
> > + dummy := $(error Error: No libbpf devel library found, please install libbpf-devel)
>
> libbpf-devel sounds like a RH/Fedora package name, but other
> distributions might have different names (Debian/Ubuntu would go by
> libbpf-dev I suppose, although I don't believe such package exists at
> the moment). Maybe use a more generic message?
sure, actually in perf we use both package names like:
Error: No libbpf devel library found, please install libbpf-devel or libbpf-dev.
or we can go with generic message:
Error: No libbpf devel library found, please install.
>
> > + endif
> > +else
> > + LIBS += $(LIBBPF)
>
> ... I believe the order of the libraries is relevant, and it seems the
> static libbpf should be passed before the dynamic libs. Here I could fix
> the build with the static library on my setup by prepending the library
> path instead, like this:
>
> LIBS := $(LIBBPF) $(LIBS)
could you please paste the build error? I don't see any on Fedora,
anyway I can make the change you're proposing
>
> On the plus side, all build attempts from
> tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
> my setup with dynamic linking from your branch.
cool, had no idea there was such test ;-)
thanks,
jirka
Em Wed, Nov 27, 2019 at 03:15:20PM +0100, Jiri Olsa escreveu:
> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
> > 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
> > On the plus side, all build attempts from
> > tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
> > my setup with dynamic linking from your branch.
>
> cool, had no idea there was such test ;-)
Should be the the equivalent to 'make -C tools/perf build-test' :-)
Perhaps we should make tools/testing/selftests/perf/ link to that?
- Arnaldo
2019-11-27 15:15 UTC+0100 ~ Jiri Olsa <[email protected]>
> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
>> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
>>> 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: libbpf-devel is missing, please install it. Stop.
>>>
>>> Adding specific bpftool's libbpf check for libbpf_netlink_open (LIBBPF_0.0.6)
>>> which is the latest we need for bpftool at the moment.
>>>
>>> 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
>>>
>>> Signed-off-by: Jiri Olsa <[email protected]>
>>> ---
>>> tools/bpf/bpftool/Makefile | 40 ++++++++++++++++++++++++++++++-
>>> tools/build/feature/test-libbpf.c | 9 +++++++
>>> 2 files changed, 48 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
>>> index 39bc6f0f4f0b..2b6ed08cb31e 100644
>>> --- a/tools/bpf/bpftool/Makefile
>>> +++ b/tools/bpf/bpftool/Makefile
>>
>>> @@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
>>> LDFLAGS += $(EXTRA_LDFLAGS)
>>> endif
>>>
>>> -LIBS = $(LIBBPF) -lelf -lz
>>> +LIBS = -lelf -lz
>>
>> Hi Jiri,
>>
>> This change seems to be breaking the build with the static library for
>> me. I know you add back $(LIBBPF) later in the Makefile, see at the end
>> of this email...
>>
>>>
>>> INSTALL ?= install
>>> RM ?= rm -f
>>> @@ -64,6 +73,23 @@ FEATURE_USER = .bpftool
>>> FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
>>> FEATURE_DISPLAY = libbfd disassembler-four-args zlib
>>>
>>> +ifdef LIBBPF_DYNAMIC
>>> + # Add libbpf check with the flags to ensure bpftool
>>> + # specific version is detected.
>>
>> Nit: We do not check for a specific bpftool version, we check for a
>> recent enough libbpf version?
>
> hi,
> we check for a version that has the latest exported function
> that bpftool needs, which is currently libbpf_netlink_open
>
> please check the '#ifdef BPFTOOL' in feature/test-libbpf.c
>
> it's like that because there's currently no support to check
> for particular library version in the build/features framework
>
> I'll make that comment more clear
Thanks!
>>> + FEATURE_CHECK_CFLAGS-libbpf := -DBPFTOOL
>>> + FEATURE_TESTS += libbpf
>>> + FEATURE_DISPLAY += libbpf
>>> +
>>> + # for linking with debug library run:
>>> + # make LIBBPF_DYNAMIC=1 LIBBPF_DIR=/opt/libbpf
>>> + 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
>>> ifdef MAKECMDGOALS
>>> @@ -88,6 +114,18 @@ ifeq ($(feature-reallocarray), 0)
>>> CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
>>> endif
>>>
>>> +ifdef LIBBPF_DYNAMIC
>>> + ifeq ($(feature-libbpf), 1)
>>> + LIBS += -lbpf
>>> + CFLAGS += $(LIBBPF_CFLAGS)
>>> + LDFLAGS += $(LIBBPF_LDFLAGS)
>>> + else
>>> + dummy := $(error Error: No libbpf devel library found, please install libbpf-devel)
>>
>> libbpf-devel sounds like a RH/Fedora package name, but other
>> distributions might have different names (Debian/Ubuntu would go by
>> libbpf-dev I suppose, although I don't believe such package exists at
>> the moment). Maybe use a more generic message?
>
> sure, actually in perf we use both package names like:
>
> Error: No libbpf devel library found, please install libbpf-devel or libbpf-dev.
>
> or we can go with generic message:
>
> Error: No libbpf devel library found, please install.
Either is fine with me, thanks!
>>> + endif
>>> +else
>>> + LIBS += $(LIBBPF)
>>
>> ... I believe the order of the libraries is relevant, and it seems the
>> static libbpf should be passed before the dynamic libs. Here I could fix
>> the build with the static library on my setup by prepending the library
>> path instead, like this:
>>
>> LIBS := $(LIBBPF) $(LIBS)
>
> could you please paste the build error? I don't see any on Fedora,
Sure, here it is. On top of your branch (on Ubuntu 18.04):
----------------
$ make -C tools/bpf/bpftool/
make: Entering directory '/root/linux/bpf-next/tools/bpf/bpftool'
Auto-detecting system features:
... libbfd: [ on ]
... disassembler-four-args: [ on ]
... zlib: [ on ]
CC map_perf_ring.o
CC xlated_dumper.o
CC btf.o
CC tracelog.o
CC perf.o
CC cfg.o
CC btf_dumper.o
CC net.o
CC netlink_dumper.o
CC common.o
CC cgroup.o
CC main.o
CC json_writer.o
CC prog.o
CC map.o
CC feature.o
CC jit_disasm.o
CC disasm.o
make[1]: Entering directory '/root/linux/bpf-next/tools/lib/bpf'
Auto-detecting system features:
... libelf: [ on ]
... bpf: [ on ]
Parsed description of 117 helper function(s)
MKDIR staticobjs/
CC staticobjs/libbpf.o
CC staticobjs/bpf.o
CC staticobjs/nlattr.o
CC staticobjs/btf.o
CC staticobjs/libbpf_errno.o
CC staticobjs/str_error.o
CC staticobjs/netlink.o
CC staticobjs/bpf_prog_linfo.o
CC staticobjs/libbpf_probes.o
CC staticobjs/xsk.o
CC staticobjs/hashmap.o
CC staticobjs/btf_dump.o
LD staticobjs/libbpf-in.o
LINK libbpf.a
make[1]: Leaving directory '/root/linux/bpf-next/tools/lib/bpf'
LINK bpftool
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__init_prog_names':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:466: undefined reference to `gelf_getsym'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:473: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__elf_finish':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:570: undefined reference to `elf_end'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__elf_init':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:600: undefined reference to `elf_memory'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:613: undefined reference to `elf_begin'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:623: undefined reference to `gelf_getehdr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object_search_section_size':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:714: undefined reference to `gelf_getshdr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:720: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:730: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:708: undefined reference to `elf_nextscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__variable_offset':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:784: undefined reference to `gelf_getsym'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:790: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__init_user_maps':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:937: undefined reference to `elf_getscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:939: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:957: undefined reference to `gelf_getsym'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:981: undefined reference to `gelf_getsym'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:990: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__init_user_btf_maps':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1359: undefined reference to `elf_getscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1361: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `section_have_execinstr':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1428: undefined reference to `elf_getscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1432: undefined reference to `gelf_getshdr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_object__elf_collect':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1608: undefined reference to `elf_getscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1608: undefined reference to `elf_rawdata'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1619: undefined reference to `gelf_getshdr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1625: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1632: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1613: undefined reference to `elf_nextscn'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `bpf_program__collect_reloc':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1928: undefined reference to `gelf_getrel'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1932: undefined reference to `gelf_getsym'
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:1941: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `__bpf_object__open':
/root/linux/bpf-next/tools/lib/bpf/libbpf.c:3934: undefined reference to `elf_version'
/root/linux/bpf-next/tools/lib/bpf/libbpf.a(libbpf-in.o): In function `btf__parse_elf':
/root/linux/bpf-next/tools/lib/bpf/btf.c:413: undefined reference to `elf_version'
/root/linux/bpf-next/tools/lib/bpf/btf.c:427: undefined reference to `elf_begin'
/root/linux/bpf-next/tools/lib/bpf/btf.c:432: undefined reference to `gelf_getehdr'
/root/linux/bpf-next/tools/lib/bpf/btf.c:440: undefined reference to `elf_getscn'
/root/linux/bpf-next/tools/lib/bpf/btf.c:440: undefined reference to `elf_rawdata'
/root/linux/bpf-next/tools/lib/bpf/btf.c:450: undefined reference to `gelf_getshdr'
/root/linux/bpf-next/tools/lib/bpf/btf.c:455: undefined reference to `elf_strptr'
/root/linux/bpf-next/tools/lib/bpf/btf.c:462: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/btf.c:470: undefined reference to `elf_getdata'
/root/linux/bpf-next/tools/lib/bpf/btf.c:445: undefined reference to `elf_nextscn'
/root/linux/bpf-next/tools/lib/bpf/btf.c:500: undefined reference to `elf_end'
collect2: error: ld returned 1 exit status
Makefile:158: recipe for target 'bpftool' failed
make: *** [bpftool] Error 1
make: Leaving directory '/root/linux/bpf-next/tools/bpf/bpftool'
----------------
Adding the V=1 flag shows the compilation is attempted with the
following command:
gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wundef -Wwrite-strings -Wformat -Wstrict-aliasing=3 -Wshadow -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ -I/root/linux/bpf-next/kernel/bpf/ -I/root/linux/bpf-next/tools/include -I/root/linux/bpf-next/tools/include/uapi -I/root/linux/bpf-next/tools/lib/bpf -I/root/linux/bpf-next/tools/perf -DBPFTOOL_VERSION='"5.4.0"' -DDISASM_FOUR_ARGS_SIGNATURE -DHAVE_LIBBFD_SUPPORT -o bpftool map_perf_ring.o perf.o net.o cgroup.o tracelog.o btf_dumper.o xlated_dumper.o json_writer.o prog.o map.o common.o btf.o cfg.o main.o netlink_dumper.o feature.o jit_disasm.o disasm.o -lelf -lz /root/linux/bpf-next/tools/lib/bpf/libbpf.a -lbfd -ldl -lopcodes
If I move -lelf and -lz after <path>/libbpf.a, then it builds fine.
Hope this helps,
Quentin
2019-11-27 11:24 UTC-0300 ~ Arnaldo Carvalho de Melo
<[email protected]>
> Em Wed, Nov 27, 2019 at 03:15:20PM +0100, Jiri Olsa escreveu:
>> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
>>> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
>>> On the plus side, all build attempts from
>>> tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
>>> my setup with dynamic linking from your branch.
>>
>> cool, had no idea there was such test ;-)
>
> Should be the the equivalent to 'make -C tools/perf build-test' :-)
>
> Perhaps we should make tools/testing/selftests/perf/ link to that?
It is already run as part of the bpf selftests, so probably no need.
Thanks,
Quentin
Em Wed, Nov 27, 2019 at 02:31:31PM +0000, Quentin Monnet escreveu:
> 2019-11-27 11:24 UTC-0300 ~ Arnaldo Carvalho de Melo
> <[email protected]>
> > Em Wed, Nov 27, 2019 at 03:15:20PM +0100, Jiri Olsa escreveu:
> >> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
> >>> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
> >>> On the plus side, all build attempts from
> >>> tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
> >>> my setup with dynamic linking from your branch.
> >>
> >> cool, had no idea there was such test ;-)
> >
> > Should be the the equivalent to 'make -C tools/perf build-test' :-)
> >
> > Perhaps we should make tools/testing/selftests/perf/ link to that?
>
> It is already run as part of the bpf selftests, so probably no need.
You mean 'make -C tools/perf build-test' is run from the bpf selftests?
> Thanks,
> Quentin
--
- Arnaldo
2019-11-27 12:48 UTC-0300 ~ Arnaldo Carvalho de Melo
<[email protected]>
> Em Wed, Nov 27, 2019 at 02:31:31PM +0000, Quentin Monnet escreveu:
>> 2019-11-27 11:24 UTC-0300 ~ Arnaldo Carvalho de Melo
>> <[email protected]>
>>> Em Wed, Nov 27, 2019 at 03:15:20PM +0100, Jiri Olsa escreveu:
>>>> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
>>>>> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
>>>>> On the plus side, all build attempts from
>>>>> tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
>>>>> my setup with dynamic linking from your branch.
>>>>
>>>> cool, had no idea there was such test ;-)
>>>
>>> Should be the the equivalent to 'make -C tools/perf build-test' :-)
>>>
>>> Perhaps we should make tools/testing/selftests/perf/ link to that?
>>
>> It is already run as part of the bpf selftests, so probably no need.
>
> You mean 'make -C tools/perf build-test' is run from the bpf selftests?
Ah, no, sorry for the confusion. I meant that test_bpftool_build.sh is
run from the bpf selftests.
I am not familiar with perf build-test, but maybe that's something worth
adding to perf selftests indeed.
Quentin
Em Wed, Nov 27, 2019 at 03:52:06PM +0000, Quentin Monnet escreveu:
> 2019-11-27 12:48 UTC-0300 ~ Arnaldo Carvalho de Melo <[email protected]>
> > Em Wed, Nov 27, 2019 at 02:31:31PM +0000, Quentin Monnet escreveu:
> >> 2019-11-27 11:24 UTC-0300 ~ Arnaldo Carvalho de Melo <[email protected]>
> >>> Em Wed, Nov 27, 2019 at 03:15:20PM +0100, Jiri Olsa escreveu:
> >>>> On Wed, Nov 27, 2019 at 01:38:55PM +0000, Quentin Monnet wrote:
> >>>>> 2019-11-27 10:48 UTC+0100 ~ Jiri Olsa <[email protected]>
> >>>>> On the plus side, all build attempts from
> >>>>> tools/testing/selftests/bpf/test_bpftool_build.sh pass successfully on
> >>>>> my setup with dynamic linking from your branch.
> >>>> cool, had no idea there was such test ;-)
> >>> Should be the the equivalent to 'make -C tools/perf build-test' :-)
> >>> Perhaps we should make tools/testing/selftests/perf/ link to that?
> >> It is already run as part of the bpf selftests, so probably no need.
> > You mean 'make -C tools/perf build-test' is run from the bpf selftests?
> Ah, no, sorry for the confusion. I meant that test_bpftool_build.sh is
> run from the bpf selftests.
> I am not familiar with perf build-test, but maybe that's something worth
> adding to perf selftests indeed.
Yeah, I think is worth considering plugging perf's build-test to
selftests, if only to expose it to the people that are used with
selftests and may start testing perf builds more regularly.
- Arnaldo
On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
>
> hi,
> adding support to link bpftool with libbpf dynamically,
> and config change for perf.
>
> It's now possible to use:
> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
>
> which will detect libbpf devel package with needed version,
> and if found, link it with bpftool.
>
> It's possible to use arbitrary installed libbpf:
> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
>
> I based this change on top of Arnaldo's perf/core, because
> it contains libbpf feature detection code as dependency.
> It's now also synced with latest bpf-next, so Toke's change
> applies correctly.
I don't like it.
Especially Toke's patch to expose netlink as public and stable libbpf api.
bpftools needs to stay tightly coupled with libbpf (and statically
linked for that reason).
Otherwise libbpf will grow a ton of public api that would have to be stable
and will quickly become a burden.
On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
>
> diff --git a/tools/build/feature/test-libbpf.c b/tools/build/feature/test-libbpf.c
> index a508756cf4cc..93566d105a64 100644
> --- a/tools/build/feature/test-libbpf.c
> +++ b/tools/build/feature/test-libbpf.c
> @@ -3,5 +3,14 @@
>
> int main(void)
> {
> +#ifdef BPFTOOL
> + /*
> + * libbpf_netlink_open (LIBBPF_0.0.6) is the latest
> + * we need for bpftool at the moment
> + */
> + libbpf_netlink_open(NULL);
> + return 0;
> +#else
> return bpf_object__open("test") ? 0 : -1;
> +#endif
Such hack should be a clear sign that it's not appropriate for libbpf to
be public netlink api library. Few functions that it already has are for
libbpf and bpftool internal usage only.
Em Wed, Nov 27, 2019 at 08:37:59AM -0800, Alexei Starovoitov escreveu:
> On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
> > adding support to link bpftool with libbpf dynamically,
> > and config change for perf.
> > It's now possible to use:
> > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > which will detect libbpf devel package with needed version,
> > and if found, link it with bpftool.
> > It's possible to use arbitrary installed libbpf:
> > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
> > I based this change on top of Arnaldo's perf/core, because
> > it contains libbpf feature detection code as dependency.
> > It's now also synced with latest bpf-next, so Toke's change
> > applies correctly.
> I don't like it.
> Especially Toke's patch to expose netlink as public and stable libbpf api.
> bpftools needs to stay tightly coupled with libbpf (and statically
> linked for that reason).
> Otherwise libbpf will grow a ton of public api that would have to be stable
> and will quickly become a burden.
I can relate to that, tools/lib/perf/ is only now getting put in place
because of these fears, hopefully the evsel/evlist/cpumap/etc
abstractions there have been in use for a long time and will be not be a
burden... :-)
- Arnaldo
On Wed, Nov 27, 2019 at 8:38 AM Alexei Starovoitov
<[email protected]> wrote:
>
> On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
> >
> > hi,
> > adding support to link bpftool with libbpf dynamically,
> > and config change for perf.
> >
> > It's now possible to use:
> > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> >
> > which will detect libbpf devel package with needed version,
> > and if found, link it with bpftool.
> >
> > It's possible to use arbitrary installed libbpf:
> > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
> >
> > I based this change on top of Arnaldo's perf/core, because
> > it contains libbpf feature detection code as dependency.
> > It's now also synced with latest bpf-next, so Toke's change
> > applies correctly.
>
> I don't like it.
> Especially Toke's patch to expose netlink as public and stable libbpf api.
> bpftools needs to stay tightly coupled with libbpf (and statically
> linked for that reason).
> Otherwise libbpf will grow a ton of public api that would have to be stable
> and will quickly become a burden.
I second that. I'm currently working on adding few more APIs that I'd
like to keep unstable for a while, until we have enough real-world
usage (and feedback) accumulated, before we stabilize them. With
LIBBPF_API and a promise of stable API, we are going to over-stress
and over-design APIs, potentially making them either too generic and
bloated, or too limited (and thus become deprecated almost at
inception time). I'd like to take that pressure off for a super-new
and in flux APIs and not hamper the progress.
I'm thinking of splitting off those non-stable, sort-of-internal APIs
into separate libbpf-experimental.h (or whatever name makes sense),
and let those be used only by tools like bpftool, which are only ever
statically link against libbpf and are ok with occasional changes to
those APIs (which we'll obviously fix in bpftool as well). Pahole
seems like another candidate that fits this bill and we might expose
some stuff early on to it, if it provides tangible benefits (e.g., BTF
dedup speeds ups, etc).
Then as APIs mature, we might decide to move them into libbpf.h with
LIBBPF_API slapped onto them. Any objections?
On 11/27/19 9:24 PM, Andrii Nakryiko wrote:
> On Wed, Nov 27, 2019 at 8:38 AM Alexei Starovoitov
> <[email protected]> wrote:
>> On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
>>>
>>> hi,
>>> adding support to link bpftool with libbpf dynamically,
>>> and config change for perf.
>>>
>>> It's now possible to use:
>>> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
>>>
>>> which will detect libbpf devel package with needed version,
>>> and if found, link it with bpftool.
>>>
>>> It's possible to use arbitrary installed libbpf:
>>> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
>>>
>>> I based this change on top of Arnaldo's perf/core, because
>>> it contains libbpf feature detection code as dependency.
>>> It's now also synced with latest bpf-next, so Toke's change
>>> applies correctly.
>>
>> I don't like it.
>> Especially Toke's patch to expose netlink as public and stable libbpf api.
>> bpftools needs to stay tightly coupled with libbpf (and statically
>> linked for that reason).
>> Otherwise libbpf will grow a ton of public api that would have to be stable
>> and will quickly become a burden.
+1, and would also be out of scope from a BPF library point of view.
> I second that. I'm currently working on adding few more APIs that I'd
> like to keep unstable for a while, until we have enough real-world
> usage (and feedback) accumulated, before we stabilize them. With
> LIBBPF_API and a promise of stable API, we are going to over-stress
> and over-design APIs, potentially making them either too generic and
> bloated, or too limited (and thus become deprecated almost at
> inception time). I'd like to take that pressure off for a super-new
> and in flux APIs and not hamper the progress.
>
> I'm thinking of splitting off those non-stable, sort-of-internal APIs
> into separate libbpf-experimental.h (or whatever name makes sense),
> and let those be used only by tools like bpftool, which are only ever
> statically link against libbpf and are ok with occasional changes to
> those APIs (which we'll obviously fix in bpftool as well). Pahole
> seems like another candidate that fits this bill and we might expose
> some stuff early on to it, if it provides tangible benefits (e.g., BTF
> dedup speeds ups, etc).
>
> Then as APIs mature, we might decide to move them into libbpf.h with
> LIBBPF_API slapped onto them. Any objections?
I don't think adding yet another libbpf_experimental.h makes sense, it feels
too much of an invitation to add all sort of random stuff in there. We already
do have libbpf.h and libbpf_internal.h, so everything that does not relate to
the /stable and public/ API should be moved from libbpf.h into libbpf_internal.h
such as the netlink helpers, as one example, and bpftool can use these since
in-tree changes also cover the latter just fine. So overall, same page, just
reuse/improve libbpf_internal.h instead of a new libbpf_experimental.h.
Thanks,
Daniel
On Wed, Nov 27, 2019 at 10:22:31PM +0100, Daniel Borkmann wrote:
> On 11/27/19 9:24 PM, Andrii Nakryiko wrote:
> > On Wed, Nov 27, 2019 at 8:38 AM Alexei Starovoitov
> > <[email protected]> wrote:
> > > On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
> > > >
> > > > hi,
> > > > adding support to link bpftool with libbpf dynamically,
> > > > and config change for perf.
> > > >
> > > > It's now possible to use:
> > > > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > > >
> > > > which will detect libbpf devel package with needed version,
> > > > and if found, link it with bpftool.
> > > >
> > > > It's possible to use arbitrary installed libbpf:
> > > > $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
> > > >
> > > > I based this change on top of Arnaldo's perf/core, because
> > > > it contains libbpf feature detection code as dependency.
> > > > It's now also synced with latest bpf-next, so Toke's change
> > > > applies correctly.
> > >
> > > I don't like it.
> > > Especially Toke's patch to expose netlink as public and stable libbpf api.
> > > bpftools needs to stay tightly coupled with libbpf (and statically
> > > linked for that reason).
> > > Otherwise libbpf will grow a ton of public api that would have to be stable
> > > and will quickly become a burden.
>
> +1, and would also be out of scope from a BPF library point of view.
ok, static it is.. ;-) thanks for the feedback,
jirka
>
> > I second that. I'm currently working on adding few more APIs that I'd
> > like to keep unstable for a while, until we have enough real-world
> > usage (and feedback) accumulated, before we stabilize them. With
> > LIBBPF_API and a promise of stable API, we are going to over-stress
> > and over-design APIs, potentially making them either too generic and
> > bloated, or too limited (and thus become deprecated almost at
> > inception time). I'd like to take that pressure off for a super-new
> > and in flux APIs and not hamper the progress.
> >
> > I'm thinking of splitting off those non-stable, sort-of-internal APIs
> > into separate libbpf-experimental.h (or whatever name makes sense),
> > and let those be used only by tools like bpftool, which are only ever
> > statically link against libbpf and are ok with occasional changes to
> > those APIs (which we'll obviously fix in bpftool as well). Pahole
> > seems like another candidate that fits this bill and we might expose
> > some stuff early on to it, if it provides tangible benefits (e.g., BTF
> > dedup speeds ups, etc).
> >
> > Then as APIs mature, we might decide to move them into libbpf.h with
> > LIBBPF_API slapped onto them. Any objections?
>
> I don't think adding yet another libbpf_experimental.h makes sense, it feels
> too much of an invitation to add all sort of random stuff in there. We already
> do have libbpf.h and libbpf_internal.h, so everything that does not relate to
> the /stable and public/ API should be moved from libbpf.h into libbpf_internal.h
> such as the netlink helpers, as one example, and bpftool can use these since
> in-tree changes also cover the latter just fine. So overall, same page, just
> reuse/improve libbpf_internal.h instead of a new libbpf_experimental.h.
>
> Thanks,
> Daniel
>
Alexei Starovoitov <[email protected]> writes:
> On Wed, Nov 27, 2019 at 1:48 AM Jiri Olsa <[email protected]> wrote:
>>
>> hi,
>> adding support to link bpftool with libbpf dynamically,
>> and config change for perf.
>>
>> It's now possible to use:
>> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
>>
>> which will detect libbpf devel package with needed version,
>> and if found, link it with bpftool.
>>
>> It's possible to use arbitrary installed libbpf:
>> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
>>
>> I based this change on top of Arnaldo's perf/core, because
>> it contains libbpf feature detection code as dependency.
>> It's now also synced with latest bpf-next, so Toke's change
>> applies correctly.
>
> I don't like it.
> Especially Toke's patch to expose netlink as public and stable libbpf
> api.
Figured you might say that :)
> bpftools needs to stay tightly coupled with libbpf (and statically
> linked for that reason).
> Otherwise libbpf will grow a ton of public api that would have to be stable
> and will quickly become a burden.
I can see why you don't want to expose the "internal" functions as
LIBBPF_API. Doesn't *have* to mean we can't link bpftool dynamically
against the .so version of libbpf, though; will see if I can figure out
a clean way to do that...
-Toke
From: Jiri Olsa <[email protected]>
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-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 <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
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 | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 39bc6f0f4f0b..397051ed9e41 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)))
@@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
LDFLAGS += $(EXTRA_LDFLAGS)
endif
-LIBS = $(LIBBPF) -lelf -lz
+LIBS = -lelf -lz
INSTALL ?= install
RM ?= rm -f
@@ -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
+ 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,20 @@ ifeq ($(feature-reallocarray), 0)
CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
endif
+ifdef LIBBPF_DYNAMIC
+ ifeq ($(feature-libbpf), 1)
+ # bpftool uses non-exported functions from libbpf, so pass both dynamic and
+ # static versions and let the linker figure it out
+ LIBS := -lbpf $(LIBBPF) $(LIBS)
+ CFLAGS += $(LIBBPF_CFLAGS)
+ LDFLAGS += $(LIBBPF_LDFLAGS)
+ else
+ dummy := $(error Error: No libbpf devel library found, please install-devel or libbpf-dev.)
+ endif
+else
+ LIBS := $(LIBBPF) $(LIBS)
+endif
+
include $(wildcard $(OUTPUT)*.d)
all: $(OUTPUT)bpftool
--
2.24.0
2019-11-28 15:53 UTC+0100 ~ Toke Høiland-Jørgensen <[email protected]>
> From: Jiri Olsa <[email protected]>
>
> 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-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 <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> 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 | 38 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 39bc6f0f4f0b..397051ed9e41 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)))
> @@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> LDFLAGS += $(EXTRA_LDFLAGS)
> endif
>
> -LIBS = $(LIBBPF) -lelf -lz
> +LIBS = -lelf -lz
Hi Toke,
You don't need to remove $(LIBBPF) here, because you add it in both
cases below (whether $(LIBBPF_DYNAMIC) is defined or not).
>
> INSTALL ?= install
> RM ?= rm -f
> @@ -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
> + 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,20 @@ ifeq ($(feature-reallocarray), 0)
> CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
> endif
>
> +ifdef LIBBPF_DYNAMIC
> + ifeq ($(feature-libbpf), 1)
> + # bpftool uses non-exported functions from libbpf, so pass both dynamic and
> + # static versions and let the linker figure it out
> + LIBS := -lbpf $(LIBBPF) $(LIBS)
[$(LIBBPF) added to $(LIBS) here...]
> + CFLAGS += $(LIBBPF_CFLAGS)
> + LDFLAGS += $(LIBBPF_LDFLAGS)
> + else
> + dummy := $(error Error: No libbpf devel library found, please install-devel or libbpf-dev.)
“install-devel” :)
> + endif
> +else
> + LIBS := $(LIBBPF) $(LIBS)
[... and here]
> +endif
> +
> include $(wildcard $(OUTPUT)*.d)
>
> all: $(OUTPUT)bpftool
>
Thanks,
Quentin
Quentin Monnet <[email protected]> writes:
> 2019-11-28 15:53 UTC+0100 ~ Toke Høiland-Jørgensen <[email protected]>
>> From: Jiri Olsa <[email protected]>
>>
>> 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-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 <[email protected]>
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> ---
>> 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 | 38 +++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
>> index 39bc6f0f4f0b..397051ed9e41 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)))
>> @@ -55,7 +64,7 @@ ifneq ($(EXTRA_LDFLAGS),)
>> LDFLAGS += $(EXTRA_LDFLAGS)
>> endif
>>
>> -LIBS = $(LIBBPF) -lelf -lz
>> +LIBS = -lelf -lz
>
> Hi Toke,
>
> You don't need to remove $(LIBBPF) here, because you add it in both
> cases below (whether $(LIBBPF_DYNAMIC) is defined or not).
Oh, right, good point; will fix :)
From: Jiri Olsa <[email protected]>
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 <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
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
+ 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)
+ 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
--
2.24.0
2019-11-28 17:07 UTC+0100 ~ Toke Høiland-Jørgensen <[email protected]>
> From: Jiri Olsa <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> 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.
Thanks for the changes!
Reviewed-by: Quentin Monnet <[email protected]>
On Thu, Nov 28, 2019 at 05:07:12PM +0100, Toke H?iland-J?rgensen wrote:
SNIP
> 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
> + 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)
nice, so linker will pick up the missing symbols and we
don't need to check on particular libbpf version then
thanks,
jirka
> + 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
> --
> 2.24.0
>
Jiri Olsa <[email protected]> writes:
> On Thu, Nov 28, 2019 at 05:07:12PM +0100, Toke Høiland-Jørgensen wrote:
>
> SNIP
>
>> 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
>> + 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)
>
> nice, so linker will pick up the missing symbols and we
> don't need to check on particular libbpf version then
Yup, exactly. I verified with objdump that the end result is a
dynamically linked bpftool with LIBBPF_DYNAMIC is set, and a statically
linked one if it isn't; so the linker seems to be smart enough to just
figure out how to do the right thing :)
-Toke
On 11/28/19 5:07 PM, Toke Høiland-Jørgensen wrote:
> From: Jiri Olsa <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> ---
> 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
Daniel Borkmann <[email protected]> writes:
> On 11/28/19 5:07 PM, Toke Høiland-Jørgensen wrote:
>> From: Jiri Olsa <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> ---
>> 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.
I see what you mean; however, LIBBPF_DIR is meant to be specifically an
override for the dynamic library, not just the path to libbpf.
Would it be less confusing to overload the LIBBPF_DYNAMIC variable
instead? I.e.,
make LIBBPF_DYNAMIC=1
would dynamically link against the libbpf installed in the system, but
make LIBBPF_DYNAMIC=/opt/libbpf
would override that and link against whatever is in /opt/libbpf instead?
WDYT?
> 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.
When adding LIBBPF_DYNAMIC in a packaging script, we *want* the build to
fail if it doesn't work, instead of just silently falling back to a
statically linked version. Also, for something in the kernel tree like
bpftool, I think it makes more sense to default to the in-tree version
and make dynamic linking explicitly opt-in.
>> + 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.
Yeah, I do think it's a little odd that bpftool is using "private" parts
of libbpf, but since we can solve it like this I think that is OK.
-Toke
On 11/29/19 3:00 PM, Toke Høiland-Jørgensen wrote:
> Daniel Borkmann <[email protected]> writes:
>> On 11/28/19 5:07 PM, Toke Høiland-Jørgensen wrote:
>>> From: Jiri Olsa <[email protected]>
[...]
>>> 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.
>
> I see what you mean; however, LIBBPF_DIR is meant to be specifically an
> override for the dynamic library, not just the path to libbpf.
>
> Would it be less confusing to overload the LIBBPF_DYNAMIC variable
> instead? I.e.,
>
> make LIBBPF_DYNAMIC=1
>
> would dynamically link against the libbpf installed in the system, but
>
> make LIBBPF_DYNAMIC=/opt/libbpf
>
> would override that and link against whatever is in /opt/libbpf instead?
> WDYT?
Hm, given perf tool has similar LIB*_DIR vars in place for its libs, it probably
makes sense to stick with this convention as well then. Perhaps better in this
case to just rename s/BPF_DIR/BPF_SRC_DIR/, s/LIBBPF_OUTPUT/LIBBPF_BUILD_OUTPUT/,
and s/LIBBPF_PATH/LIBBPF_BUILD_PATH/ to make their purpose more clear.
One thing that would be good to do as well for this patch is to:
i) Document both LIBBPF_DYNAMIC and LIBBPF_DIR in the Makefile comment you
added at the top along with a simple usage example.
ii) Extend tools/testing/selftests/bpf/test_bpftool_build.sh with a dynamic
linking test case, e.g. building libbpf into a temp dir and pointing
LIBBPF_DIR to it for bpftool LIBBPF_DYNAMIC=1 build.
Thanks,
Daniel
On Sat, Nov 30, 2019 at 12:56:49AM +0100, Daniel Borkmann wrote:
> On 11/29/19 3:00 PM, Toke H?iland-J?rgensen wrote:
> > Daniel Borkmann <[email protected]> writes:
> > > On 11/28/19 5:07 PM, Toke H?iland-J?rgensen wrote:
> > > > From: Jiri Olsa <[email protected]>
> [...]
> > > > 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.
> >
> > I see what you mean; however, LIBBPF_DIR is meant to be specifically an
> > override for the dynamic library, not just the path to libbpf.
> >
> > Would it be less confusing to overload the LIBBPF_DYNAMIC variable
> > instead? I.e.,
> >
> > make LIBBPF_DYNAMIC=1
> >
> > would dynamically link against the libbpf installed in the system, but
> >
> > make LIBBPF_DYNAMIC=/opt/libbpf
> >
> > would override that and link against whatever is in /opt/libbpf instead?
> > WDYT?
>
> Hm, given perf tool has similar LIB*_DIR vars in place for its libs, it probably
> makes sense to stick with this convention as well then. Perhaps better in this
> case to just rename s/BPF_DIR/BPF_SRC_DIR/, s/LIBBPF_OUTPUT/LIBBPF_BUILD_OUTPUT/,
> and s/LIBBPF_PATH/LIBBPF_BUILD_PATH/ to make their purpose more clear.
ok, will have separate patch for this
>
> One thing that would be good to do as well for this patch is to:
>
> i) Document both LIBBPF_DYNAMIC and LIBBPF_DIR in the Makefile comment you
> added at the top along with a simple usage example.
ok
>
> ii) Extend tools/testing/selftests/bpf/test_bpftool_build.sh with a dynamic
> linking test case, e.g. building libbpf into a temp dir and pointing
> LIBBPF_DIR to it for bpftool LIBBPF_DYNAMIC=1 build.
ok, will send new version soon
thanks,
jirka
>
> Thanks,
> Daniel
>
Andrii Nakryiko <[email protected]> writes:
> On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
>>
>> hi,
>> adding support to link bpftool with libbpf dynamically,
>> and config change for perf.
>>
>> It's now possible to use:
>> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
>
> I wonder what's the motivation behind these changes, though? Why is
> linking bpftool dynamically with libbpf is necessary and important?
> They are both developed tightly within kernel repo, so I fail to see
> what are the huge advantages one can get from linking them
> dynamically.
Well, all the regular reasons for using dynamic linking (memory usage,
binary size, etc). But in particular, the ability to update the libbpf
package if there's a serious bug, and have that be picked up by all
utilities making use of it. No reason why bpftool should be special in
that respect.
-Toke
On Mon, Dec 02, 2019 at 10:42:53AM -0800, Andrii Nakryiko wrote:
> On Mon, Dec 2, 2019 at 10:09 AM Toke H?iland-J?rgensen <[email protected]> wrote:
> >
> > Andrii Nakryiko <[email protected]> writes:
> >
> > > On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> > >>
> > >> hi,
> > >> adding support to link bpftool with libbpf dynamically,
> > >> and config change for perf.
> > >>
> > >> It's now possible to use:
> > >> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > >
> > > I wonder what's the motivation behind these changes, though? Why is
> > > linking bpftool dynamically with libbpf is necessary and important?
> > > They are both developed tightly within kernel repo, so I fail to see
> > > what are the huge advantages one can get from linking them
> > > dynamically.
> >
> > Well, all the regular reasons for using dynamic linking (memory usage,
> > binary size, etc).
>
> bpftool is 327KB with statically linked libbpf. Hardly a huge problem
> for either binary size or memory usage. CPU instruction cache usage is
> also hardly a concern for bpftool specifically.
>
> > But in particular, the ability to update the libbpf
> > package if there's a serious bug, and have that be picked up by all
> > utilities making use of it.
>
> I agree, and that works only for utilities linking with libbpf
> dynamically. For tools that build statically, you'd have to update
> tools anyways. And if you can update libbpf, you can as well update
> bpftool at the same time, so I don't think linking bpftool statically
> with libbpf causes any new problems.
it makes difference for us if we need to respin just one library
instead of several applications (bpftool and perf at the moment),
because of the bug in the library
with the Toke's approach we compile some bits of libbpf statically into
bpftool, but there's still the official API in the dynamic libbpf that
we care about and that could carry on the fix without bpftool respin
> > No reason why bpftool should be special in that respect.
>
> But I think bpftool is special and we actually want it to be special
> and tightly coupled to libbpf with sometimes very intimate knowledge
> of libbpf and access to "hidden" APIs. That allows us to experiment
> with new stuff that requires use of bpftool (e.g., code generation for
> BPF programs), without having to expose and seal public APIs. And I
> don't think it's a problem from the point of code maintenance, because
> both live in the same repository and are updated "atomically" when new
> features are added or changed.
I thought we solved this by Toke's approach, so there' no need
to expose any new/experimental API .. also you guys will probably
continue using static linking I guess
jirka
>
> Beyond superficial binary size worries, I don't see any good reason
> why we should add more complexity and variables to libbpf and bpftool
> build processes just to have a "nice to have" option of linking
> bpftool dynamically with libbpf.
On Mon, Dec 2, 2019 at 11:21 AM Jiri Olsa <[email protected]> wrote:
>
> On Mon, Dec 02, 2019 at 10:42:53AM -0800, Andrii Nakryiko wrote:
> > On Mon, Dec 2, 2019 at 10:09 AM Toke Høiland-Jørgensen <[email protected]> wrote:
> > >
> > > Andrii Nakryiko <[email protected]> writes:
> > >
> > > > On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> > > >>
> > > >> hi,
> > > >> adding support to link bpftool with libbpf dynamically,
> > > >> and config change for perf.
> > > >>
> > > >> It's now possible to use:
> > > >> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > > >
> > > > I wonder what's the motivation behind these changes, though? Why is
> > > > linking bpftool dynamically with libbpf is necessary and important?
> > > > They are both developed tightly within kernel repo, so I fail to see
> > > > what are the huge advantages one can get from linking them
> > > > dynamically.
> > >
> > > Well, all the regular reasons for using dynamic linking (memory usage,
> > > binary size, etc).
> >
> > bpftool is 327KB with statically linked libbpf. Hardly a huge problem
> > for either binary size or memory usage. CPU instruction cache usage is
> > also hardly a concern for bpftool specifically.
> >
> > > But in particular, the ability to update the libbpf
> > > package if there's a serious bug, and have that be picked up by all
> > > utilities making use of it.
> >
> > I agree, and that works only for utilities linking with libbpf
> > dynamically. For tools that build statically, you'd have to update
> > tools anyways. And if you can update libbpf, you can as well update
> > bpftool at the same time, so I don't think linking bpftool statically
> > with libbpf causes any new problems.
>
> it makes difference for us if we need to respin just one library
> instead of several applications (bpftool and perf at the moment),
> because of the bug in the library
>
> with the Toke's approach we compile some bits of libbpf statically into
> bpftool, but there's still the official API in the dynamic libbpf that
> we care about and that could carry on the fix without bpftool respin
See my replies on v4 of your patchset. I have doubts this actually
works as we hope it works.
I also don't see how that is going to work in general. Imagine
something like this:
static int some_state = 123;
LIBBPF_API void set_state(int x) { some_state = x; }
int get_state() { return some_state; }
If bpftool does:
set_state(42);
printf("%d\n", get_state());
How is this supposed to work with set_state() coming from libbpf.so,
while get_state() being statically linked? Who "owns" memory of `int
some_state` -- bpftool or libbpf.so? Can they magically share it? Or
rather maybe some_state will be actually two different variables in
two different memory regions? And set_state() would be setting one of
them, while get_state() would be reading another one?
It would be good to test this out. Do you mind checking?
>
> > > No reason why bpftool should be special in that respect.
> >
> > But I think bpftool is special and we actually want it to be special
> > and tightly coupled to libbpf with sometimes very intimate knowledge
> > of libbpf and access to "hidden" APIs. That allows us to experiment
> > with new stuff that requires use of bpftool (e.g., code generation for
> > BPF programs), without having to expose and seal public APIs. And I
> > don't think it's a problem from the point of code maintenance, because
> > both live in the same repository and are updated "atomically" when new
> > features are added or changed.
>
> I thought we solved this by Toke's approach, so there' no need
> to expose any new/experimental API .. also you guys will probably
> continue using static linking I guess
>
> jirka
>
> >
> > Beyond superficial binary size worries, I don't see any good reason
> > why we should add more complexity and variables to libbpf and bpftool
> > build processes just to have a "nice to have" option of linking
> > bpftool dynamically with libbpf.
>
On Mon, Dec 02, 2019 at 11:54:27AM -0800, Andrii Nakryiko wrote:
> On Mon, Dec 2, 2019 at 11:21 AM Jiri Olsa <[email protected]> wrote:
> >
> > On Mon, Dec 02, 2019 at 10:42:53AM -0800, Andrii Nakryiko wrote:
> > > On Mon, Dec 2, 2019 at 10:09 AM Toke H?iland-J?rgensen <[email protected]> wrote:
> > > >
> > > > Andrii Nakryiko <[email protected]> writes:
> > > >
> > > > > On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> > > > >>
> > > > >> hi,
> > > > >> adding support to link bpftool with libbpf dynamically,
> > > > >> and config change for perf.
> > > > >>
> > > > >> It's now possible to use:
> > > > >> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > > > >
> > > > > I wonder what's the motivation behind these changes, though? Why is
> > > > > linking bpftool dynamically with libbpf is necessary and important?
> > > > > They are both developed tightly within kernel repo, so I fail to see
> > > > > what are the huge advantages one can get from linking them
> > > > > dynamically.
> > > >
> > > > Well, all the regular reasons for using dynamic linking (memory usage,
> > > > binary size, etc).
> > >
> > > bpftool is 327KB with statically linked libbpf. Hardly a huge problem
> > > for either binary size or memory usage. CPU instruction cache usage is
> > > also hardly a concern for bpftool specifically.
> > >
> > > > But in particular, the ability to update the libbpf
> > > > package if there's a serious bug, and have that be picked up by all
> > > > utilities making use of it.
> > >
> > > I agree, and that works only for utilities linking with libbpf
> > > dynamically. For tools that build statically, you'd have to update
> > > tools anyways. And if you can update libbpf, you can as well update
> > > bpftool at the same time, so I don't think linking bpftool statically
> > > with libbpf causes any new problems.
> >
> > it makes difference for us if we need to respin just one library
> > instead of several applications (bpftool and perf at the moment),
> > because of the bug in the library
> >
> > with the Toke's approach we compile some bits of libbpf statically into
> > bpftool, but there's still the official API in the dynamic libbpf that
> > we care about and that could carry on the fix without bpftool respin
>
> See my replies on v4 of your patchset. I have doubts this actually
> works as we hope it works.
>
> I also don't see how that is going to work in general. Imagine
> something like this:
>
> static int some_state = 123;
>
> LIBBPF_API void set_state(int x) { some_state = x; }
>
> int get_state() { return some_state; }
>
> If bpftool does:
>
> set_state(42);
> printf("%d\n", get_state());
>
>
> How is this supposed to work with set_state() coming from libbpf.so,
> while get_state() being statically linked? Who "owns" memory of `int
> some_state` -- bpftool or libbpf.so? Can they magically share it? Or
> rather maybe some_state will be actually two different variables in
> two different memory regions? And set_state() would be setting one of
> them, while get_state() would be reading another one?
>
> It would be good to test this out. Do you mind checking?
I think you're right.. sry, I should have checked on this more,
there are no relocations for libbpf.so, so it's all statically
linked and the libbpf is just in 'needed' libs record.. ugh :-\
jirka
On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
>
> hi,
> adding support to link bpftool with libbpf dynamically,
> and config change for perf.
>
> It's now possible to use:
> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
I wonder what's the motivation behind these changes, though? Why is
linking bpftool dynamically with libbpf is necessary and important?
They are both developed tightly within kernel repo, so I fail to see
what are the huge advantages one can get from linking them
dynamically.
>
> which will detect libbpf devel package with needed version,
> and if found, link it with bpftool.
>
> It's possible to use arbitrary installed libbpf:
> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/libbpf/
>
> I based this change on top of Arnaldo's perf/core, because
> it contains libbpf feature detection code as dependency.
> It's now also synced with latest bpf-next, so Toke's change
> applies correctly.
>
> Also available in:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> libbpf/dyn
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (2):
> perf tools: Allow to specify libbpf install directory
> bpftool: Allow to link libbpf dynamically
>
> Toke Høiland-Jørgensen (1):
> libbpf: Export netlink functions used by bpftool
>
> tools/bpf/bpftool/Makefile | 40 +++++++++++++++++++++++++++++++++++++++-
> tools/build/feature/test-libbpf.c | 9 +++++++++
> tools/lib/bpf/libbpf.h | 22 +++++++++++++---------
> tools/lib/bpf/libbpf.map | 7 +++++++
> tools/lib/bpf/nlattr.h | 15 ++++++++++-----
> tools/perf/Makefile.config | 27 ++++++++++++++++++++-------
> 6 files changed, 98 insertions(+), 22 deletions(-)
>
On Mon, Dec 2, 2019 at 10:09 AM Toke Høiland-Jørgensen <[email protected]> wrote:
>
> Andrii Nakryiko <[email protected]> writes:
>
> > On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> >>
> >> hi,
> >> adding support to link bpftool with libbpf dynamically,
> >> and config change for perf.
> >>
> >> It's now possible to use:
> >> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> >
> > I wonder what's the motivation behind these changes, though? Why is
> > linking bpftool dynamically with libbpf is necessary and important?
> > They are both developed tightly within kernel repo, so I fail to see
> > what are the huge advantages one can get from linking them
> > dynamically.
>
> Well, all the regular reasons for using dynamic linking (memory usage,
> binary size, etc).
bpftool is 327KB with statically linked libbpf. Hardly a huge problem
for either binary size or memory usage. CPU instruction cache usage is
also hardly a concern for bpftool specifically.
> But in particular, the ability to update the libbpf
> package if there's a serious bug, and have that be picked up by all
> utilities making use of it.
I agree, and that works only for utilities linking with libbpf
dynamically. For tools that build statically, you'd have to update
tools anyways. And if you can update libbpf, you can as well update
bpftool at the same time, so I don't think linking bpftool statically
with libbpf causes any new problems.
> No reason why bpftool should be special in that respect.
But I think bpftool is special and we actually want it to be special
and tightly coupled to libbpf with sometimes very intimate knowledge
of libbpf and access to "hidden" APIs. That allows us to experiment
with new stuff that requires use of bpftool (e.g., code generation for
BPF programs), without having to expose and seal public APIs. And I
don't think it's a problem from the point of code maintenance, because
both live in the same repository and are updated "atomically" when new
features are added or changed.
Beyond superficial binary size worries, I don't see any good reason
why we should add more complexity and variables to libbpf and bpftool
build processes just to have a "nice to have" option of linking
bpftool dynamically with libbpf.
>
> -Toke
>
Em Mon, Dec 02, 2019 at 10:42:53AM -0800, Andrii Nakryiko escreveu:
> On Mon, Dec 2, 2019 at 10:09 AM Toke H?iland-J?rgensen <[email protected]> wrote:
> > Andrii Nakryiko <[email protected]> writes:
> > > On Wed, Nov 27, 2019 at 1:49 AM Jiri Olsa <[email protected]> wrote:
> > >> adding support to link bpftool with libbpf dynamically,
> > >> and config change for perf.
> > >> It's now possible to use:
> > >> $ make -C tools/bpf/bpftool/ LIBBPF_DYNAMIC=1
> > > I wonder what's the motivation behind these changes, though? Why is
> > > linking bpftool dynamically with libbpf is necessary and important?
> > > They are both developed tightly within kernel repo, so I fail to see
> > > what are the huge advantages one can get from linking them
> > > dynamically.
> > Well, all the regular reasons for using dynamic linking (memory usage,
> > binary size, etc).
> bpftool is 327KB with statically linked libbpf. Hardly a huge problem
> for either binary size or memory usage. CPU instruction cache usage is
> also hardly a concern for bpftool specifically.
> > But in particular, the ability to update the libbpf
> > package if there's a serious bug, and have that be picked up by all
> > utilities making use of it.
> I agree, and that works only for utilities linking with libbpf
> dynamically. For tools that build statically, you'd have to update
> tools anyways. And if you can update libbpf, you can as well update
> bpftool at the same time, so I don't think linking bpftool statically
> with libbpf causes any new problems.
> > No reason why bpftool should be special in that respect.
> But I think bpftool is special and we actually want it to be special
> and tightly coupled to libbpf with sometimes very intimate knowledge
> of libbpf and access to "hidden" APIs. That allows us to experiment
> with new stuff that requires use of bpftool (e.g., code generation for
> BPF programs), without having to expose and seal public APIs. And I
> don't think it's a problem from the point of code maintenance, because
> both live in the same repository and are updated "atomically" when new
> features are added or changed.
> Beyond superficial binary size worries, I don't see any good reason
> why we should add more complexity and variables to libbpf and bpftool
> build processes just to have a "nice to have" option of linking
> bpftool dynamically with libbpf.
s/bpftool/perf/g
s/libbpf/libperf/g
And I would also agree 8-)
- Arnaldo