[SRU][BIONIC][PATCH v2 0/2] add bpftool to linux-tools-common

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][BIONIC][PATCH v2 0/2] add bpftool to linux-tools-common

Quentin Monnet
BugLink: https://bugs.launchpad.net/bugs/1774815

[Impact]

bpftool is a debugging and introspection tool for BPF elements,
developed by the BPF kernel community. It is essential to list and dump
BPF programs and maps loaded on the system. Its sources are located in
the kernel repository, and because it is not packaged, administrators
willing to use bpftool must download the whole kernel sources, compile
and install the utility.

[Fix]

Adding bpftool to linux-tools and linux-tools-common packages makes it
easily accessible. These packages are already used to provide other
tools located in the kernel repository, such as perf.

Because the bpftool version provided with kernel 4.15 does not build
properly (API changed at some point in bfd.h from binutils-dev),
backport a patch from 4.16 to fix the calls to libbfd's disassembler.

[Testcase]

A test linux package was successfully built, at:

https://launchpad.net/~qmonnet/+archive/ubuntu/ppa-linux-bpftool

(Built with:
  do_zfs=false
  do_dkms_nvidia=false
  do_dkms_vbox=false
  skipabi=true
  skipmodule=true
  skipretpoline=true)

Packages linux-tools-$(uname -r) and linux-tools-common can be built
with "debian/rules binary", and contain bpftool's binary and related
files (redirection script, bpftool manual pages, bash completion),
respectively.

[Regression Potential]

Low, as far as I can tell:

- The backported patch touches only bpftool and one feature in
  tools/build/feature (only used with bpftool), all of it user space
  tools not used for anything other than bpftool itself.

- bpftool packaging does not change the way other tools are packaged
  (apart from creating $(toolsman)/man8 a few lines earlier), and should
  have no impact on the packaging of other tools. One dependency is
  added to Build-Depends-Indep, none is removed.

---
First version of this set was posted for bionic, but before the patch
for packaging bpftool had been submitted and accepted to the development
branch (currently eoan) [0]. That same patch is now upstream in
eoan/master-next as commit 8579afd20b0c.

[0] https://lists.ubuntu.com/archives/kernel-team/2019-July/102462.html

Changes in v2:
- Patch has been applied to eoan.
- Switch from python-docutils to python3-docutils for the dependency
  providing rst2man for manual pages.
- Set CROSS_COMPILE when building bpftool.

Quentin Monnet (2):
  tools/bpftool: fix bpftool build with bintutils >= 2.9
  UBUNTU: [Debian] package bpftool in linux-tools-common

 debian.master/control.stub.in                 |  1 +
 debian.master/rules.d/amd64.mk                |  1 +
 debian.master/rules.d/arm64.mk                |  1 +
 debian.master/rules.d/armhf.mk                |  1 +
 debian.master/rules.d/i386.mk                 |  1 +
 debian.master/rules.d/ppc64el.mk              |  1 +
 debian.master/rules.d/s390x.mk                |  1 +
 debian/rules                                  |  2 +-
 debian/rules.d/1-maintainer.mk                |  1 +
 debian/rules.d/2-binary-arch.mk               |  9 ++++++
 debian/rules.d/3-binary-indep.mk              | 12 +++++++-
 tools/bpf/Makefile                            | 29 +++++++++++++++++++
 tools/bpf/bpf_jit_disasm.c                    |  7 +++++
 tools/bpf/bpftool/Makefile                    | 24 +++++++++++++++
 tools/bpf/bpftool/jit_disasm.c                |  7 +++++
 tools/build/feature/Makefile                  |  4 +++
 .../feature/test-disassembler-four-args.c     | 15 ++++++++++
 17 files changed, 115 insertions(+), 2 deletions(-)
 create mode 100644 tools/build/feature/test-disassembler-four-args.c

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][BIONIC][PATCH v2 1/2] tools/bpftool: fix bpftool build with bintutils >= 2.9

Quentin Monnet
From: Roman Gushchin <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1774815

