Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752208AbdLSQW4 (ORCPT ); Tue, 19 Dec 2017 11:22:56 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34386 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751326AbdLSQWy (ORCPT ); Tue, 19 Dec 2017 11:22:54 -0500 X-Google-Smtp-Source: ACJfBosMOxoqL1mMqL+HCoSYOZqnMRuRy9FQrRRqbtknbs1AcdgNCnfbCyGZ4nQTqzp0eEvxnGSvRg== Subject: Re: [RFC PATCH net-next] tools/bpf: fix build with binutils >= 2.28 To: Roman Gushchin Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Jakub Kicinski , Alexei Starovoitov , Daniel Borkmann References: <20171219143821.26291-1-guro@fb.com> <79acdc04-bba9-c9d5-a651-57d0e9628653@netronome.com> <20171219161009.GA30572@castle.DHCP.thefacebook.com> From: Quentin Monnet Message-ID: <140ea0e9-964f-6f62-0721-88c9f8905cd3@netronome.com> Date: Tue, 19 Dec 2017 16:22:51 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171219161009.GA30572@castle.DHCP.thefacebook.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4837 Lines: 122 2017-12-19 16:10 UTC+0000 ~ Roman Gushchin > On Tue, Dec 19, 2017 at 03:57:02PM +0000, Quentin Monnet wrote: >> Hi Roman, thanks for working on this! >> >> 2017-12-19 14:38 UTC+0000 ~ Roman Gushchin >>> Bpftool build is broken with binutils version 2.28 and later. >>> The cause is commit 003ca0fd2286 ("Refactor disassembler selection") >>> in the binutils repo, which changed the disassembler() function >>> signature. >>> >>> Fix this by checking binutils version and use an appropriate >>> disassembler() signature. >>> >>> Signed-off-by: Roman Gushchin >>> Cc: Jakub Kicinski >>> Cc: Alexei Starovoitov >>> Cc: Daniel Borkmann >>> --- >>> tools/bpf/Makefile | 6 ++++++ >>> tools/bpf/bpf_jit_disasm.c | 7 +++++++ >>> tools/bpf/bpftool/Makefile | 6 ++++++ >>> tools/bpf/bpftool/jit_disasm.c | 5 +++++ >>> 4 files changed, 24 insertions(+) >>> >>> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile >>> index 07a6697466ef..3fd32fd0daa1 100644 >>> --- a/tools/bpf/Makefile >>> +++ b/tools/bpf/Makefile >>> @@ -6,8 +6,14 @@ LEX = flex >>> YACC = bison >>> MAKE = make >>> >>> +BINUTILS_VER := $(word 4, $(shell readelf -v | head -n 1)) >>> +BINUTILS_VER_MAJ := $(word 1, $(subst ., , $(subst -, , ${BINUTILS_VER}))) >>> +BINUTILS_VER_MIN := $(word 2, $(subst ., , $(subst -, , ${BINUTILS_VER}))) >>> + >>> CFLAGS += -Wall -O2 >>> CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include >>> +CFLAGS += -DBINUTILS_VER_MAJ=${BINUTILS_VER_MAJ} >>> +CFLAGS += -DBINUTILS_VER_MIN=${BINUTILS_VER_MIN} >>> >>> %.yacc.c: %.y >>> $(YACC) -o $@ -d $< >>> diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c >>> index 75bf526a0168..3ef7c8bdc0f3 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); >>> >>> +#if (BINUTILS_VER_MAJ >= 2) && (BINUTILS_VER_MIN >= 28) >>> + disassemble = disassembler(bfd_get_arch(bfdf), >>> + bfd_big_endian(bfdf), >>> + bfd_get_mach(bfdf), >>> + bfdf); >>> +#else >>> disassemble = disassembler(bfdf); >>> +#endif >>> assert(disassemble); >>> >>> do { >>> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile >>> index 3f17ad317512..94ad51bf14b5 100644 >>> --- a/tools/bpf/bpftool/Makefile >>> +++ b/tools/bpf/bpftool/Makefile >>> @@ -40,6 +40,12 @@ 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) >>> >>> +BINUTILS_VER := $(word 4, $(shell readelf -v | head -n 1)) >> >> This does not seem to be portable. I tried that on Ubuntu and `readelf >> -v` returns "GNU readelf (GNU Binutils for Ubuntu) 2.26.1", and >> BINUTILS_VER catches "Binutils". >> >>> +BINUTILS_VER_MAJ := $(word 1, $(subst ., , $(subst -, , ${BINUTILS_VER}))) >>> +BINUTILS_VER_MIN := $(word 2, $(subst ., , $(subst -, , ${BINUTILS_VER}))) >>> +CFLAGS += -DBINUTILS_VER_MAJ=${BINUTILS_VER_MAJ} >>> +CFLAGS += -DBINUTILS_VER_MIN=${BINUTILS_VER_MIN} >>> + >>> INSTALL ?= install >>> RM ?= rm -f >>> >>> diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c >>> index 1551d3918d4c..eaa7127e9eeb 100644 >>> --- a/tools/bpf/bpftool/jit_disasm.c >>> +++ b/tools/bpf/bpftool/jit_disasm.c >>> @@ -107,7 +107,12 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes) >>> >>> disassemble_init_for_target(&info); >>> >>> +#if (BINUTILS_VER_MAJ >= 2) && (BINUTILS_VER_MIN >= 28) >>> + disassemble = disassembler(bfd_get_arch(bfdf), bfd_big_endian(bfdf), >>> + bfd_get_mach(bfdf), bfdf); >>> +#else >>> disassemble = disassembler(bfdf); >>> +#endif >>> assert(disassemble); >>> >>> if (json_output) >> >> I discussed this issue with Jakub recently, and one suggestion he had >> was to look in tools/build/feature to add a new "feature", by trying to >> compile short programs, for making the distinction between binutils >> versions. It probably requires more work, but could be more robust than >> parsing the version from the command line? > > Hm, might be an option. Parsing readelf output is pretty ugly, here I agree. > In general it feels more like a binutils issue, so we have to workaround it > in either way. > > Is Jakub or someone else working on it? > > Thanks! > Jakub isn't. On our side, I noticed last week that there was this change in binutils, and started to have a look at how these "features" work. But I have nothing that works so far, so feel free to tackle this. Quentin