bpftool: make libbfd optional

PATCH Applied Stanislav Fomichev 5 participants 8 messages Nov 13, 2018 – Nov 17, 2018 1 message not in the archive

Subsystems (4)

Networking Network drivers BPF bpftool

lore

Share & tools

lore

Stanislav Fomichev

author

2018-11-13 07:39 UTC #0 lore raw

PATCH v2 1/1 bpftool: make libbfd optional

Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.

Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd

Signed-off-by Stanislav Fomichev <[email protected]>

---
 tools/bpf/bpftool/Makefile     | 13 +++++++++++--
 tools/bpf/bpftool/jit_disasm.c |  8 +++++++-
 tools/bpf/bpftool/main.c       |  3 ---
 tools/bpf/bpftool/main.h       | 14 ++++++++++++++
 tools/bpf/bpftool/prog.c       |  3 +++
 5 files changed, 35 insertions(+), 6 deletions(-)

tools/bpf/bpftool/Makefile +11 -2

index dac7eff4c7e5..1bea6b979082 100644
@@ -53,7 +53,7 @@ ifneq ($(EXTRA_LDFLAGS),)
 LDFLAGS += $(EXTRA_LDFLAGS)
 endif
 
-LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
+LIBS = -lelf $(LIBBPF)
 
 INSTALL ?= install
 RM ?= rm -f

@@ -90,7 +90,16 @@ include $(wildcard $(OUTPUT)*.d)
 
 all: $(OUTPUT)bpftool
 
-SRCS = $(wildcard *.c)
+BFD_SRCS = jit_disasm.c
+
+SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
+
+ifeq ($(feature-libbfd),1)
+CFLAGS += -DHAVE_LIBBFD_SUPPORT
+SRCS += $(BFD_SRCS)
+LIBS += -lbfd -lopcodes
+endif
+
 OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
 
 $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c

tools/bpf/bpftool/jit_disasm.c +7 -1

index c75ffd9ce2bb..b2ed5ee1af5f 100644
@@ -109,7 +109,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 		if (inf) {
 			bfdf->arch_info = inf;
 		} else {
-			p_err("No libfd support for %s", arch);
+			p_err("No libbfd support for %s", arch);
 			return;
 		}
 	}

@@ -183,3 +183,9 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 
 	bfd_close(bfdf);
 }
+
+int disasm_init(void)
+{
+	bfd_init();
+	return 0;
+}

tools/bpf/bpftool/main.c +0 -3

index 75a3296dc0bc..5c4c1cd5a7ba 100644
@@ -31,7 +31,6 @@
  * SOFTWARE.
  */
 
-#include <bfd.h>
 #include <ctype.h>
 #include <errno.h>
 #include <getopt.h>

@@ -399,8 +398,6 @@ int main(int argc, char **argv)
 	if (argc < 0)
 		usage();
 
-	bfd_init();
-
 	ret = cmd_select(cmds, argc, argv, do_help);
 
 	if (json_output)

tools/bpf/bpftool/main.h +14 -0

index 61d82020af58..10c6c16fae29 100644
@@ -147,8 +147,22 @@ int prog_parse_fd(int *argc, char ***argv);
 int map_parse_fd(int *argc, char ***argv);
 int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
 
+#ifdef HAVE_LIBBFD_SUPPORT
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 		       const char *arch, const char *disassembler_options);
+int disasm_init(void);
+#else
+static inline
+void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
+		       const char *arch, const char *disassembler_options)
+{
+}
+static inline int disasm_init(void)
+{
+	p_err("No libbfd support");
+	return -1;
+}
+#endif
 void print_data_json(uint8_t *data, size_t len);
 void print_hex_data_json(uint8_t *data, size_t len);

tools/bpf/bpftool/prog.c +3 -0