Bpftool build is broken with binutils version 2.29 and later.
The cause is commit 003ca0fd2286 ("Refactor disassembler selection")
in the binutils repo, which changed the disassembler() function
signature.

Fix this by adding a new "feature" to the tools/build/features
infrastructure and make it responsible for decision which
disassembler() function signature to use.

Signed-off-by: Roman Gushchin <[hidden email]>
Cc: Jakub Kicinski <[hidden email]>
Cc: Alexei Starovoitov <[hidden email]>
Cc: Daniel Borkmann <[hidden email]>
Acked-by: Jakub Kicinski <[hidden email]>
Signed-off-by: Daniel Borkmann <[hidden email]>
(backported from commit fb982666e380c1632a74495b68b3c33a66e76430)
Signed-off-by: Quentin Monnet <[hidden email]>
---
 tools/bpf/Makefile                            | 29 +++++++++++++++++++
 tools/bpf/bpf_jit_disasm.c                    |  7 +++++
 tools/bpf/bpftool/Makefile                    | 24 +++++++++++++++
 tools/bpf/bpftool/jit_disasm.c                |  7 +++++
 tools/build/feature/Makefile                  |  4 +++
 .../feature/test-disassembler-four-args.c     | 15 ++++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 tools/build/feature/test-disassembler-four-args.c

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 07a6697466ef..c8ec0ae16bf0 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -9,6 +9,35 @@ MAKE = make
 CFLAGS += -Wall -O2
 CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
 
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
+FEATURE_USER = .bpf
+FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_DISPLAY = libbfd disassembler-four-args
+
+check_feat := 1
+NON_CHECK_FEAT_TARGETS := clean bpftool_clean
+ifdef MAKECMDGOALS
+ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
+  check_feat := 0
+endif
+endif
+
+ifeq ($(check_feat),1)
+ifeq ($(FEATURES_DUMP),)
+include $(srctree)/tools/build/Makefile.feature
+else
+include $(FEATURES_DUMP)
+endif
+endif
+
+ifeq ($(feature-disassembler-four-args), 1)
+CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
+endif
+
 %.yacc.c: %.y
  $(YACC) -o $@ -d $<
 
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index 75bf526a0168..30044bc4f389 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -72,7 +72,14 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
 
  disassemble_init_for_target(&info);
 
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+ disassemble = disassembler(info.arch,
+   bfd_big_endian(bfdf),
+   info.mach,
+   bfdf);
+#else
  disassemble = disassembler(bfdf);
