* [PATCH bpf-next] bpftool: make libbfd optional
@ 2018-11-12 18:53 Stanislav Fomichev
2018-11-12 19:42 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 18:53 UTC (permalink / raw)
To: netdev, ast, daniel, jakub.kicinski, quentin.monnet; +Cc: Stanislav Fomichev
Make it possible to build bpftool without libbfd. 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 <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 13 +++++++++++--
tools/bpf/bpftool/jit_disasm.c | 7 ++++++-
tools/bpf/bpftool/main.c | 3 +--
tools/bpf/bpftool/main.h | 11 +++++++++++
4 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index dac7eff4c7e5..1bea6b979082 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -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
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index c75ffd9ce2bb..6176bfe66f22 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -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,8 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
bfd_close(bfdf);
}
+
+void disasm_init(void)
+{
+ bfd_init();
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 75a3296dc0bc..51aa71069b14 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -31,7 +31,6 @@
* SOFTWARE.
*/
-#include <bfd.h>
#include <ctype.h>
#include <errno.h>
#include <getopt.h>
@@ -399,7 +398,7 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
- bfd_init();
+ disasm_init();
ret = cmd_select(cmds, argc, argv, do_help);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 61d82020af58..ec1bc2ae3c71 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -147,8 +147,19 @@ 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);
+void 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)
+{
+ p_err("No libbfd support");
+}
+static inline void disasm_init(void) {}
+#endif
void print_data_json(uint8_t *data, size_t len);
void print_hex_data_json(uint8_t *data, size_t len);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: make libbfd optional
2018-11-12 18:53 [PATCH bpf-next] bpftool: make libbfd optional Stanislav Fomichev
@ 2018-11-12 19:42 ` Jakub Kicinski
2018-11-12 19:58 ` Stanislav Fomichev
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2018-11-12 19:42 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, ast, daniel, quentin.monnet
On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> Make it possible to build bpftool without libbfd. 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 <sdf@google.com>
Would you mind spelling out the motivation?
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 61d82020af58..ec1bc2ae3c71 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -147,8 +147,19 @@ 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);
> +void 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)
> +{
> + p_err("No libbfd support");
> +}
I think an error per instruction is a bit much, could we make sure we
error out earlier?
> +static inline void disasm_init(void) {}
> +#endif
> void print_data_json(uint8_t *data, size_t len);
> void print_hex_data_json(uint8_t *data, size_t len);
Otherwise LGTM.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: make libbfd optional
2018-11-12 19:42 ` Jakub Kicinski
@ 2018-11-12 19:58 ` Stanislav Fomichev
2018-11-12 20:35 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 19:58 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
On 11/12, Jakub Kicinski wrote:
> On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > Make it possible to build bpftool without libbfd. 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 <sdf@google.com>
>
> Would you mind spelling out the motivation?
Ack, will do in v2 commit message. tldr - some fleet machines don't
have dev/dbg stuff installed; would like an option to have bpftool that
works everywhere for introspection.
> > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > index 61d82020af58..ec1bc2ae3c71 100644
> > --- a/tools/bpf/bpftool/main.h
> > +++ b/tools/bpf/bpftool/main.h
> > @@ -147,8 +147,19 @@ 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);
> > +void 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)
> > +{
> > + p_err("No libbfd support");
> > +}
>
> I think an error per instruction is a bit much, could we make sure we
> error out earlier?
I can return int from disasm_print_insn as an indication to
stop/continue. Let me know if you have better ideas, will post v2 later
today.
> > +static inline void disasm_init(void) {}
> > +#endif
> > void print_data_json(uint8_t *data, size_t len);
> > void print_hex_data_json(uint8_t *data, size_t len);
>
> Otherwise LGTM.
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: make libbfd optional
2018-11-12 19:58 ` Stanislav Fomichev
@ 2018-11-12 20:35 ` Jakub Kicinski
2018-11-12 21:30 ` Stanislav Fomichev
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2018-11-12 20:35 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
On Mon, 12 Nov 2018 11:58:27 -0800, Stanislav Fomichev wrote:
> On 11/12, Jakub Kicinski wrote:
> > On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > index 61d82020af58..ec1bc2ae3c71 100644
> > > --- a/tools/bpf/bpftool/main.h
> > > +++ b/tools/bpf/bpftool/main.h
> > > @@ -147,8 +147,19 @@ 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);
> > > +void 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)
> > > +{
> > > + p_err("No libbfd support");
> > > +}
> >
> > I think an error per instruction is a bit much, could we make sure we
> > error out earlier?
>
> I can return int from disasm_print_insn as an indication to
> stop/continue. Let me know if you have better ideas, will post v2 later
> today.
Why not simply:
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..44edf6705ad2 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -466,6 +466,10 @@ static int do_dump(int argc, char **argv)
int fd;
if (is_prefix(*argv, "jited")) {
+#ifndef HAVE_LIBBFD_SUPPORT
+ p_err("No libbfd support");
+ return -1;
+#endif
member_len = &info.jited_prog_len;
member_ptr = &info.jited_prog_insns;
} else if (is_prefix(*argv, "xlated")) {
Perhaps wrapped into some helper in the header to avoid the yuckiness.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] bpftool: make libbfd optional
2018-11-12 20:35 ` Jakub Kicinski
@ 2018-11-12 21:30 ` Stanislav Fomichev
0 siblings, 0 replies; 5+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 21:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
On 11/12, Jakub Kicinski wrote:
> On Mon, 12 Nov 2018 11:58:27 -0800, Stanislav Fomichev wrote:
> > On 11/12, Jakub Kicinski wrote:
> > > On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > > index 61d82020af58..ec1bc2ae3c71 100644
> > > > --- a/tools/bpf/bpftool/main.h
> > > > +++ b/tools/bpf/bpftool/main.h
> > > > @@ -147,8 +147,19 @@ 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);
> > > > +void 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)
> > > > +{
> > > > + p_err("No libbfd support");
> > > > +}
> > >
> > > I think an error per instruction is a bit much, could we make sure we
> > > error out earlier?
> >
> > I can return int from disasm_print_insn as an indication to
> > stop/continue. Let me know if you have better ideas, will post v2 later
> > today.
>
> Why not simply:
Makes sense. I think I can actually repurpose disasm_init for that (the
function I added to wrap bfd_init, I think I can move the callsite here
without any problems).
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee282409..44edf6705ad2 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -466,6 +466,10 @@ static int do_dump(int argc, char **argv)
> int fd;
>
> if (is_prefix(*argv, "jited")) {
> +#ifndef HAVE_LIBBFD_SUPPORT
> + p_err("No libbfd support");
> + return -1;
> +#endif
> member_len = &info.jited_prog_len;
> member_ptr = &info.jited_prog_insns;
> } else if (is_prefix(*argv, "xlated")) {
>
> Perhaps wrapped into some helper in the header to avoid the yuckiness.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-13 7:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-12 18:53 [PATCH bpf-next] bpftool: make libbfd optional Stanislav Fomichev
2018-11-12 19:42 ` Jakub Kicinski
2018-11-12 19:58 ` Stanislav Fomichev
2018-11-12 20:35 ` Jakub Kicinski
2018-11-12 21:30 ` Stanislav Fomichev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.