index 5ff5544596e7..c176e1aa66fe 100644
@@ -467,6 +467,9 @@ static int do_dump(int argc, char **argv)
 	int fd;
 
 	if (is_prefix(*argv, "jited")) {
+		if (disasm_init())
+			return -1;
+
 		member_len = &info.jited_prog_len;
 		member_ptr = &info.jited_prog_insns;
 	} else if (is_prefix(*argv, "xlated")) {

Signature · 1 line

2.19.1.930.g4563a0d9d0-goog

Jakub Kicinski

Acked-by on v2 1/1

2018-11-13 07:57 UTC #1 lore raw

Quoted 19 lines · Stanislav Fomichev

Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd

Quoted 1 line

Signed-off-by Stanislav Fomichev <[email protected]>

Seems reasonable, thanks!

Acked-by Jakub Kicinski <[email protected]>

Quentin Monnet

2018-11-13 16:00 UTC #2 lore raw

2018-11-12 14:02 UTC-0800 ~ Jakub Kicinski <[email protected]>

Quoted 20 lines · Stanislav Fomichev

Quoted 19 lines
Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd

Quoted 6 lines

Quoted 1 line
Signed-off-by Stanislav Fomichev <[email protected]>
Seems reasonable, thanks!
Acked-by Jakub Kicinski <[email protected]>

Thanks Stanislav!

There is a problem with this patch on some distributions, Ubuntu at least.

Feature detection for libbfd has been used for perf before being also used with bpftool. Since commit 280e7c48c3b8 the feature needs libz and libiberty to be present on the system, otherwise the feature would not compile (and be detected) on OpenSuse.

On Ubuntu, libiberty is not needed (libbfd might be statically linked against it, if I remember correctly?), which means that we are able to build bpftool as long as binutils-dev has been installed, even if libiberty-dev has not been installed. The BFD feature, in that case, will appear as “undetected”. It is a bug. But since the Makefile does not stop compilation in that case (another bug), in the end we're good.

With your patch, the problem is that libbpf detection will fail on Ubuntu if libiberty-dev is not present, even though all the necessary libraries for using the JIT disassembler are available. And in that case it _will_ make a difference, since the Makefile will no more compile the libbfd-related bits.

So I'm not against the idea, but we have to fix libbfd detection first.

Thanks,
Quentin

Stanislav Fomichev

2018-11-14 01:44 UTC #3 lore raw

Quoted 44 lines · Quentin Monnet

2018-11-12 14:02 UTC-0800 ~ Jakub Kicinski <[email protected]>
Quoted 20 lines · Stanislav Fomichev
Quoted 19 lines
Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Quoted 6 lines
Quoted 1 line
Signed-off-by Stanislav Fomichev <[email protected]>
Seems reasonable, thanks!
Acked-by Jakub Kicinski <[email protected]>
Thanks Stanislav!
There is a problem with this patch on some distributions, Ubuntu at least.
Feature detection for libbfd has been used for perf before being also used with bpftool. Since commit 280e7c48c3b8 the feature needs libz and libiberty to be present on the system, otherwise the feature would not compile (and be detected) on OpenSuse.
On Ubuntu, libiberty is not needed (libbfd might be statically linked against it, if I remember correctly?), which means that we are able to build bpftool as long as binutils-dev has been installed, even if libiberty-dev has not been installed. The BFD feature, in that case, will appear as “undetected”. It is a bug. But since the Makefile does not stop compilation in that case (another bug), in the end we're good.

Quoted 7 lines

With your patch, the problem is that libbpf detection will fail on Ubuntu if libiberty-dev is not present, even though all the necessary libraries for using the JIT disassembler are available. And in that case it _will_ make a difference, since the Makefile will no more compile the libbfd-related bits.
So I'm not against the idea, but we have to fix libbfd detection first.

Yeah, libbfd feature detection looks broken on ubuntu, this patch just exercises this brokenness :-). I can take a look somewhere this week, thanks for spotting it!

I wonder, how does it work on opensuse currently if we link only against -lbfd
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/bpf/bpftool/Makefile#n56)?
It might be broken everywhere...

(Resent with proper formatting and without HTML).

Quoted 2 lines

Thanks,
Quentin

Stanislav Fomichev

2018-11-16 10:52 UTC #4 lore raw

In reply to Quentin Monnet

Quoted 44 lines · Quentin Monnet