+#endif
  assert(disassemble);
 
  do {
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index ec3052c0b004..9e433c3f8bbe 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -55,6 +55,30 @@ CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
 CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
 LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
 
+FEATURE_USER = .bpftool
+FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_DISPLAY = libbfd disassembler-four-args
+
+check_feat := 1
+NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
+ifdef MAKECMDGOALS
+ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
+  check_feat := 0
+endif
+endif
+
+ifeq ($(check_feat),1)
+ifeq ($(FEATURES_DUMP),)
+include $(srctree)/tools/build/Makefile.feature
+else
+include $(FEATURES_DUMP)
+endif
+endif
+
+ifeq ($(feature-disassembler-four-args), 1)
+CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
+endif
+
 include $(wildcard *.d)
 
 all: $(OUTPUT)bpftool
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 86376f923d28..034d91284ae5 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -110,7 +110,14 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
 
  disassemble_init_for_target(&info);
 
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+ disassemble = disassembler(info.arch,
+   bfd_big_endian(bfdf),
+   info.mach,
+   bfdf);
+#else
  disassemble = disassembler(bfdf);
+#endif
  assert(disassemble);
 
  if (json_output)
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 15071cb176bc..3a0de97284ee 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -13,6 +13,7 @@ FILES=                                          \
          test-hello.bin                         \
          test-libaudit.bin                      \
          test-libbfd.bin                        \
+         test-disassembler-four-args.bin        \
          test-libbfd-liberty.bin                \
          test-libbfd-liberty-z.bin              \
          test-cplus-demangle.bin                \
@@ -188,6 +189,9 @@ $(OUTPUT)test-libpython-version.bin:
 $(OUTPUT)test-libbfd.bin:
  $(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
 
+$(OUTPUT)test-disassembler-four-args.bin:
+ $(BUILD) -lbfd -lopcodes
+
 $(OUTPUT)test-libbfd-liberty.bin:
  $(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
 
diff --git a/tools/build/feature/test-disassembler-four-args.c b/tools/build/feature/test-disassembler-four-args.c
new file mode 100644
index 000000000000..45ce65cfddf0
--- /dev/null
+++ b/tools/build/feature/test-disassembler-four-args.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <bfd.h>
+#include <dis-asm.h>
+
+int main(void)
+{
+ bfd *abfd = bfd_openr(NULL, NULL);
+
+ disassembler(bfd_get_arch(abfd),
+     bfd_big_endian(abfd),
+     bfd_get_mach(abfd),
+     abfd);
+
+ return 0;
+}
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][BIONIC][PATCH v2 2/2] UBUNTU: [Debian] package bpftool in linux-tools-common

Quentin Monnet
In reply to this post by Quentin Monnet
BugLink: https://bugs.launchpad.net/bugs/1774815

bpftool is a debugging and introspection tool for BPF elements,
developed by the BPF kernel community. Its source code is located in the
kernel repository, at tools/bpf/bpftool. Package it in linux-tools and
linux-tools-common.

Along the binary, package manual pages and bash completion file.

bpftool itself is installed under /usr/sbin/, to be consistent with its
Makefile.

Dependency python-docutils is added to Build-Depends-Indep, in order to
provide rst2man which is necessary to build bpftool's manual pages.

Signed-off-by: Quentin Monnet <[hidden email]>
---
 debian.master/control.stub.in    |  1 +
 debian.master/rules.d/amd64.mk   |  1 +
 debian.master/rules.d/arm64.mk   |  1 +
 debian.master/rules.d/armhf.mk   |  1 +
 debian.master/rules.d/i386.mk    |  1 +
 debian.master/rules.d/ppc64el.mk |  1 +
 debian.master/rules.d/s390x.mk   |  1 +
 debian/rules                     |  2 +-
 debian/rules.d/1-maintainer.mk   |  1 +
 debian/rules.d/2-binary-arch.mk  |  9 +++++++++
 debian/rules.d/3-binary-indep.mk | 12 +++++++++++-
 11 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/debian.master/control.stub.in b/debian.master/control.stub.in
index 0afc7e6219a7..94b79fa780c5 100644
--- a/debian.master/control.stub.in
+++ b/debian.master/control.stub.in
@@ -48,6 +48,7 @@ Build-Depends-Indep:
  asciidoc <!stage1>,
  python-sphinx <!stage1>,
  python-sphinx-rtd-theme <!stage1>,
+ python3-docutils <!stage1>,
 Vcs-Git: git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/bionic
 XS-Testsuite: autopkgtest
 #XS-Testsuite-Depends: gcc-4.7 binutils
diff --git a/debian.master/rules.d/amd64.mk b/debian.master/rules.d/amd64.mk
index 4046adf3ebb3..3d248c270232 100644
--- a/debian.master/rules.d/amd64.mk
+++ b/debian.master/rules.d/amd64.mk
@@ -14,6 +14,7 @@ do_tools_usbip  = true
 do_tools_cpupower = true
 do_tools_perf   = true
 do_tools_perf_jvmti = true
+do_tools_bpftool = true
 do_tools_x86 = true
 do_tools_hyperv = true
 do_tools_host = true
diff --git a/debian.master/rules.d/arm64.mk b/debian.master/rules.d/arm64.mk
index c7e9a9c1e199..c49d25e752df 100644
--- a/debian.master/rules.d/arm64.mk
+++ b/debian.master/rules.d/arm64.mk
@@ -16,6 +16,7 @@ do_tools_usbip  = true
 do_tools_cpupower = true
 do_tools_perf   = true
 do_tools_perf_jvmti = true
+do_tools_bpftool = true
 
 do_dtbs = true
 do_zfs = true
diff --git a/debian.master/rules.d/armhf.mk b/debian.master/rules.d/armhf.mk
index d516ae13454c..085e8acdc8aa 100644
--- a/debian.master/rules.d/armhf.mk
+++ b/debian.master/rules.d/armhf.mk
@@ -14,5 +14,6 @@ do_tools_usbip  = true
 do_tools_cpupower = true
 do_tools_perf = true
 do_tools_perf_jvmti = true
+do_tools_bpftool = true
 
 do_dtbs = true
diff --git a/debian.master/rules.d/i386.mk b/debian.master/rules.d/i386.mk
index a422f9feba6c..89ad2a6c26cf 100644
--- a/debian.master/rules.d/i386.mk
+++ b/debian.master/rules.d/i386.mk
@@ -13,6 +13,7 @@ do_tools_usbip  = true
 do_tools_cpupower = true
 do_tools_perf   = true
 do_tools_perf_jvmti = true
+do_tools_bpftool = true
 do_tools_x86 = true
 do_tools_hyperv = true
 do_extras_package = true
diff --git a/debian.master/rules.d/ppc64el.mk b/debian.master/rules.d/ppc64el.mk
index c540bacaa166..3934ef482a57 100644
--- a/debian.master/rules.d/ppc64el.mk
+++ b/debian.master/rules.d/ppc64el.mk
@@ -15,6 +15,7 @@ do_tools_usbip    = true
 do_tools_cpupower = true
 do_tools_perf  = true
 do_tools_perf_jvmti = true
+do_tools_bpftool  = true
 
 #do_flavour_image_package = false
 do_zfs = true
diff --git a/debian.master/rules.d/s390x.mk b/debian.master/rules.d/s390x.mk
index dad66b1a674f..cbc188507892 100644
--- a/debian.master/rules.d/s390x.mk
+++ b/debian.master/rules.d/s390x.mk
@@ -16,5 +16,6 @@ do_tools_usbip    = true
 do_tools_cpupower = true
 do_tools_perf     = true
 do_tools_perf_jvmti = true
+do_tools_bpftool  = true
 
 do_zfs = true
diff --git a/debian/rules b/debian/rules
index 04a71812802c..49f59bca2f23 100755
--- a/debian/rules
+++ b/debian/rules
@@ -33,7 +33,7 @@ include $(DROOT)/rules.d/0-common-vars.mk
 # Maintainer targets
 include $(DROOT)/rules.d/1-maintainer.mk
 
-do_linux_tools=$(sort $(filter-out false,$(do_tools_usbip) $(do_tools_cpupower) $(do_tools_perf) $(do_tools_x86)))
+do_linux_tools=$(sort $(filter-out false,$(do_tools_usbip) $(do_tools_cpupower) $(do_tools_perf) $(do_tools_bpftool) $(do_tools_x86)))
 do_cloud_tools=$(sort $(filter-out false,$(do_tools_hyperv)))
 do_tools_common?=true
 do_tools_host?=false
diff --git a/debian/rules.d/1-maintainer.mk b/debian/rules.d/1-maintainer.mk
index 8144be29523e..e515965d6f73 100644
--- a/debian/rules.d/1-maintainer.mk
+++ b/debian/rules.d/1-maintainer.mk
@@ -84,6 +84,7 @@ printenv:
  @echo "do_linux_tools            = $(do_linux_tools)"
  @echo " do_tools_cpupower         = $(do_tools_cpupower)"
  @echo " do_tools_perf             = $(do_tools_perf)"
+ @echo " do_tools_bpftool          = $(do_tools_bpftool)"
  @echo " do_tools_x86              = $(do_tools_x86)"
  @echo " do_tools_host             = $(do_tools_host)"
  @echo "do_cloud_tools            = $(do_cloud_tools)"
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
index 3e4c81a88fc7..14435bf86d85 100644
--- a/debian/rules.d/2-binary-arch.mk
+++ b/debian/rules.d/2-binary-arch.mk
@@ -375,6 +375,9 @@ ifeq ($(do_tools_perf_jvmti),true)
  $(LN) ../../$(src_pkg_name)-tools-$(abi_release)/libperf-jvmti.so $(toolspkgdir)/usr/lib/linux-tools/$(abi_release)-$*
 endif
 endif
+ifeq ($(do_tools_bpftool),true)
+ $(LN) ../../$(src_pkg_name)-tools-$(abi_release)/bpftool $(toolspkgdir)/usr/lib/linux-tools/$(abi_release)-$*
+endif
 ifeq ($(do_tools_x86),true)
  $(LN) ../../$(src_pkg_name)-tools-$(abi_release)/x86_energy_perf_policy $(toolspkgdir)/usr/lib/linux-tools/$(abi_release)-$*
  $(LN) ../../$(src_pkg_name)-tools-$(abi_release)/turbostat $(toolspkgdir)/usr/lib/linux-tools/$(abi_release)-$*
@@ -650,6 +653,9 @@ ifeq ($(do_tools_perf),true)
  cd $(builddirpa)/tools/perf && \
  $(kmake) prefix=/usr HAVE_NO_LIBBFD=1 HAVE_CPLUS_DEMANGLE_SUPPORT=1 CROSS_COMPILE=$(CROSS_COMPILE) NO_LIBPYTHON=1 NO_LIBPERL=1 PYTHON=python2.7
 endif
+ifeq ($(do_tools_bpftool),true)
+ $(kmake) CROSS_COMPILE=$(CROSS_COMPILE) -C $(builddirpa)/tools/bpf/bpftool
+endif
 ifeq ($(do_tools_x86),true)
  cd $(builddirpa)/tools/power/x86/x86_energy_perf_policy && make CROSS_COMPILE=$(CROSS_COMPILE)
  cd $(builddirpa)/tools/power/x86/turbostat && make CROSS_COMPILE=$(CROSS_COMPILE)
@@ -694,6 +700,9 @@ ifeq ($(do_tools_perf_jvmti),true)
  install -m755 $(builddirpa)/tools/perf/libperf-jvmti.so $(toolspkgdir)/usr/lib/$(src_pkg_name)-tools-$(abi_release)
 endif
 endif
+ifeq ($(do_tools_bpftool),true)
+ install -m755 $(builddirpa)/tools/bpf/bpftool/bpftool $(toolspkgdir)/usr/lib/$(src_pkg_name)-tools-$(abi_release)
+endif
 ifeq ($(do_tools_x86),true)
  install -m755 $(builddirpa)/tools/power/x86/x86_energy_perf_policy/x86_energy_perf_policy \
  $(toolspkgdir)/usr/lib/$(src_pkg_name)-tools-$(abi_release)
diff --git a/debian/rules.d/3-binary-indep.mk b/debian/rules.d/3-binary-indep.mk
index b27275685612..e5c6bc0c2c8a 100644
--- a/debian/rules.d/3-binary-indep.mk
+++ b/debian/rules.d/3-binary-indep.mk
@@ -84,6 +84,7 @@ install-tools: toolspkg = $(tools_common_pkg_name)
 install-tools: toolsbin = $(CURDIR)/debian/$(toolspkg)/usr/bin
 install-tools: toolssbin = $(CURDIR)/debian/$(toolspkg)/usr/sbin
 install-tools: toolsman = $(CURDIR)/debian/$(toolspkg)/usr/share/man
+install-tools: toolsbashcomp = $(CURDIR)/debian/$(toolspkg)/usr/share/bash-completion/completions
 install-tools: hosttoolspkg = $(hosttools_pkg_name)
 install-tools: hosttoolsbin = $(CURDIR)/debian/$(hosttoolspkg)/usr/bin
 install-tools: hosttoolsman = $(CURDIR)/debian/$(hosttoolspkg)/usr/share/man
@@ -102,7 +103,10 @@ ifeq ($(do_tools_common),true)
  rsync -a tools/ $(builddir)/tools/tools/
 
  install -d $(toolsbin)
+ install -d $(toolssbin)
  install -d $(toolsman)/man1
+ install -d $(toolsman)/man8
+ install -d $(toolsbashcomp)
 
  install -m755 debian/tools/generic $(toolsbin)/usbip
  install -m755 debian/tools/generic $(toolsbin)/usbipd
@@ -113,6 +117,13 @@ ifeq ($(do_tools_common),true)
 
  install -m755 debian/tools/generic $(toolsbin)/perf
 
+ install -m755 debian/tools/generic $(toolssbin)/bpftool
+ make -C $(builddir)/tools/tools/bpf/bpftool doc
+ install -m644 $(builddir)/tools/tools/bpf/bpftool/Documentation/*.8 \
+ $(toolsman)/man8
+ install -m644 $(builddir)/tools/tools/bpf/bpftool/bash-completion/bpftool \
+ $(toolsbashcomp)
+
  install -m755 debian/tools/generic $(toolsbin)/x86_energy_perf_policy
  install -m755 debian/tools/generic $(toolsbin)/turbostat
 
@@ -120,7 +131,6 @@ ifeq ($(do_tools_common),true)
  install -m644 $(builddir)/tools/tools/perf/Documentation/*.1 \
  $(toolsman)/man1
 
- install -d $(toolsman)/man8
  install -m644 $(CURDIR)/tools/power/x86/x86_energy_perf_policy/*.8 $(toolsman)/man8
  install -m644 $(CURDIR)/tools/power/x86/turbostat/*.8 $(toolsman)/man8
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NAK: [SRU][BIONIC][PATCH v2 0/2] add bpftool to linux-tools-common

Seth Forshee
In reply to this post by Quentin Monnet
On Mon, Sep 09, 2019 at 12:39:21PM +0100, Quentin Monnet wrote:

> BugLink: https://bugs.launchpad.net/bugs/1774815
>
> [Impact]
>
> bpftool is a debugging and introspection tool for BPF elements,
> developed by the BPF kernel community. It is essential to list and dump
> BPF programs and maps loaded on the system. Its sources are located in
> the kernel repository, and because it is not packaged, administrators
> willing to use bpftool must download the whole kernel sources, compile
> and install the utility.
>
> [Fix]
>
> Adding bpftool to linux-tools and linux-tools-common packages makes it
> easily accessible. These packages are already used to provide other
> tools located in the kernel repository, such as perf.
>
> Because the bpftool version provided with kernel 4.15 does not build
> properly (API changed at some point in bfd.h from binutils-dev),
> backport a patch from 4.16 to fix the calls to libbfd's disassembler.

What should be done instead is to remove binutils-dev from the package
build-depends, as binary packages should not have dependencies on
libbinutils. It causes linux-tools to be locked to a specific binutils
version, causing issues if binutils needs to be updated.

We've gone through this before with perf, where we disabled use of libfd
in perf but seemingly forgot to remove the binutils-dev build-depends
(I've looked through our history to confirm that the dependency was only
added for perf). I've already removed the dependency in eoan, and we
should do the same when bringing bpftool to older releases.

Thanks,
Seth

>
> [Testcase]
>
> A test linux package was successfully built, at:
>
> https://launchpad.net/~qmonnet/+archive/ubuntu/ppa-linux-bpftool
>
> (Built with:
>   do_zfs=false
>   do_dkms_nvidia=false
>   do_dkms_vbox=false
>   skipabi=true
>   skipmodule=true
>   skipretpoline=true)
>
> Packages linux-tools-$(uname -r) and linux-tools-common can be built
> with "debian/rules binary", and contain bpftool's binary and related
> files (redirection script, bpftool manual pages, bash completion),
> respectively.
>
> [Regression Potential]
>
> Low, as far as I can tell:
>
> - The backported patch touches only bpftool and one feature in
>   tools/build/feature (only used with bpftool), all of it user space
>   tools not used for anything other than bpftool itself.
>
> - bpftool packaging does not change the way other tools are packaged
>   (apart from creating $(toolsman)/man8 a few lines earlier), and should
>   have no impact on the packaging of other tools. One dependency is
>   added to Build-Depends-Indep, none is removed.
>
> ---
> First version of this set was posted for bionic, but before the patch
> for packaging bpftool had been submitted and accepted to the development
> branch (currently eoan) [0]. That same patch is now upstream in
> eoan/master-next as commit 8579afd20b0c.
>
> [0] https://lists.ubuntu.com/archives/kernel-team/2019-July/102462.html
>
> Changes in v2:
> - Patch has been applied to eoan.
> - Switch from python-docutils to python3-docutils for the dependency
>   providing rst2man for manual pages.
> - Set CROSS_COMPILE when building bpftool.
>
> Quentin Monnet (2):
>   tools/bpftool: fix bpftool build with bintutils >= 2.9
>   UBUNTU: [Debian] package bpftool in linux-tools-common
>
>  debian.master/control.stub.in                 |  1 +
>  debian.master/rules.d/amd64.mk                |  1 +
>  debian.master/rules.d/arm64.mk                |  1 +
>  debian.master/rules.d/armhf.mk                |  1 +
>  debian.master/rules.d/i386.mk                 |  1 +
>  debian.master/rules.d/ppc64el.mk              |  1 +
>  debian.master/rules.d/s390x.mk                |  1 +
>  debian/rules                                  |  2 +-
>  debian/rules.d/1-maintainer.mk                |  1 +
>  debian/rules.d/2-binary-arch.mk               |  9 ++++++
>  debian/rules.d/3-binary-indep.mk              | 12 +++++++-
>  tools/bpf/Makefile                            | 29 +++++++++++++++++++
>  tools/bpf/bpf_jit_disasm.c                    |  7 +++++
>  tools/bpf/bpftool/Makefile                    | 24 +++++++++++++++
>  tools/bpf/bpftool/jit_disasm.c                |  7 +++++
>  tools/build/feature/Makefile                  |  4 +++
>  .../feature/test-disassembler-four-args.c     | 15 ++++++++++
>  17 files changed, 115 insertions(+), 2 deletions(-)
>  create mode 100644 tools/build/feature/test-disassembler-four-args.c
>
> --
> 2.17.1
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: NAK: [SRU][BIONIC][PATCH v2 0/2] add bpftool to linux-tools-common

Quentin Monnet
2019-09-27 07:38 UTC-0500 ~ Seth Forshee <[hidden email]>

> On Mon, Sep 09, 2019 at 12:39:21PM +0100, Quentin Monnet wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1774815
>>
>> [Impact]
>>
>> bpftool is a debugging and introspection tool for BPF elements,
>> developed by the BPF kernel community. It is essential to list and dump
>> BPF programs and maps loaded on the system. Its sources are located in
>> the kernel repository, and because it is not packaged, administrators
>> willing to use bpftool must download the whole kernel sources, compile
>> and install the utility.
>>
>> [Fix]
>>
>> Adding bpftool to linux-tools and linux-tools-common packages makes it
>> easily accessible. These packages are already used to provide other
>> tools located in the kernel repository, such as perf.
>>
>> Because the bpftool version provided with kernel 4.15 does not build
>> properly (API changed at some point in bfd.h from binutils-dev),
>> backport a patch from 4.16 to fix the calls to libbfd's disassembler.
>
> What should be done instead is to remove binutils-dev from the package
> build-depends, as binary packages should not have dependencies on
> libbinutils. It causes linux-tools to be locked to a specific binutils
> version, causing issues if binutils needs to be updated.
>
> We've gone through this before with perf, where we disabled use of libfd
> in perf but seemingly forgot to remove the binutils-dev build-depends
> (I've looked through our history to confirm that the dependency was only
> added for perf). I've already removed the dependency in eoan, and we
> should do the same when bringing bpftool to older releases.
>
> Thanks,
> Seth

Hi Seth,

Thanks for the feedback. You removed binutils-dev from the dependencies
in commit 721d5f0c5807, right? As you mentioned, we loose some
functionalities by doing so. For bpftool, this means no disassembling of
the JIT-compiled code. I'd like to keep that feature if possible, but I
understand the dependency on a specific version of binutils-dev is
painful to manage :s. Do you see any alternative solution that would
allow keeping binutils-dev?

If not, and regarding bpftool compilation: binutils-dev was made
optional in kernel patch 29a9c10e4110, in linux v5.0 (so not in bionic's
kernel). Would you have me backport this patch instead, then (plus the
patch to remove binutils-dev from build-depends)?

Best regards,
Quentin

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: NAK: [SRU][BIONIC][PATCH v2 0/2] add bpftool to linux-tools-common

Seth Forshee
On Wed, Oct 09, 2019 at 02:10:46AM +0100, Quentin Monnet wrote:

> 2019-09-27 07:38 UTC-0500 ~ Seth Forshee <[hidden email]>
> > On Mon, Sep 09, 2019 at 12:39:21PM +0100, Quentin Monnet wrote:
> >> BugLink: https://bugs.launchpad.net/bugs/1774815
> >>
> >> [Impact]
> >>
> >> bpftool is a debugging and introspection tool for BPF elements,
> >> developed by the BPF kernel community. It is essential to list and dump
> >> BPF programs and maps loaded on the system. Its sources are located in
> >> the kernel repository, and because it is not packaged, administrators
> >> willing to use bpftool must download the whole kernel sources, compile
> >> and install the utility.
> >>
> >> [Fix]
> >>
> >> Adding bpftool to linux-tools and linux-tools-common packages makes it
> >> easily accessible. These packages are already used to provide other
> >> tools located in the kernel repository, such as perf.
> >>
> >> Because the bpftool version provided with kernel 4.15 does not build
> >> properly (API changed at some point in bfd.h from binutils-dev),
> >> backport a patch from 4.16 to fix the calls to libbfd's disassembler.
> >
> > What should be done instead is to remove binutils-dev from the package
> > build-depends, as binary packages should not have dependencies on
> > libbinutils. It causes linux-tools to be locked to a specific binutils
> > version, causing issues if binutils needs to be updated.
> >
> > We've gone through this before with perf, where we disabled use of libfd
> > in perf but seemingly forgot to remove the binutils-dev build-depends
> > (I've looked through our history to confirm that the dependency was only
> > added for perf). I've already removed the dependency in eoan, and we
> > should do the same when bringing bpftool to older releases.
> >
> > Thanks,
> > Seth
>
> Hi Seth,
>
> Thanks for the feedback. You removed binutils-dev from the dependencies
> in commit 721d5f0c5807, right? As you mentioned, we loose some
> functionalities by doing so. For bpftool, this means no disassembling of
> the JIT-compiled code. I'd like to keep that feature if possible, but I
> understand the dependency on a specific version of binutils-dev is
> painful to manage :s. Do you see any alternative solution that would
> allow keeping binutils-dev?

The binutils-dev package description forbids dependencies on libbfd:

 This package includes header files and static libraries necessary to build
 programs which use the GNU BFD library, which is part of binutils.  Note
 that building Debian packages which depend on the shared libbfd is Not
 Allowed.

My understanding is that this is because libbfd does not promise ABI
compatibility, and we would end up locked to a specific libbfd version
meaning binutils updates get blocked by the kernel also needing to be
updated. Then once both are updated the tools packages for any older
kernels will not have the libbfd version they need. As long as this is
the situation with libbfd I don't know any way around it.

> If not, and regarding bpftool compilation: binutils-dev was made
> optional in kernel patch 29a9c10e4110, in linux v5.0 (so not in bionic's
> kernel). Would you have me backport this patch instead, then (plus the
> patch to remove binutils-dev from build-depends)?

Yes, from what I know about the situation this is probably the only way
forward currently.

Thanks,
Seth

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team