netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
@ 2019-12-02 13:18 Jiri Olsa
  2019-12-02 13:18 ` [PATCH 1/6] perf tools: Allow to specify libbpf install directory Jiri Olsa
                   ` (6 more replies)
  0 siblings, 7 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

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

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  libbpf/dyn

v4 changes:
  - based on Toke's v3 post, there's no need for additional API exports:

    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.

  - changing some Makefile variable names
  - documenting LIBBPF_DYNAMIC and LIBBPF_DIR in the Makefile comment
  - extending test_bpftool_build.sh with libbpf dynamic link

thanks,
jirka


---
Jiri Olsa (6):
      perf tools: Allow to specify libbpf install directory
      bpftool: Allow to link libbpf dynamically
      bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR
      bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT
      bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH
      selftests, bpftool: Add build test for libbpf dynamic linking

 tools/bpf/bpftool/Makefile                        | 54 ++++++++++++++++++++++++++++++++++++++++++++++--------
 tools/perf/Makefile.config                        | 27 ++++++++++++++++++++-------
 tools/testing/selftests/bpf/test_bpftool_build.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 15 deletions(-)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/6] perf tools: Allow to specify libbpf install directory
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 13:18 ` [PATCH 2/6] bpftool: Allow to link libbpf dynamically Jiri Olsa
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

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/ VF=1

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 <jolsa@kernel.org>
---
 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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 2/6] bpftool: Allow to link libbpf dynamically
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
  2019-12-02 13:18 ` [PATCH 1/6] perf tools: Allow to specify libbpf install directory Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 13:18 ` [PATCH 3/6] bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR Jiri Olsa
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

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: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/Makefile | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 39bc6f0f4f0b..be6dea7eb9fd 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -1,6 +1,21 @@
 # SPDX-License-Identifier: GPL-2.0-only
+
+# To link dynamically with libbpf run:
+#   make LIBBPF_DYNAMIC=1
+# build will try to detect installed devel package of libbpf.
+#
+# For linking dynamically with libbpf installed at specific PATH run:
+#   make LIBBPF_DYNAMIC=1 LIBBPF_DIR=<PATH>
+
 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 +78,17 @@ 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
+
+  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 +114,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.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 3/6] bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
  2019-12-02 13:18 ` [PATCH 1/6] perf tools: Allow to specify libbpf install directory Jiri Olsa
  2019-12-02 13:18 ` [PATCH 2/6] bpftool: Allow to link libbpf dynamically Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 13:18 ` [PATCH 4/6] bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT Jiri Olsa
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

To properly describe the usage of the variable.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index be6dea7eb9fd..512504956315 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -29,13 +29,13 @@ else
   Q = @
 endif
 
-BPF_DIR = $(srctree)/tools/lib/bpf/
+LIBBPF_SRC_DIR = $(srctree)/tools/lib/bpf/
 
 ifneq ($(OUTPUT),)
   LIBBPF_OUTPUT = $(OUTPUT)/libbpf/
   LIBBPF_PATH = $(LIBBPF_OUTPUT)
 else
-  LIBBPF_PATH = $(BPF_DIR)
+  LIBBPF_PATH = $(LIBBPF_SRC_DIR)
 endif
 
 LIBBPF = $(LIBBPF_PATH)libbpf.a
@@ -44,11 +44,11 @@ BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelvers
 
 $(LIBBPF): FORCE
 	$(if $(LIBBPF_OUTPUT),@mkdir -p $(LIBBPF_OUTPUT))
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) $(LIBBPF_OUTPUT)libbpf.a
+	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_OUTPUT) $(LIBBPF_OUTPUT)libbpf.a
 
 $(LIBBPF)-clean:
 	$(call QUIET_CLEAN, libbpf)