2018-11-12 14:02 UTC-0800 ~ Jakub Kicinski <[email protected]>
Quoted 20 lines · Stanislav Fomichev
Quoted 19 lines
Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Quoted 6 lines
Quoted 1 line
Signed-off-by Stanislav Fomichev <[email protected]>
Seems reasonable, thanks!
Acked-by Jakub Kicinski <[email protected]>
Thanks Stanislav!
There is a problem with this patch on some distributions, Ubuntu at least.
Feature detection for libbfd has been used for perf before being also used with bpftool. Since commit 280e7c48c3b8 the feature needs libz and libiberty to be present on the system, otherwise the feature would not compile (and be detected) on OpenSuse.
On Ubuntu, libiberty is not needed (libbfd might be statically linked against it, if I remember correctly?), which means that we are able to build bpftool as long as binutils-dev has been installed, even if libiberty-dev has not been installed. The BFD feature, in that case, will appear as “undetected”. It is a bug. But since the Makefile does not stop compilation in that case (another bug), in the end we're good.

Quoted 7 lines

With your patch, the problem is that libbpf detection will fail on Ubuntu if libiberty-dev is not present, even though all the necessary libraries for using the JIT disassembler are available. And in that case it _will_ make a difference, since the Makefile will no more compile the libbfd-related bits.
So I'm not against the idea, but we have to fix libbfd detection first.

Sent out https://lkml.org/lkml/2018/11/16/243, let's see how it goes :-)

Quoted 2 lines

Thanks,
Quentin

Alexei Starovoitov

Applied

2018-11-17 15:03 UTC #5 lore raw

In reply to Stanislav Fomichev

Quoted 20 lines · Stanislav Fomichev

Make it possible to build bpftool without libbfd. libbfd and libopcodes are typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way to have bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error if the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd

Quoted 1 line

Signed-off-by Stanislav Fomichev <[email protected]>

applied, thanks

A message that is not in the archive (sent off-list or not yet received): CAKH8qBtbrNCNJkjqNH3vm_s_+x6vHbp8MFWSLgROMCF5A=xtjA@mail.gmail.com

Alexei Starovoitov

2018-11-17 15:12 UTC #7 lore raw

Quoted 3 lines · Stanislav Fomichev

I actually wanted to follow up with a v2 when
https://lkml.org/lkml/2018/11/16/243 reaches bpf-next (I got an ack already).

it will go via perf tree, so not related.

Quoted 2 lines

Alternatively, I can follow up with another patch on top of that to fix libbfd feature detection (it's semi broken on ubuntu/fedora now).

I think you'd need to wait until all trees merge.

pls don't top post.

Quoted 31 lines

Quoted 28 lines · Alexei Starovoitov
Quoted 2 lines · Stanislav Fomichev
Make it possible to build bpftool without libbfd. libbfd and libopcodes
are
Quoted 2 lines
typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way
to have
Quoted 2 lines
bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error
if
Quoted 14 lines
the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Quoted 1 line
Signed-off-by Stanislav Fomichev <[email protected]>
applied, thanks

Stanislav Fomichev

2018-11-17 15:22 UTC #8 lore raw

Quoted 6 lines · Alexei Starovoitov

Quoted 3 lines · Stanislav Fomichev
I actually wanted to follow up with a v2 when
https://lkml.org/lkml/2018/11/16/243 reaches bpf-next (I got an ack already).
it will go via perf tree, so not related.

My understanding was that you periodically merge whatever goes to Linus back to bpf-next, so my plan was to wait for that event and propose a v3.

Quoted 4 lines

Quoted 2 lines
Alternatively, I can follow up with another patch on top of that to fix libbfd feature detection (it's semi broken on ubuntu/fedora now).
I think you'd need to wait until all trees merge.

Sure, that's no problem. I'll follow up with another patch whenever that happens.

Quoted 1 line

pls don't top post.

Sorry, Gmail :-/

Quoted 31 lines

Quoted 31 lines
Quoted 28 lines · Alexei Starovoitov
Quoted 2 lines · Stanislav Fomichev
Make it possible to build bpftool without libbfd. libbfd and libopcodes
are
Quoted 2 lines
typically provided in dev/dbg packages (binutils-dev in debian) which we usually don't have installed on the fleet machines and we'd like a way
to have
Quoted 2 lines
bpftool version that works without installing any additional packages. This excludes support for disassembling jit-ted code and prints an error
if
Quoted 14 lines
the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Quoted 1 line
Signed-off-by Stanislav Fomichev <[email protected]>
applied, thanks