-	$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(LIBBPF_OUTPUT) clean >/dev/null
+	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_OUTPUT) clean >/dev/null
 
 prefix ?= /usr/local
 bash_compdir ?= /usr/share/bash-completion/completions
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 4/6] bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
                   ` (2 preceding siblings ...)
  2019-12-02 13:18 ` [PATCH 3/6] bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 13:18 ` [PATCH 5/6] bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH Jiri Olsa
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

To properly describe the usage of the variable.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 512504956315..38f831750ffe 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -32,8 +32,8 @@ endif
 LIBBPF_SRC_DIR = $(srctree)/tools/lib/bpf/
 
 ifneq ($(OUTPUT),)
-  LIBBPF_OUTPUT = $(OUTPUT)/libbpf/
-  LIBBPF_PATH = $(LIBBPF_OUTPUT)
+  LIBBPF_BUILD_OUTPUT = $(OUTPUT)/libbpf/
+  LIBBPF_PATH = $(LIBBPF_BUILD_OUTPUT)
 else
   LIBBPF_PATH = $(LIBBPF_SRC_DIR)
 endif
@@ -43,12 +43,12 @@ LIBBPF = $(LIBBPF_PATH)libbpf.a
 BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 
 $(LIBBPF): FORCE
-	$(if $(LIBBPF_OUTPUT),@mkdir -p $(LIBBPF_OUTPUT))
-	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_OUTPUT) $(LIBBPF_OUTPUT)libbpf.a
+	$(if $(LIBBPF_BUILD_OUTPUT),@mkdir -p $(LIBBPF_BUILD_OUTPUT))
+	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_BUILD_OUTPUT) $(LIBBPF_BUILD_OUTPUT)libbpf.a
 
 $(LIBBPF)-clean:
 	$(call QUIET_CLEAN, libbpf)
-	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_OUTPUT) clean >/dev/null
+	$(Q)$(MAKE) -C $(LIBBPF_SRC_DIR) OUTPUT=$(LIBBPF_BUILD_OUTPUT) clean >/dev/null
 
 prefix ?= /usr/local
 bash_compdir ?= /usr/share/bash-completion/completions
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 5/6] bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
                   ` (3 preceding siblings ...)
  2019-12-02 13:18 ` [PATCH 4/6] bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 13:18 ` [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking Jiri Olsa
  2019-12-02 19:41 ` [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Andrii Nakryiko
  6 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

To properly describe the usage of the variable.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/bpftool/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 38f831750ffe..e66d49bd6aff 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -33,12 +33,12 @@ LIBBPF_SRC_DIR = $(srctree)/tools/lib/bpf/
 
 ifneq ($(OUTPUT),)
   LIBBPF_BUILD_OUTPUT = $(OUTPUT)/libbpf/
-  LIBBPF_PATH = $(LIBBPF_BUILD_OUTPUT)
+  LIBBPF_BUILD_PATH = $(LIBBPF_BUILD_OUTPUT)
 else
-  LIBBPF_PATH = $(LIBBPF_SRC_DIR)
+  LIBBPF_BUILD_PATH = $(LIBBPF_SRC_DIR)
 endif
 
-LIBBPF = $(LIBBPF_PATH)libbpf.a
+LIBBPF = $(LIBBPF_BUILD_PATH)libbpf.a
 
 BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
                   ` (4 preceding siblings ...)
  2019-12-02 13:18 ` [PATCH 5/6] bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH Jiri Olsa
@ 2019-12-02 13:18 ` Jiri Olsa
  2019-12-02 15:38   ` Quentin Monnet
  2019-12-02 19:41 ` [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Andrii Nakryiko
  6 siblings, 1 reply; 29+ messages in thread
From: Jiri Olsa @ 2019-12-02 13:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

Adding new test to test_bpftool_build.sh script to
test the dynamic linkage of libbpf for bpftool:

  $ ./test_bpftool_build.sh
  [SNIP]

  ... with dynamic libbpf

  $PWD:    /home/jolsa/kernel/linux-perf/tools/bpf/bpftool
  command: make -s -C ../../build/feature clean >/dev/null
  command: make -s -C ../../lib/bpf clean >/dev/null
  command: make -s -C ../../lib/bpf prefix=/tmp/tmp.fG8O2Ps8ER install_lib install_headers >/dev/null
  Parsed description of 117 helper function(s)
  command: make -s clean >/dev/null
  command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/tmp.fG8O2Ps8ER >/dev/null
  binary:  /home/jolsa/kernel/linux-perf/tools/bpf/bpftool/bpftool
  binary:  linked with libbpf

The test installs libbpf into temp directory
and links bpftool dynamically with it.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/test_bpftool_build.sh       | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
index ac349a5cea7e..e4a6a0520f8e 100755
--- a/tools/testing/selftests/bpf/test_bpftool_build.sh
+++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
@@ -85,6 +85,55 @@ make_with_tmpdir() {
 	echo
 }
 
+# Assumes current directory is tools/bpf/bpftool
+make_with_dynamic_libbpf() {
+	TMPDIR=$(mktemp -d)
+	echo -e "\$PWD:    $PWD"
+
+	# It might be needed to clean build tree first because features
+	# framework does not detect the change properly
+	echo -e "command: make -s -C ../../build/feature clean >/dev/null"
+	make $J -s -C ../../build/feature clean >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+	echo -e "command: make -s -C ../../lib/bpf clean >/dev/null"
+	make $J -s -C ../../lib/bpf clean >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+
+	# Now install libbpf into TMPDIR
+	echo -e "command: make -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null"
+	make $J -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+
+	# And final bpftool build (with clean first) with libbpf dynamic link
+	echo -e "command: make -s clean >/dev/null"
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+	echo -e "command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null"
+	make $J -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null
+	if [ $? -ne 0 ] ; then
+		ERROR=1
+	fi
+
+	check .
+	ldd bpftool | grep -q libbpf.so
+	if [ $? -ne 0 ] ; then
+		printf "FAILURE: Did not find libbpf linked\n"
+	else
+		echo "binary:  linked with libbpf"
+	fi
+	make -s -C ../../lib/bpf clean
+	make -s clean
+	rm -rf -- $TMPDIR
+	echo
+}
+
 echo "Trying to build bpftool"
 echo -e "... through kbuild\n"
 
@@ -145,3 +194,7 @@ make_and_clean
 make_with_tmpdir OUTPUT
 
 make_with_tmpdir O
+
+echo -e "... with dynamic libbpf\n"
+
+make_with_dynamic_libbpf
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking
  2019-12-02 13:18 ` [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking Jiri Olsa
@ 2019-12-02 15:38   ` Quentin Monnet
  0 siblings, 0 replies; 29+ messages in thread
From: Quentin Monnet @ 2019-12-02 15:38 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: lkml, netdev, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Toke Høiland-Jørgensen,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko

Thanks Jiri ! A few comments inline.

2019-12-02 14:18 UTC+0100 ~ Jiri Olsa <jolsa@kernel.org>
> Adding new test to test_bpftool_build.sh script to
> test the dynamic linkage of libbpf for bpftool:
> 
>   $ ./test_bpftool_build.sh
>   [SNIP]
> 
>   ... with dynamic libbpf
> 
>   $PWD:    /home/jolsa/kernel/linux-perf/tools/bpf/bpftool
>   command: make -s -C ../../build/feature clean >/dev/null
>   command: make -s -C ../../lib/bpf clean >/dev/null
>   command: make -s -C ../../lib/bpf prefix=/tmp/tmp.fG8O2Ps8ER install_lib install_headers >/dev/null
>   Parsed description of 117 helper function(s)
>   command: make -s clean >/dev/null
>   command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=/tmp/tmp.fG8O2Ps8ER >/dev/null
>   binary:  /home/jolsa/kernel/linux-perf/tools/bpf/bpftool/bpftool
>   binary:  linked with libbpf
> 
> The test installs libbpf into temp directory
> and links bpftool dynamically with it.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../selftests/bpf/test_bpftool_build.sh       | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/test_bpftool_build.sh b/tools/testing/selftests/bpf/test_bpftool_build.sh
> index ac349a5cea7e..e4a6a0520f8e 100755
> --- a/tools/testing/selftests/bpf/test_bpftool_build.sh
> +++ b/tools/testing/selftests/bpf/test_bpftool_build.sh
> @@ -85,6 +85,55 @@ make_with_tmpdir() {
>  	echo
>  }
>  
> +# Assumes current directory is tools/bpf/bpftool
> +make_with_dynamic_libbpf() {
> +	TMPDIR=$(mktemp -d)
> +	echo -e "\$PWD:    $PWD"
> +
> +	# It might be needed to clean build tree first because features
> +	# framework does not detect the change properly
> +	echo -e "command: make -s -C ../../build/feature clean >/dev/null"

(So far I did not echo the "make clean" commands, I printed only the
ones used to build bpftool. But that's your call.)

> +	make $J -s -C ../../build/feature clean >/dev/null
> +	if [ $? -ne 0 ] ; then
> +		ERROR=1
> +	fi
> +	echo -e "command: make -s -C ../../lib/bpf clean >/dev/null"
> +	make $J -s -C ../../lib/bpf clean >/dev/null
> +	if [ $? -ne 0 ] ; then
> +		ERROR=1
> +	fi
> +
> +	# Now install libbpf into TMPDIR
> +	echo -e "command: make -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null"
> +	make $J -s -C ../../lib/bpf prefix=$TMPDIR install_lib install_headers >/dev/null
> +	if [ $? -ne 0 ] ; then
> +		ERROR=1
> +	fi
> +
> +	# And final bpftool build (with clean first) with libbpf dynamic link
> +	echo -e "command: make -s clean >/dev/null"
> +	if [ $? -ne 0 ] ; then
> +		ERROR=1
> +	fi

I do not believe you need to "make clean" here, this should have been
done by the previous test in that dir earlier in the script (cd
tools/bpf/bpftool; make_and_clean)

> +	echo -e "command: make -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null"
> +	make $J -s LIBBPF_DYNAMIC=1 LIBBPF_DIR=$TMPDIR >/dev/null
> +	if [ $? -ne 0 ] ; then
> +		ERROR=1
> +	fi
> +
> +	check .
> +	ldd bpftool | grep -q libbpf.so
> +	if [ $? -ne 0 ] ; then

(Or "if ldd bpftool | grep -q libbpf.so ; then")

> +		printf "FAILURE: Did not find libbpf linked\n"

Please also set $(ERROR) here.

(Also, stick to echo rather than mixing with printf? I can't remember
why I used one over echo in the check() function, that was probably not
on purpose.)

> +	else
> +		echo "binary:  linked with libbpf"
> +	fi
> +	make -s -C ../../lib/bpf clean
> +	make -s clean
> +	rm -rf -- $TMPDIR

We probably want to clean features too? We tried to check that libbpf
was available, but with a very specific $(LIBBPF_DIR), which was
temporary and no longer exist. So better to reset features to a clean state?

> +	echo
> +}
> +
>  echo "Trying to build bpftool"
>  echo -e "... through kbuild\n"
>  
> @@ -145,3 +194,7 @@ make_and_clean
>  make_with_tmpdir OUTPUT
>  
>  make_with_tmpdir O
> +
> +echo -e "... with dynamic libbpf\n"
> +
> +make_with_dynamic_libbpf
> 

Thanks,
Quentin

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
                   ` (5 preceding siblings ...)
  2019-12-02 13:18 ` [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking Jiri Olsa
@ 2019-12-02 19:41 ` Andrii Nakryiko
  2019-12-02 21:15   ` Toke Høiland-Jørgensen
  6 siblings, 1 reply; 29+ messages in thread
From: Andrii Nakryiko @ 2019-12-02 19:41 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Toke Høiland-Jørgensen, Jesper Dangaard Brouer,
	Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
	Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Mon, Dec 2, 2019 at 5:19 AM Jiri Olsa <jolsa@kernel.org> 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 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.
>
> Also available in:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   libbpf/dyn
>
> v4 changes:
>   - based on Toke's v3 post, there's no need for additional API exports:
>
>     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.

Whoever understands how this is supposed to work, can you please
explain? From reading this, I think what we **want** is:

- all LIBBPF_API-exposed APIs should be dynamically linked against libbpf.so;
- everything else used from libbpf (e.g., netlink APIs), should come
from libbpf.a.

Am I getting the idea right?

If yes, are we sure it actually works like that in practice? I've
compiled with LIBBPF_DYNAMIC=1, and what I see is that libelf, libc,
zlib, etc functions do have relocations against them in ".rela.plt"
section. None of libbpf exposed APIs, though, have any of such
relocations. Which to me suggests that they are just statically linked
against libbpf.a and libbpf.so is just recorded in ELF as a dynamic
library dependency because of this extra -lbpf flag. Which kind of
defeats the purpose of this whole endeavor, no?

I'm no linker expert, though, so I apologize if I got it completely
wrong, would really appreciate someone to detail this a bit more.
Thanks!

>
>   - changing some Makefile variable names
>   - documenting LIBBPF_DYNAMIC and LIBBPF_DIR in the Makefile comment
>   - extending test_bpftool_build.sh with libbpf dynamic link
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (6):
>       perf tools: Allow to specify libbpf install directory
>       bpftool: Allow to link libbpf dynamically
>       bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR
>       bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT
>       bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH
>       selftests, bpftool: Add build test for libbpf dynamic linking
>
>  tools/bpf/bpftool/Makefile                        | 54 ++++++++++++++++++++++++++++++++++++++++++++++--------
>  tools/perf/Makefile.config                        | 27 ++++++++++++++++++++-------
>  tools/testing/selftests/bpf/test_bpftool_build.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 119 insertions(+), 15 deletions(-)
>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-02 19:41 ` [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Andrii Nakryiko
@ 2019-12-02 21:15   ` Toke Høiland-Jørgensen
  2019-12-04  5:52     ` Alexei Starovoitov
  0 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-02 21:15 UTC (permalink / raw)
  To: Andrii Nakryiko, Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Mon, Dec 2, 2019 at 5:19 AM Jiri Olsa <jolsa@kernel.org> 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 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.
>>
>> Also available in:
>>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>>   libbpf/dyn
>>
>> v4 changes:
>>   - based on Toke's v3 post, there's no need for additional API exports:
>>
>>     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.
>
> Whoever understands how this is supposed to work, can you please
> explain? From reading this, I think what we **want** is:
>
> - all LIBBPF_API-exposed APIs should be dynamically linked against libbpf.so;
> - everything else used from libbpf (e.g., netlink APIs), should come
> from libbpf.a.
>
> Am I getting the idea right?
>
> If yes, are we sure it actually works like that in practice? I've
> compiled with LIBBPF_DYNAMIC=1, and what I see is that libelf, libc,
> zlib, etc functions do have relocations against them in ".rela.plt"
> section. None of libbpf exposed APIs, though, have any of such
> relocations. Which to me suggests that they are just statically linked
> against libbpf.a and libbpf.so is just recorded in ELF as a dynamic
> library dependency because of this extra -lbpf flag. Which kind of
> defeats the purpose of this whole endeavor, no?
>
> I'm no linker expert, though, so I apologize if I got it completely
> wrong, would really appreciate someone to detail this a bit more.
> Thanks!

Ah, that is my mistake: I was getting dynamic libbpf symbols with this
approach, but that was because I had the version of libbpf.so in my
$LIBDIR that had the patch to expose the netlink APIs as versioned
symbols; so it was just pulling in everything from the shared library.

So what I was going for was exactly what you described above; but it
seems that doesn't actually work. Too bad, and sorry for wasting your
time on this :/

-Toke


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-02 21:15   ` Toke Høiland-Jørgensen
@ 2019-12-04  5:52     ` Alexei Starovoitov
  2019-12-04  9:01       ` Jiri Olsa
                         ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-04  5:52 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Andrii Nakryiko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	Networking, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Jesper Dangaard Brouer,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, Quentin Monnet

On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Ah, that is my mistake: I was getting dynamic libbpf symbols with this
> approach, but that was because I had the version of libbpf.so in my
> $LIBDIR that had the patch to expose the netlink APIs as versioned
> symbols; so it was just pulling in everything from the shared library.
>
> So what I was going for was exactly what you described above; but it
> seems that doesn't actually work. Too bad, and sorry for wasting your
> time on this :/

bpftool is currently tightly coupled with libbpf and very likely
in the future the dependency will be even tighter.
In that sense bpftool is an extension of libbpf and libbpf is an extension
of bpftool.
Andrii is working on set of patches to generate user space .c code
from bpf program.
bpftool will be generating the code that is specific for the version
bpftool and for
the version of libbpf. There will be compatibility layers as usual.
But in general the situation where a bug in libbpf is so criticial
that bpftool needs to repackaged is imo less likely than a bug in
bpftool that will require re-packaging of libbpf.
bpftool is quite special. It's not a typical user of libbpf.
The other way around is more correct. libbpf is a user of the code
that bpftool generates and both depend on each other.
perf on the other side is what typical user space app that uses
libbpf will look like.
I think keeping bpftool in the kernel while packaging libbpf
out of github was an oversight.
I think we need to mirror bpftool into github/libbpf as well
and make sure they stay together. The version of libbpf == version of bpftool.
Both should come from the same package and so on.
May be they can be two different packages but
upgrading one should trigger upgrade of another and vice versa.
I think one package would be easier though.
Thoughts?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04  5:52     ` Alexei Starovoitov
@ 2019-12-04  9:01       ` Jiri Olsa
  2019-12-04 10:57       ` Toke Høiland-Jørgensen
  2019-12-04 21:16       ` Andrii Nakryiko
  2 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2019-12-04  9:01 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Tue, Dec 03, 2019 at 09:52:11PM -0800, Alexei Starovoitov wrote:
> On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Ah, that is my mistake: I was getting dynamic libbpf symbols with this
> > approach, but that was because I had the version of libbpf.so in my
> > $LIBDIR that had the patch to expose the netlink APIs as versioned
> > symbols; so it was just pulling in everything from the shared library.
> >
> > So what I was going for was exactly what you described above; but it
> > seems that doesn't actually work. Too bad, and sorry for wasting your
> > time on this :/
> 
> bpftool is currently tightly coupled with libbpf and very likely
> in the future the dependency will be even tighter.
> In that sense bpftool is an extension of libbpf and libbpf is an extension
> of bpftool.
> Andrii is working on set of patches to generate user space .c code
> from bpf program.
> bpftool will be generating the code that is specific for the version
> bpftool and for
> the version of libbpf. There will be compatibility layers as usual.
> But in general the situation where a bug in libbpf is so criticial
> that bpftool needs to repackaged is imo less likely than a bug in
> bpftool that will require re-packaging of libbpf.
> bpftool is quite special. It's not a typical user of libbpf.
> The other way around is more correct. libbpf is a user of the code
> that bpftool generates and both depend on each other.
> perf on the other side is what typical user space app that uses
> libbpf will look like.
> I think keeping bpftool in the kernel while packaging libbpf
> out of github was an oversight.
> I think we need to mirror bpftool into github/libbpf as well
> and make sure they stay together. The version of libbpf == version of bpftool.
> Both should come from the same package and so on.
> May be they can be two different packages but
> upgrading one should trigger upgrade of another and vice versa.
> I think one package would be easier though.
> Thoughts?

great, makes sense.. Toke already mentioned we might need to move
bpftool to libbpf package to solve our issues, now when you guys
have already plans for that, it works for us ;-)

thanks for sharing the plan,
jirka


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04  5:52     ` Alexei Starovoitov
  2019-12-04  9:01       ` Jiri Olsa
@ 2019-12-04 10:57       ` Toke Høiland-Jørgensen
  2019-12-04 17:39         ` Alexei Starovoitov
  2019-12-04 21:16       ` Andrii Nakryiko
  2 siblings, 1 reply; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-04 10:57 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	Networking, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Jesper Dangaard Brouer,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, Quentin Monnet

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>>
>> Ah, that is my mistake: I was getting dynamic libbpf symbols with this
>> approach, but that was because I had the version of libbpf.so in my
>> $LIBDIR that had the patch to expose the netlink APIs as versioned
>> symbols; so it was just pulling in everything from the shared library.
>>
>> So what I was going for was exactly what you described above; but it
>> seems that doesn't actually work. Too bad, and sorry for wasting your
>> time on this :/
>
> bpftool is currently tightly coupled with libbpf and very likely
> in the future the dependency will be even tighter.
> In that sense bpftool is an extension of libbpf and libbpf is an extension
> of bpftool.
> Andrii is working on set of patches to generate user space .c code
> from bpf program.
> bpftool will be generating the code that is specific for the version
> bpftool and for
> the version of libbpf. There will be compatibility layers as usual.
> But in general the situation where a bug in libbpf is so criticial
> that bpftool needs to repackaged is imo less likely than a bug in
> bpftool that will require re-packaging of libbpf.
> bpftool is quite special. It's not a typical user of libbpf.
> The other way around is more correct. libbpf is a user of the code
> that bpftool generates and both depend on each other.
> perf on the other side is what typical user space app that uses
> libbpf will look like.
> I think keeping bpftool in the kernel while packaging libbpf
> out of github was an oversight.
> I think we need to mirror bpftool into github/libbpf as well
> and make sure they stay together. The version of libbpf == version of bpftool.
> Both should come from the same package and so on.
> May be they can be two different packages but
> upgrading one should trigger upgrade of another and vice versa.
> I think one package would be easier though.
> Thoughts?

Yup, making bpftool explicitly the "libbpf command line interface" makes
sense and would help clarify the relationship between the two. As Jiri
said, we are already moving in that direction packaging-wise...

-Toke


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 10:57       ` Toke Høiland-Jørgensen
@ 2019-12-04 17:39         ` Alexei Starovoitov
  2019-12-04 18:27           ` Daniel Borkmann
  0 siblings, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-04 17:39 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Andrii Nakryiko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	Networking, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Jesper Dangaard Brouer,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, Quentin Monnet

On Wed, Dec 4, 2019 at 2:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> Ah, that is my mistake: I was getting dynamic libbpf symbols with this
> >> approach, but that was because I had the version of libbpf.so in my
> >> $LIBDIR that had the patch to expose the netlink APIs as versioned
> >> symbols; so it was just pulling in everything from the shared library.
> >>
> >> So what I was going for was exactly what you described above; but it
> >> seems that doesn't actually work. Too bad, and sorry for wasting your
> >> time on this :/
> >
> > bpftool is currently tightly coupled with libbpf and very likely
> > in the future the dependency will be even tighter.
> > In that sense bpftool is an extension of libbpf and libbpf is an extension
> > of bpftool.
> > Andrii is working on set of patches to generate user space .c code
> > from bpf program.
> > bpftool will be generating the code that is specific for the version
> > bpftool and for
> > the version of libbpf. There will be compatibility layers as usual.
> > But in general the situation where a bug in libbpf is so criticial
> > that bpftool needs to repackaged is imo less likely than a bug in
> > bpftool that will require re-packaging of libbpf.
> > bpftool is quite special. It's not a typical user of libbpf.
> > The other way around is more correct. libbpf is a user of the code
> > that bpftool generates and both depend on each other.
> > perf on the other side is what typical user space app that uses
> > libbpf will look like.
> > I think keeping bpftool in the kernel while packaging libbpf
> > out of github was an oversight.
> > I think we need to mirror bpftool into github/libbpf as well
> > and make sure they stay together. The version of libbpf == version of bpftool.
> > Both should come from the same package and so on.
> > May be they can be two different packages but
> > upgrading one should trigger upgrade of another and vice versa.
> > I think one package would be easier though.
> > Thoughts?
>
> Yup, making bpftool explicitly the "libbpf command line interface" makes
> sense and would help clarify the relationship between the two. As Jiri
> said, we are already moving in that direction packaging-wise...

Awesome. Let's figure out the logistics.
Should we do:
git mv tools/bpf/bpftool/ tools/lib/bpf/
and appropriate adjustment to Makefiles ?
or keep it where it is and only add to
https://github.com/libbpf/libbpf/blob/master/scripts/sync-kernel.sh ?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 17:39         ` Alexei Starovoitov
@ 2019-12-04 18:27           ` Daniel Borkmann
  2019-12-04 20:22             ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Borkmann @ 2019-12-04 18:27 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, Andrii Nakryiko, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Martin KaFai Lau, Song Liu,
	Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, Dec 04, 2019 at 09:39:59AM -0800, Alexei Starovoitov wrote:
> On Wed, Dec 4, 2019 at 2:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >>
> > >> Ah, that is my mistake: I was getting dynamic libbpf symbols with this
> > >> approach, but that was because I had the version of libbpf.so in my
> > >> $LIBDIR that had the patch to expose the netlink APIs as versioned
> > >> symbols; so it was just pulling in everything from the shared library.
> > >>
> > >> So what I was going for was exactly what you described above; but it
> > >> seems that doesn't actually work. Too bad, and sorry for wasting your
> > >> time on this :/
> > >
> > > bpftool is currently tightly coupled with libbpf and very likely
> > > in the future the dependency will be even tighter.
> > > In that sense bpftool is an extension of libbpf and libbpf is an extension
> > > of bpftool.
> > > Andrii is working on set of patches to generate user space .c code
> > > from bpf program.
> > > bpftool will be generating the code that is specific for the version
> > > bpftool and for
> > > the version of libbpf. There will be compatibility layers as usual.
> > > But in general the situation where a bug in libbpf is so criticial
> > > that bpftool needs to repackaged is imo less likely than a bug in
> > > bpftool that will require re-packaging of libbpf.
> > > bpftool is quite special. It's not a typical user of libbpf.
> > > The other way around is more correct. libbpf is a user of the code
> > > that bpftool generates and both depend on each other.
> > > perf on the other side is what typical user space app that uses
> > > libbpf will look like.
> > > I think keeping bpftool in the kernel while packaging libbpf
> > > out of github was an oversight.
> > > I think we need to mirror bpftool into github/libbpf as well
> > > and make sure they stay together. The version of libbpf == version of bpftool.
> > > Both should come from the same package and so on.
> > > May be they can be two different packages but
> > > upgrading one should trigger upgrade of another and vice versa.
> > > I think one package would be easier though.
> > > Thoughts?
> >
> > Yup, making bpftool explicitly the "libbpf command line interface" makes
> > sense and would help clarify the relationship between the two. As Jiri
> > said, we are already moving in that direction packaging-wise...
> 
> Awesome. Let's figure out the logistics.
> Should we do:
> git mv tools/bpf/bpftool/ tools/lib/bpf/
> and appropriate adjustment to Makefiles ?
> or keep it where it is and only add to
> https://github.com/libbpf/libbpf/blob/master/scripts/sync-kernel.sh ?

I'd be in preference of the latter aka keeping where it is.

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 18:27           ` Daniel Borkmann
@ 2019-12-04 20:22             ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 29+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-12-04 20:22 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov
  Cc: Andrii Nakryiko, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
	Networking, bpf, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
	Peter Zijlstra, Michael Petlan, Jesper Dangaard Brouer,
	Martin KaFai Lau, Song Liu, Yonghong Song, Andrii Nakryiko,
	Quentin Monnet

Daniel Borkmann <daniel@iogearbox.net> writes:

> On Wed, Dec 04, 2019 at 09:39:59AM -0800, Alexei Starovoitov wrote:
>> On Wed, Dec 4, 2019 at 2:58 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>> > > On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>> > >>
>> > >> Ah, that is my mistake: I was getting dynamic libbpf symbols with this
>> > >> approach, but that was because I had the version of libbpf.so in my
>> > >> $LIBDIR that had the patch to expose the netlink APIs as versioned
>> > >> symbols; so it was just pulling in everything from the shared library.
>> > >>
>> > >> So what I was going for was exactly what you described above; but it
>> > >> seems that doesn't actually work. Too bad, and sorry for wasting your
>> > >> time on this :/
>> > >
>> > > bpftool is currently tightly coupled with libbpf and very likely
>> > > in the future the dependency will be even tighter.
>> > > In that sense bpftool is an extension of libbpf and libbpf is an extension
>> > > of bpftool.
>> > > Andrii is working on set of patches to generate user space .c code
>> > > from bpf program.
>> > > bpftool will be generating the code that is specific for the version
>> > > bpftool and for
>> > > the version of libbpf. There will be compatibility layers as usual.
>> > > But in general the situation where a bug in libbpf is so criticial
>> > > that bpftool needs to repackaged is imo less likely than a bug in
>> > > bpftool that will require re-packaging of libbpf.
>> > > bpftool is quite special. It's not a typical user of libbpf.
>> > > The other way around is more correct. libbpf is a user of the code
>> > > that bpftool generates and both depend on each other.
>> > > perf on the other side is what typical user space app that uses
>> > > libbpf will look like.
>> > > I think keeping bpftool in the kernel while packaging libbpf
>> > > out of github was an oversight.
>> > > I think we need to mirror bpftool into github/libbpf as well
>> > > and make sure they stay together. The version of libbpf == version of bpftool.
>> > > Both should come from the same package and so on.
>> > > May be they can be two different packages but
>> > > upgrading one should trigger upgrade of another and vice versa.
>> > > I think one package would be easier though.
>> > > Thoughts?
>> >
>> > Yup, making bpftool explicitly the "libbpf command line interface" makes
>> > sense and would help clarify the relationship between the two. As Jiri
>> > said, we are already moving in that direction packaging-wise...
>> 
>> Awesome. Let's figure out the logistics.
>> Should we do:
>> git mv tools/bpf/bpftool/ tools/lib/bpf/
>> and appropriate adjustment to Makefiles ?
>> or keep it where it is and only add to
>> https://github.com/libbpf/libbpf/blob/master/scripts/sync-kernel.sh ?
>
> I'd be in preference of the latter aka keeping where it is.

I don't have any strong preference either way. It would make sense to
move it to make clear the interdependency (and that bpftool is really
the "libbpf cli interface"); but it could also just be kept separate and
just document this in the existing bpftool dir.

The github repository may need some surgery, though. So maybe let the
changes in the kernel tree depend on what's easiest for that? IDK?

-Toke


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04  5:52     ` Alexei Starovoitov
  2019-12-04  9:01       ` Jiri Olsa
  2019-12-04 10:57       ` Toke Høiland-Jørgensen
@ 2019-12-04 21:16       ` Andrii Nakryiko
  2019-12-04 21:54         ` Jakub Kicinski
  2 siblings, 1 reply; 29+ messages in thread
From: Andrii Nakryiko @ 2019-12-04 21:16 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Tue, Dec 3, 2019 at 9:52 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Dec 2, 2019 at 1:15 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >
> > Ah, that is my mistake: I was getting dynamic libbpf symbols with this
> > approach, but that was because I had the version of libbpf.so in my
> > $LIBDIR that had the patch to expose the netlink APIs as versioned
> > symbols; so it was just pulling in everything from the shared library.
> >
> > So what I was going for was exactly what you described above; but it
> > seems that doesn't actually work. Too bad, and sorry for wasting your
> > time on this :/
>
> bpftool is currently tightly coupled with libbpf and very likely
> in the future the dependency will be even tighter.
> In that sense bpftool is an extension of libbpf and libbpf is an extension
> of bpftool.
> Andrii is working on set of patches to generate user space .c code
> from bpf program.
> bpftool will be generating the code that is specific for the version
> bpftool and for
> the version of libbpf. There will be compatibility layers as usual.
> But in general the situation where a bug in libbpf is so criticial
> that bpftool needs to repackaged is imo less likely than a bug in
> bpftool that will require re-packaging of libbpf.
> bpftool is quite special. It's not a typical user of libbpf.
> The other way around is more correct. libbpf is a user of the code
> that bpftool generates and both depend on each other.
> perf on the other side is what typical user space app that uses
> libbpf will look like.
> I think keeping bpftool in the kernel while packaging libbpf
> out of github was an oversight.

I wonder what big advantage having bpftool in libbpf's Github repo
brings, actually? The reason we need libbpf on github is to allow
other projects like pahole to be able to use libbpf from submodule.
There is no such need for bpftool.

I agree about preference to release them in sync, but that could be
easily done by releasing based on corresponding commits in github's
libbpf repo and kernel repo. bpftool doesn't have to physically live
next to libbpf on Github, does it?

Calling github repo a "mirror" is incorrect. It's not a 1:1 copy of
files. We have a completely separate Makefile for libbpf, and we have
a bunch of stuff we had to re-implement to detach libbpf code from
kernel's non-UAPI headers. Doing this for bpftool as well seems like
just more maintenance. Keeping github's Makefile in sync with kernel's
Makefile (for libbpf) is PITA, I'd rather avoid similar pains for
bpftool without a really good reason.

> I think we need to mirror bpftool into github/libbpf as well
> and make sure they stay together. The version of libbpf == version of bpftool.
> Both should come from the same package and so on.
> May be they can be two different packages but
> upgrading one should trigger upgrade of another and vice versa.
> I think one package would be easier though.
> Thoughts?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 21:16       ` Andrii Nakryiko
@ 2019-12-04 21:54         ` Jakub Kicinski
  2019-12-04 23:39           ` Alexei Starovoitov
  0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-12-04 21:54 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, 4 Dec 2019 13:16:13 -0800, Andrii Nakryiko wrote:
> I wonder what big advantage having bpftool in libbpf's Github repo
> brings, actually? The reason we need libbpf on github is to allow
> other projects like pahole to be able to use libbpf from submodule.
> There is no such need for bpftool.
> 
> I agree about preference to release them in sync, but that could be
> easily done by releasing based on corresponding commits in github's
> libbpf repo and kernel repo. bpftool doesn't have to physically live
> next to libbpf on Github, does it?

+1

> Calling github repo a "mirror" is incorrect. It's not a 1:1 copy of
> files. We have a completely separate Makefile for libbpf, and we have
> a bunch of stuff we had to re-implement to detach libbpf code from
> kernel's non-UAPI headers. Doing this for bpftool as well seems like
> just more maintenance. Keeping github's Makefile in sync with kernel's
> Makefile (for libbpf) is PITA, I'd rather avoid similar pains for
> bpftool without a really good reason.

Agreed. Having libbpf on GH is definitely useful today, but one can hope
a day will come when distroes will get up to speed on packaging libbpf,
and perhaps we can retire it? Maybe 2, 3 years from now? Putting
bpftool in the same boat is just more baggage.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 21:54         ` Jakub Kicinski
@ 2019-12-04 23:39           ` Alexei Starovoitov
  2019-12-05  0:23             ` Jakub Kicinski
  2019-12-05  8:35             ` Jesper Dangaard Brouer
  0 siblings, 2 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-04 23:39 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, Dec 04, 2019 at 01:54:05PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Dec 2019 13:16:13 -0800, Andrii Nakryiko wrote:
> > I wonder what big advantage having bpftool in libbpf's Github repo
> > brings, actually? The reason we need libbpf on github is to allow
> > other projects like pahole to be able to use libbpf from submodule.
> > There is no such need for bpftool.
> > 
> > I agree about preference to release them in sync, but that could be
> > easily done by releasing based on corresponding commits in github's
> > libbpf repo and kernel repo. bpftool doesn't have to physically live
> > next to libbpf on Github, does it?
> 
> +1
> 
> > Calling github repo a "mirror" is incorrect. It's not a 1:1 copy of
> > files. We have a completely separate Makefile for libbpf, and we have
> > a bunch of stuff we had to re-implement to detach libbpf code from
> > kernel's non-UAPI headers. Doing this for bpftool as well seems like
> > just more maintenance. Keeping github's Makefile in sync with kernel's
> > Makefile (for libbpf) is PITA, I'd rather avoid similar pains for
> > bpftool without a really good reason.
> 
> Agreed. Having libbpf on GH is definitely useful today, but one can hope
> a day will come when distroes will get up to speed on packaging libbpf,
> and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> bpftool in the same boat is just more baggage.

Distros should be packaging libbpf and bpftool from single repo on github.
Kernel tree is for packaging kernel.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 23:39           ` Alexei Starovoitov
@ 2019-12-05  0:23             ` Jakub Kicinski
  2019-12-05  0:29               ` David Miller
  2019-12-05  1:09               ` Alexei Starovoitov
  2019-12-05  8:35             ` Jesper Dangaard Brouer
  1 sibling, 2 replies; 29+ messages in thread
From: Jakub Kicinski @ 2019-12-05  0:23 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:
> > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > a day will come when distroes will get up to speed on packaging libbpf,
> > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > bpftool in the same boat is just more baggage.  
> 
> Distros should be packaging libbpf and bpftool from single repo on github.
> Kernel tree is for packaging kernel.

Okay, single repo on GitHub:

https://github.com/torvalds/linux

we are in agreement 😝

Jokes aside, you may need to provide some reasoning on this one..
The recommendation for packaging libbpf from GitHub never had any 
clear justification either AFAICR.

I honestly don't see why location matters. bpftool started out on GitHub
but we moved it into the tree for... ease of packaging/distribution(?!)
Now it's handy to have it in the tree to reuse the uapi headers.

As much as I don't care if we move it (back) out of the tree - having
two copies makes no sense to me. As does having it in the libbpf repo.
The sync effort is not warranted. User confusion is not warranted.

The distroes already package bpftool from the kernel sources, people had
put in time to get to this stage and there aren't any complaints.

In fact all the BPF projects and test suites we are involved in at
Netronome are entirely happy the packaged versions of LLVM and libbpf
in Fedora _today_, IOW the GH libbpf is irrelevant to us already.

As for the problem which sparked this discussion - I disagree that
bpftool should have "special relationship" with the library. In fact
bpftool uses the widest range of libbpf's interfaces of all known
projects so it's invaluable for making sure that those interfaces are
usable, consistent and complete.

You also said a few times you don't want to merge fixes into bpf/net.
That divergence from kernel development process is worrying.

None of this makes very much sense to me. We're diverging from well
established development practices without as much as a justification.

Perhaps I'm not clever enough to follow. But if I'm allowed to make an
uneducated guess it would be that it's some Facebook internal reason,
like it's hard to do backports? :/

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  0:23             ` Jakub Kicinski
@ 2019-12-05  0:29               ` David Miller
  2019-12-05  1:25                 ` Alexei Starovoitov
  2019-12-05  1:09               ` Alexei Starovoitov
  1 sibling, 1 reply; 29+ messages in thread
From: David Miller @ 2019-12-05  0:29 UTC (permalink / raw)
  Cc: alexei.starovoitov, andrii.nakryiko, toke, jolsa, acme,
	linux-kernel, netdev, bpf, mingo, namhyung, alexander.shishkin,
	a.p.zijlstra, mpetlan, brouer, daniel, kafai, songliubraving,
	yhs, andriin, quentin.monnet

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 4 Dec 2019 16:23:48 -0800

> Jokes aside, you may need to provide some reasoning on this one..
> The recommendation for packaging libbpf from GitHub never had any 
> clear justification either AFAICR.
> 
> I honestly don't see why location matters. bpftool started out on GitHub
> but we moved it into the tree for... ease of packaging/distribution(?!)
> Now it's handy to have it in the tree to reuse the uapi headers.
> 
> As much as I don't care if we move it (back) out of the tree - having
> two copies makes no sense to me. As does having it in the libbpf repo.
> The sync effort is not warranted. User confusion is not warranted.

Part of this story has to do with how bug fixes propagate via bpf-next
instead of the bpf tree, as I understand it.

But yeah it would be nice to have a clear documentation on all of the
reasoning.

On the distro side, people seem to not want to use the separate repo.
If you're supporting enterprise customers you don't just sync with
upstream, you cherry pick.  When cherry picking gets too painful, you
sync with upstream possibly eliding upstream new features you don't
want to appear in your supported product yet.

I agree with tying bpftool and libbpf into the _resulting_ binary
distro package, but I'm not totally convinced about separating them
out of the kernel source tree.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  0:23             ` Jakub Kicinski
  2019-12-05  0:29               ` David Miller
@ 2019-12-05  1:09               ` Alexei Starovoitov
  2019-12-05  2:10                 ` Jakub Kicinski
  1 sibling, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-05  1:09 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, Dec 04, 2019 at 04:23:48PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:
> > > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > > a day will come when distroes will get up to speed on packaging libbpf,
> > > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > > bpftool in the same boat is just more baggage.  
> > 
> > Distros should be packaging libbpf and bpftool from single repo on github.
> > Kernel tree is for packaging kernel.
> 
> Okay, single repo on GitHub:
> 
> https://github.com/torvalds/linux

and how will you git submodule only libbpf part of kernel github into bcc
and other projects?

> You also said a few times you don't want to merge fixes into bpf/net.
> That divergence from kernel development process is worrying.

worrying - why? what exactly the concern you see?
Tying user space release into kernel release and user space process into
kernel process makes little sense to me. Packaging is different. Compatibility
requirements are different. CI is different. Integration with other projects is
different.

libbpf source code is in the kernel tree only because kernel changes plus
libbpf changes plus selftests changes come as single patchset. That is really
the only reason. Packaging scripts, CI scripts, etc should be kept out of
kernel tree. All that stuff belongs at github/libbpf.

> None of this makes very much sense to me. We're diverging from well
> established development practices without as much as a justification.

The kernel development process was never used for libbpf. Even coding style is
different. I'm puzzled why you think user space should be tied to kernel.
Everything is so vastly different.
Some people say that 8 weeks to bump libbpf version is too long.
Other people say that it's too often.
libbpf version numbers != kernel version numbers.
There is no definition of LTS for libbpf. One day it will be
and the version of libbpf picked for LTS will likely have
nothing to do with kernel LTS choices.
libbpf has to run on all kernels. Newer and older. How do you support that if
libbpf is tied with the kernel?

> Perhaps I'm not clever enough to follow. But if I'm allowed to make an
> uneducated guess it would be that it's some Facebook internal reason,
> like it's hard to do backports? :/

hard to do backports? of what?


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  0:29               ` David Miller
@ 2019-12-05  1:25                 ` Alexei Starovoitov
  0 siblings, 0 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-05  1:25 UTC (permalink / raw)
  To: David Miller
  Cc: andrii.nakryiko, toke, jolsa, acme, linux-kernel, netdev, bpf,
	mingo, namhyung, alexander.shishkin, a.p.zijlstra, mpetlan,
	brouer, daniel, kafai, songliubraving, yhs, andriin,
	quentin.monnet

On Wed, Dec 04, 2019 at 04:29:29PM -0800, David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Wed, 4 Dec 2019 16:23:48 -0800
> 
> > Jokes aside, you may need to provide some reasoning on this one..
> > The recommendation for packaging libbpf from GitHub never had any 
> > clear justification either AFAICR.
> > 
> > I honestly don't see why location matters. bpftool started out on GitHub
> > but we moved it into the tree for... ease of packaging/distribution(?!)
> > Now it's handy to have it in the tree to reuse the uapi headers.
> > 
> > As much as I don't care if we move it (back) out of the tree - having
> > two copies makes no sense to me. As does having it in the libbpf repo.
> > The sync effort is not warranted. User confusion is not warranted.
> 
> Part of this story has to do with how bug fixes propagate via bpf-next
> instead of the bpf tree, as I understand it.
> 
> But yeah it would be nice to have a clear documentation on all of the
> reasoning.
> 
> On the distro side, people seem to not want to use the separate repo.
> If you're supporting enterprise customers you don't just sync with
> upstream, you cherry pick.  When cherry picking gets too painful, you
> sync with upstream possibly eliding upstream new features you don't
> want to appear in your supported product yet.
> 
> I agree with tying bpftool and libbpf into the _resulting_ binary
> distro package, but I'm not totally convinced about separating them
> out of the kernel source tree.

Looks like there is a confusion here.
I'm not proposing to move bpftool out of kernel tree.
The kernel+libbpf+bpftool+selftests already come as single patch set.
bpftool has to stay in the kernel tree otherwise things like skeleton
patchset won't be possible to accomplish without a lot of coordination
between different trees and propagation delays.

I'm proposing to tweak github/libbpf sync script to sync bpftool
sources from kernel into github, so both libbpf and bpftool can be
tested and packaged together.
People are working on adding proper CI to github/libbpf.
bpftool testing will automatically get more mileage out of that effort.

github/libbpf is self contained. It should be built and tested
on many different kernels and build environments (like any user
space package should be). That's an important goal of CI.
When bpftool is part of github/libbpf it will get the same treatment.
I see only advantages and not a single disadvantage of building,
testing, packaging bpftool out of github/libbpf.

To support stable libbpf+bpftool releases we can branch in github and push
fixes into branches. Same CI can test master and stable branches.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  1:09               ` Alexei Starovoitov
@ 2019-12-05  2:10                 ` Jakub Kicinski
  2019-12-05  3:17                   ` Alexei Starovoitov
  0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-12-05  2:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, 4 Dec 2019 17:09:32 -0800, Alexei Starovoitov wrote:
> On Wed, Dec 04, 2019 at 04:23:48PM -0800, Jakub Kicinski wrote:
> > On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:  
> > > > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > > > a day will come when distroes will get up to speed on packaging libbpf,
> > > > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > > > bpftool in the same boat is just more baggage.    
> > > 
> > > Distros should be packaging libbpf and bpftool from single repo on github.
> > > Kernel tree is for packaging kernel.  
> > 
> > Okay, single repo on GitHub:
> > 
> > https://github.com/torvalds/linux  
> 
> and how will you git submodule only libbpf part of kernel github into bcc
> and other projects?

Why does bcc have to submodule libbpf? Is it in a "special
relationship" with libbpf as well? 

dnf/apt install libbpf

Or rather:

dnf/apt install bcc

since BCC's user doesn't care about dependencies. The day distroes
started packaging libbpf and bpftool the game has changed.

> > You also said a few times you don't want to merge fixes into bpf/net.
> > That divergence from kernel development process is worrying.  
> 
> worrying - why? what exactly the concern you see?

First it's still in the tree so it just feels wrong to have the process
fundamentally different for sections of the tree. 

Secondly it's best to stick to the tried and tested processes unless
there's a good and stated reason to diverge.

Now we're neither doing standard user library process nor kernel
process. Both of which must account for stable fixes (Dave explained
the enterprise distro needs in his reply).

> Tying user space release into kernel release and user space process into
> kernel process makes little sense to me.

I'm not saying we can't do user space, but we should pick one.

I have slight preference for kernel process, because it's familiar and
it automatically takes of policy questions, which otherwise consume
developer's time.

Please accept iproute2 as an example of a user space toolset closely
related to the kernel. If kernel release model and process made no
sense in user space, why do iproute2s developers continue to follow it
for years? 

Let's learn from experience :/

> Packaging is different.

There are mostly disadvantages, but the process should be well known.
perf has been packaged for years.

iproute2 sometimes lags behind the upstream, more than perf in my
experience. Also there's rarely a need to update tools closely related
to the kernel without the kernel..

> Compatibility requirements are different.

True.

> CI is different.

Well, hard to argue about things which don't exist :/

> Integration with other projects is different.

IMHO the submodule use case is fading. dnf/apt install, it's just a
normal C library, we've done this for decades.

And there is no submodule need for bpftool, which we are discussing.

> libbpf source code is in the kernel tree only because kernel changes plus
> libbpf changes plus selftests changes come as single patchset. That is really
> the only reason. Packaging scripts, CI scripts, etc should be kept out of
> kernel tree. All that stuff belongs at github/libbpf.
> 
> > None of this makes very much sense to me. We're diverging from well
> > established development practices without as much as a justification.  
> 
> The kernel development process was never used for libbpf.

What do you mean? I've sure as hell sent patches to net with Fixes tags,
S-o-B and all that jazz for libbpf and bpftool.

> Even coding style is different.

Is it? You mean the damn underscores people are making fun of? :/

> I'm puzzled why you think user space
> should be tied to kernel. Everything is so vastly different.

Not what I'm saying, I'm saying either run it as user space project in
separate repos or kernel project.

You're proposing we do both with some syncs and no clear process.

> Some people say that 8 weeks to bump libbpf version is too long.
> Other people say that it's too often.

Those are two conflicting requirements, Alexei, how is breaking apart
from the kernel going to make any difference?

At least kernel gives us this somewhat reasonable 8 week cadence and 
we don't have to ponder whether it should be shorter or longer until
someone presents us with a compelling reason.

> libbpf version numbers != kernel version numbers.

Well, libbpf's numbers are arbitrary, and meaningless. They could as
well be kernel numbers, like they are for bpftool. Or dates.

libbpf doesn't have a roadmap either, it's not really a full-on project
on its own. What's 0.1.0 gonna be? 

Besides stuff lands in libbpf before it hits a major kernel release.
So how are you gonna make libbpf releases independently from kernel
ones? What if a feature gets a last minute revert in the kernel and it's
in libbpf's ABI?

Eh, you're just convincing me it should just stay in the kernel now.

> There is no definition of LTS for libbpf. One day it will be
> and the version of libbpf picked for LTS will likely have
> nothing to do with kernel LTS choices.

Again, libbpf inherits the kernel process so there is LTS process, the
kernel one, and it's taken care of for us by all the existing stable
automation.

By merging everything into -next you're loosing that, with all but a
vague mirage of how there may one day be multiple stable branches..

> libbpf has to run on all kernels. Newer and older. How do you support
> that if libbpf is tied with the kernel?

Say I have built N kernels UM or for a VM, and we have some test
suite: I pull libbpf, build it, run its tests. The only difference
between in tree and out of tree is that "pull libbpf" means pulling
smaller or larger repo. Doesn't matter that match, it's a low --depth
local clone.

Sorry for the long response.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  2:10                 ` Jakub Kicinski
@ 2019-12-05  3:17                   ` Alexei Starovoitov
  2019-12-05  4:26                     ` Jakub Kicinski
  0 siblings, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-05  3:17 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, Dec 04, 2019 at 06:10:28PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Dec 2019 17:09:32 -0800, Alexei Starovoitov wrote:
> > On Wed, Dec 04, 2019 at 04:23:48PM -0800, Jakub Kicinski wrote:
> > > On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:  
> > > > > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > > > > a day will come when distroes will get up to speed on packaging libbpf,
> > > > > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > > > > bpftool in the same boat is just more baggage.    
> > > > 
> > > > Distros should be packaging libbpf and bpftool from single repo on github.
> > > > Kernel tree is for packaging kernel.  
> > > 
> > > Okay, single repo on GitHub:
> > > 
> > > https://github.com/torvalds/linux  
> > 
> > and how will you git submodule only libbpf part of kernel github into bcc
> > and other projects?
> 
> Why does bcc have to submodule libbpf? Is it in a "special
> relationship" with libbpf as well? 
> 
> dnf/apt install libbpf
> 
> Or rather:
> 
> dnf/apt install bcc
> 
> since BCC's user doesn't care about dependencies. The day distroes
> started packaging libbpf and bpftool the game has changed.

have you ever built bcc ? or bpftrace?
I'm not sure how to answer such 'suggestion'.

> Please accept iproute2 as an example of a user space toolset closely
> related to the kernel. If kernel release model and process made no
> sense in user space, why do iproute2s developers continue to follow it
> for years? 

imo iproute2 is an example how things should not be run.
But that's a very different topic.

> > Packaging is different.
> 
> There are mostly disadvantages, but the process should be well known.
> perf has been packaged for years.

perf was initially seen as something that should match kernel one to one.
yet it diverged over years. I think it's a counter example.

> What do you mean? I've sure as hell sent patches to net with Fixes tags

which was complete waste of time for people who were sending these
patches, for maintainers who applied them and for all stables folks
who carried them into kernel stable releases.
Not a single libbpf build was made out of those sources.

> S-o-B and all that jazz for libbpf and bpftool.

Many open source projects use SOB. It's not kernel specific.

> 
> > Even coding style is different.
> 
> Is it? You mean the damn underscores people are making fun of? :/

Are you trolling? Do you understand why __ is there?

> libbpf doesn't have a roadmap either, 

I think you're contrasting that with kernel and saying
that kernel has a roadmap ? What is kernel roadmap?

> it's not really a full-on project
> on its own. What's 0.1.0 gonna be?

whenever this bpf community decides to call it 0.1.0.

> Besides stuff lands in libbpf before it hits a major kernel release.
> So how are you gonna make libbpf releases independently from kernel
> ones? What if a feature gets a last minute revert in the kernel and it's
> in libbpf's ABI?

You mean when kernel gets new feature, then libbpf gets new feature, then
libbpf is released, but then kernel feature is reverted? Obviously we should
avoid making a libbpf release that relies on kernel features that didn't reach
the mainline. Yet there could be plenty of reasons why making libbpf release in
the middle of kernel development cycle makes perfect sense.

Also reaching Linus's tree in rc1 is also not a guarantee of non-revert. Yet we
release libbpf around rc1 because everyone expects bug-fixes after rc1. So it's
an exception that solidifies the rule.

> > libbpf has to run on all kernels. Newer and older. How do you support
> > that if libbpf is tied with the kernel?
> 
> Say I have built N kernels UM or for a VM, and we have some test
> suite: I pull libbpf, build it, run its tests. The only difference
> between in tree and out of tree is that "pull libbpf" means pulling
> smaller or larger repo. Doesn't matter that match, it's a low --depth
> local clone.

The expected CI is:
1. pull-req proposed.
2. CI picks it up, builds, run tests.
3. humans see results and land or reject pull-req.
Now try to think through how CI on top of full kernel tree will
be able to pick just the right commits to start build/test cycle.
Is it going to cherry-pick from patchworks? That would be awesome.
Yet intel 0bot results show that it's easier said than done.
I'm not saying it's not possible. Just complex.
If you have cycles to integrate *-next into kernelci.org, please go ahead.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  3:17                   ` Alexei Starovoitov
@ 2019-12-05  4:26                     ` Jakub Kicinski
  2019-12-05  6:44                       ` Alexei Starovoitov
  0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-12-05  4:26 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, 4 Dec 2019 19:17:20 -0800, Alexei Starovoitov wrote:
> On Wed, Dec 04, 2019 at 06:10:28PM -0800, Jakub Kicinski wrote:
> > On Wed, 4 Dec 2019 17:09:32 -0800, Alexei Starovoitov wrote:  
> > > On Wed, Dec 04, 2019 at 04:23:48PM -0800, Jakub Kicinski wrote:  
> > > > On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:    
> > > > > > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > > > > > a day will come when distroes will get up to speed on packaging libbpf,
> > > > > > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > > > > > bpftool in the same boat is just more baggage.      
> > > > > 
> > > > > Distros should be packaging libbpf and bpftool from single repo on github.
> > > > > Kernel tree is for packaging kernel.    
> > > > 
> > > > Okay, single repo on GitHub:
> > > > 
> > > > https://github.com/torvalds/linux    
> > > 
> > > and how will you git submodule only libbpf part of kernel github into bcc
> > > and other projects?  
> > 
> > Why does bcc have to submodule libbpf? Is it in a "special
> > relationship" with libbpf as well? 
> > 
> > dnf/apt install libbpf
> > 
> > Or rather:
> > 
> > dnf/apt install bcc
> > 
> > since BCC's user doesn't care about dependencies. The day distroes
> > started packaging libbpf and bpftool the game has changed.  
> 
> have you ever built bcc ? or bpftrace?
> I'm not sure how to answer such 'suggestion'.

Perhaps someone else has more patience to explain it - why bcc can't
just use binary libbpf distribution (static lib + headers) and link
against it like it links against other libraries?

> > Please accept iproute2 as an example of a user space toolset closely
> > related to the kernel. If kernel release model and process made no
> > sense in user space, why do iproute2s developers continue to follow it
> > for years?   
> 
> imo iproute2 is an example how things should not be run.
> But that's a very different topic.

Please explain, the topic is how to maintain user space closely related
to the kernel.

Share with us what you dislike about iproute2 so we can fix it. Instead
of adding parts of it to bpftool and then pretending that the API added
to libbpf to facilitate that duplication is some internal bpftool-only
magic which then prevents us from dynamic linking..... 😠

> > > Packaging is different.  
> > 
> > There are mostly disadvantages, but the process should be well known.
> > perf has been packaged for years.  
> 
> perf was initially seen as something that should match kernel one to one.
> yet it diverged over years. I think it's a counter example.
> 
> > What do you mean? I've sure as hell sent patches to net with Fixes tags  
> 
> which was complete waste of time for people who were sending these
> patches, for maintainers who applied them and for all stables folks
> who carried them into kernel stable releases.
> Not a single libbpf build was made out of those sources.

Because libbpf just now entered the distroes, and you suggested the
distroes use the GH repo, so sure now it's wasted work.

IIRC there were bpftool crash fixes which landed in Fedora via stable.

> > > Even coding style is different.  
> > 
> > Is it? You mean the damn underscores people are making fun of? :/  
> 
> Are you trolling? Do you understand why __ is there?

Not the point. Tell me how the coding style is different. The
underscores is the only thing I could think of that's not common 
in the kernel.

> > libbpf doesn't have a roadmap either,   
> 
> I think you're contrasting that with kernel and saying
> that kernel has a roadmap ? What is kernel roadmap?

Kernel road map is the same as libbpf's road map.

> > it's not really a full-on project
> > on its own. What's 0.1.0 gonna be?  
> 
> whenever this bpf community decides to call it 0.1.0.
>
> > Besides stuff lands in libbpf before it hits a major kernel release.
> > So how are you gonna make libbpf releases independently from kernel
> > ones? What if a feature gets a last minute revert in the kernel and it's
> > in libbpf's ABI?  
> 
> You mean when kernel gets new feature, then libbpf gets new feature, then
> libbpf is released, but then kernel feature is reverted? Obviously we should
> avoid making a libbpf release that relies on kernel features that didn't reach
> the mainline. Yet there could be plenty of reasons why making libbpf release in
> the middle of kernel development cycle makes perfect sense.

But master of libbpf must have all features to test the kernel with,
right? So how do we branch of a release in the middle? That's only
possible if kernel cycle happens to not have had any features that
required libbpf yet?

Or are you thinking 3 tier branching where we'd branch off libbpf
release, say 2.6.0 that corresponds to kernel X, but it wouldn't be a
stable-only release, and we can still backport features added in kernel
X + 1 cycle, features which don't require kernel support, and release
libbpf 2.7.0?

Could work but it'd get tricky, cause if we want to break ABI we'd
actually need 4 tiers. ABI compat, kernel version, feature version,
stable version.

> Also reaching Linus's tree in rc1 is also not a guarantee of non-revert. Yet we
> release libbpf around rc1 because everyone expects bug-fixes after rc1. 

I consider current process to be broken. Hopefully we can improve it.

> So it's an exception that solidifies the rule.
>
> > > libbpf has to run on all kernels. Newer and older. How do you support
> > > that if libbpf is tied with the kernel?  
> > 
> > Say I have built N kernels UM or for a VM, and we have some test
> > suite: I pull libbpf, build it, run its tests. The only difference
> > between in tree and out of tree is that "pull libbpf" means pulling
> > smaller or larger repo. Doesn't matter that match, it's a low --depth
> > local clone.  
> 
> The expected CI is:
> 1. pull-req proposed.
> 2. CI picks it up, builds, run tests.
> 3. humans see results and land or reject pull-req.
> Now try to think through how CI on top of full kernel tree will
> be able to pick just the right commits to start build/test cycle.
> Is it going to cherry-pick from patchworks? That would be awesome.
> Yet intel 0bot results show that it's easier said than done.
> I'm not saying it's not possible. Just complex.
> If you have cycles to integrate *-next into kernelci.org, please go ahead.

Yes, it is very complex, I know. I've been hacking on something along
those lines for the last few weeks. Hopefully I'll have results at some
point..

First stab is just doing build testing, checkpatch, verify tags etc.
Uploading to patchwork, and sending an email if there were failures.

Even that's not easy as a weekend/evening task :( And it requires a lot
of manual inspection upfront before it's unleashed on the ML, because it
will catch a lot of stupid little stuff and a lot of people will get
grumpy.

We need to modernize the process across the board. I don't think having
zombie read-only repos on GitHub will give contributors confidence so
it's not a step in right direction. We should start from the hard
problem, that is the CI itself.

The problem of correlating user space and kernel patches will have to 
be solved for netdev, because netdev tests depend on iproute2.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  4:26                     ` Jakub Kicinski
@ 2019-12-05  6:44                       ` Alexei Starovoitov
  0 siblings, 0 replies; 29+ messages in thread
From: Alexei Starovoitov @ 2019-12-05  6:44 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrii Nakryiko, Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Jesper Dangaard Brouer, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, Andrii Nakryiko, Quentin Monnet

On Wed, Dec 04, 2019 at 08:26:38PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Dec 2019 19:17:20 -0800, Alexei Starovoitov wrote:
> > On Wed, Dec 04, 2019 at 06:10:28PM -0800, Jakub Kicinski wrote:
> > > On Wed, 4 Dec 2019 17:09:32 -0800, Alexei Starovoitov wrote:  
> > > > On Wed, Dec 04, 2019 at 04:23:48PM -0800, Jakub Kicinski wrote:  
> > > > > On Wed, 4 Dec 2019 15:39:49 -0800, Alexei Starovoitov wrote:    
> > > > > > > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > > > > > > a day will come when distroes will get up to speed on packaging libbpf,
> > > > > > > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > > > > > > bpftool in the same boat is just more baggage.      
> > > > > > 
> > > > > > Distros should be packaging libbpf and bpftool from single repo on github.
> > > > > > Kernel tree is for packaging kernel.    
> > > > > 
> > > > > Okay, single repo on GitHub:
> > > > > 
> > > > > https://github.com/torvalds/linux    
> > > > 
> > > > and how will you git submodule only libbpf part of kernel github into bcc
> > > > and other projects?  
> > > 
> > > Why does bcc have to submodule libbpf? Is it in a "special
> > > relationship" with libbpf as well? 
> > > 
> > > dnf/apt install libbpf
> > > 
> > > Or rather:
> > > 
> > > dnf/apt install bcc
> > > 
> > > since BCC's user doesn't care about dependencies. The day distroes
> > > started packaging libbpf and bpftool the game has changed.  
> > 
> > have you ever built bcc ? or bpftrace?
> > I'm not sure how to answer such 'suggestion'.
> 
> Perhaps someone else has more patience to explain it - why bcc can't
> just use binary libbpf distribution (static lib + headers) and link
> against it like it links against other libraries?

why systemd considered using libbpf as submodule ?
When project like bcc needs to run on different distros and
different versions of the same distro such project cannot force users
to upgrade core libraries to get features that project might need.
libbpf is far from stability of libc, libmnl, libelf.
There are mature libraries and actively developed libraries.
libbpf is at its infancy stage. We're trying hard to be stable,
but accumulated baggage is already huge and it is slowing us down.
systemd is considering to use libbpf without being submodule too,
but it's mainly driven by systemd's CI limitations and nothing else.

> Share with us what you dislike about iproute2 so we can fix it. 

let's start with 24-hr review cycle that we're trying hard to keep in bpf/net
trees. Can you help with 24-hr review in iproute2 ?

> Because libbpf just now entered the distroes, and you suggested the
> distroes use the GH repo, so sure now it's wasted work.

You got the timeline wrong. GH repo was done before some distros started
packaging libbpf. libbpf is still not available in centos/rhel afaik.
But bcc and bpftrace need to work there. Answer? submodules...

> > Are you trolling? Do you understand why __ is there?
> 
> Not the point. Tell me how the coding style is different. The
> underscores is the only thing I could think of that's not common 
> in the kernel.

we don't actively enforce xmas tree :)

> But master of libbpf must have all features to test the kernel with,
> right? So how do we branch of a release in the middle? That's only
> possible if kernel cycle happens to not have had any features that
> required libbpf yet?

libbpf rate of changes is higher than bpf bits of the kernel.
Amount of patches per week is higher as well.
Something like skeleton work might warrant independent release just
to get into the hands of people faster. Why wait 8 weeks for kernel
release? No reason.

> Or are you thinking 3 tier branching where we'd branch off libbpf
> release, say 2.6.0 that corresponds to kernel X, 

libbpf does not correspond to kernel.
All libbpf releases must work with any kernel.

> but it wouldn't be a
> stable-only release, and we can still backport features added in kernel
> X + 1 cycle, features which don't require kernel support, and release
> libbpf 2.7.0?

Say libbpf 0.1 was developed during kernel 5.4.
We can 'backport' all libbpf features from kernel tree 5.5 into libbpf 0.1+
and that libbpf 0.1+ must work with kernels 5.4 and 5.5.
That is what existing github/libbpf CI is testing.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-04 23:39           ` Alexei Starovoitov
  2019-12-05  0:23             ` Jakub Kicinski
@ 2019-12-05  8:35             ` Jesper Dangaard Brouer
  2019-12-05 12:09               ` Michal Rostecki
  1 sibling, 1 reply; 29+ messages in thread
From: Jesper Dangaard Brouer @ 2019-12-05  8:35 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jakub Kicinski, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, Quentin Monnet, brouer, Laura Abbott

On Wed, 4 Dec 2019 15:39:49 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Wed, Dec 04, 2019 at 01:54:05PM -0800, Jakub Kicinski wrote:
> > On Wed, 4 Dec 2019 13:16:13 -0800, Andrii Nakryiko wrote:  
> > > I wonder what big advantage having bpftool in libbpf's Github repo
> > > brings, actually? The reason we need libbpf on github is to allow
> > > other projects like pahole to be able to use libbpf from submodule.
> > > There is no such need for bpftool.
> > > 
> > > I agree about preference to release them in sync, but that could be
> > > easily done by releasing based on corresponding commits in github's
> > > libbpf repo and kernel repo. bpftool doesn't have to physically live
> > > next to libbpf on Github, does it?  
> > 
> > +1

I don't see any advantage of having bpftool in libbpf's GitHub repo.

As Jakub mention we have seen bpftool crash fixes, which would be
painful/annoying to maintain fixes for in the libbpf GitHub repo.

As Andrii also points out, it requires more work, as GitHub libbpf have
to maintain a separate Makefile for bpftool.


> > > Calling github repo a "mirror" is incorrect. It's not a 1:1 copy of
> > > files. We have a completely separate Makefile for libbpf, and we have
> > > a bunch of stuff we had to re-implement to detach libbpf code from
> > > kernel's non-UAPI headers. Doing this for bpftool as well seems like
> > > just more maintenance. Keeping github's Makefile in sync with kernel's
> > > Makefile (for libbpf) is PITA, I'd rather avoid similar pains for
> > > bpftool without a really good reason.  
> > 
> > Agreed. Having libbpf on GH is definitely useful today, but one can hope
> > a day will come when distroes will get up to speed on packaging libbpf,
> > and perhaps we can retire it? Maybe 2, 3 years from now? Putting
> > bpftool in the same boat is just more baggage.  
> 
> Distros should be packaging libbpf and bpftool from single repo on github.
> Kernel tree is for packaging kernel.

I don't think that is a good idea.  You are creating double work and
wasting distro developers time.  Let me explain: 

1. First of all, GitHub libbpf does not have a stable branches (which
makes sense, given this is a read-only clone of kernel tree). Thus,
distro developers have to maintain that themselves, in their internal
package tree (that is based on GitHub libbpf).

2. Kernel BPF changes usually require updates to libbpf, as selftests
uses libbpf.  Thus, the distro kernel backporter is already required to
backport libbpf parts.

This is double work, the code changes to libbpf are now maintained in
two places for the distro.


The disadvantage for distros to package libbpf (+ bpftool and perf) off
their distro kernel tree is that a fix to libbpf, requires rolling a
new kernel minor release.  The solution to that is simply that distro
package for libbpf have a separate (RPM) spec file, with own
versioning, which sources points to distro kernel tree.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically
  2019-12-05  8:35             ` Jesper Dangaard Brouer
@ 2019-12-05 12:09               ` Michal Rostecki
  0 siblings, 0 replies; 29+ messages in thread
From: Michal Rostecki @ 2019-12-05 12:09 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, Jakub Kicinski, Andrii Nakryiko,
	Toke Høiland-Jørgensen, Jiri Olsa,
	Arnaldo Carvalho de Melo, lkml, Networking, bpf, Ingo Molnar,
	Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Andrii Nakryiko, Quentin Monnet, Laura Abbott

On Thu, Dec 05, 2019 at 09:35:48AM +0100, Jesper Dangaard Brouer wrote:
> I don't think that is a good idea.  You are creating double work and
> wasting distro developers time.  Let me explain: 
> 
> 1. First of all, GitHub libbpf does not have a stable branches (which
> makes sense, given this is a read-only clone of kernel tree). Thus,
> distro developers have to maintain that themselves, in their internal
> package tree (that is based on GitHub libbpf).
> 
> 2. Kernel BPF changes usually require updates to libbpf, as selftests
> uses libbpf.  Thus, the distro kernel backporter is already required to
> backport libbpf parts.
> 
> This is double work, the code changes to libbpf are now maintained in
> two places for the distro.

I totally agree with Jesper here.

I don't know how the situatiom with packaging libbpf and bpftool looks
like in Fedora/Centos/RHEL now, but in openSUSE we would like to build
both of them from the kernel source - use kernel-source package as a
requirement and use the kernel tree from /usr/src/linux to build those.
We do that for bpftool and perf currently.

So far we are building bpftool and perf without libbpf being dynamically
linked, so there is no dependency between those packages, although we
would like to change it as soon as we find a consensus on this series of
patches.

> The disadvantage for distros to package libbpf (+ bpftool and perf) off
> their distro kernel tree is that a fix to libbpf, requires rolling a
> new kernel minor release.  The solution to that is simply that distro
> package for libbpf have a separate (RPM) spec file, with own
> versioning, which sources points to distro kernel tree.

That's a great idea. So far, we are using the same version for kernel,
bfptool and perf, but all of these have separate RPM specs, so we can do
that.

Michal

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-12-05 12:09 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 13:18 [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Jiri Olsa
2019-12-02 13:18 ` [PATCH 1/6] perf tools: Allow to specify libbpf install directory Jiri Olsa
2019-12-02 13:18 ` [PATCH 2/6] bpftool: Allow to link libbpf dynamically Jiri Olsa
2019-12-02 13:18 ` [PATCH 3/6] bpftool: Rename BPF_DIR Makefile variable to LIBBPF_SRC_DIR Jiri Olsa
2019-12-02 13:18 ` [PATCH 4/6] bpftool: Rename LIBBPF_OUTPUT Makefile variable to LIBBPF_BUILD_OUTPUT Jiri Olsa
2019-12-02 13:18 ` [PATCH 5/6] bpftool: Rename LIBBPF_PATH Makefile variable to LIBBPF_BUILD_PATH Jiri Olsa
2019-12-02 13:18 ` [PATCH 6/6] selftests, bpftool: Add build test for libbpf dynamic linking Jiri Olsa
2019-12-02 15:38   ` Quentin Monnet
2019-12-02 19:41 ` [PATCHv4 0/6] perf/bpftool: Allow to link libbpf dynamically Andrii Nakryiko
2019-12-02 21:15   ` Toke Høiland-Jørgensen
2019-12-04  5:52     ` Alexei Starovoitov
2019-12-04  9:01       ` Jiri Olsa
2019-12-04 10:57       ` Toke Høiland-Jørgensen
2019-12-04 17:39         ` Alexei Starovoitov
2019-12-04 18:27           ` Daniel Borkmann
2019-12-04 20:22             ` Toke Høiland-Jørgensen
2019-12-04 21:16       ` Andrii Nakryiko
2019-12-04 21:54         ` Jakub Kicinski
2019-12-04 23:39           ` Alexei Starovoitov
2019-12-05  0:23             ` Jakub Kicinski
2019-12-05  0:29               ` David Miller
2019-12-05  1:25                 ` Alexei Starovoitov
2019-12-05  1:09               ` Alexei Starovoitov
2019-12-05  2:10                 ` Jakub Kicinski
2019-12-05  3:17                   ` Alexei Starovoitov
2019-12-05  4:26                     ` Jakub Kicinski
2019-12-05  6:44                       ` Alexei Starovoitov
2019-12-05  8:35             ` Jesper Dangaard Brouer
2019-12-05 12:09               ` Michal Rostecki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).