From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load Date: Thu, 5 Jul 2018 15:57:18 -0700 Message-ID: <20180705155718.35b9bdd1@cakuba.netronome.com> References: <20180704025417.8848-1-jakub.kicinski@netronome.com> <20180704025417.8848-12-jakub.kicinski@netronome.com> <3db866ff-5712-c308-9eef-18deaadf490d@iogearbox.net> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: alexei.starovoitov@gmail.com, oss-drivers@netronome.com, netdev@vger.kernel.org To: Daniel Borkmann Return-path: Received: from mail-qt0-f194.google.com ([209.85.216.194]:34028 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753013AbeGEW5X (ORCPT ); Thu, 5 Jul 2018 18:57:23 -0400 Received: by mail-qt0-f194.google.com with SMTP id m13-v6so8548564qth.1 for ; Thu, 05 Jul 2018 15:57:23 -0700 (PDT) In-Reply-To: <3db866ff-5712-c308-9eef-18deaadf490d@iogearbox.net> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 5 Jul 2018 10:35:24 +0200, Daniel Borkmann wrote: > On 07/04/2018 04:54 AM, Jakub Kicinski wrote: > > Add map parameter to prog load which will allow reuse of existing > > maps instead of creating new ones. > >=20 > > Signed-off-by: Jakub Kicinski > > Reviewed-by: Quentin Monnet =20 > [...] > > + > > + fd =3D map_parse_fd(&argc, &argv); > > + if (fd < 0) > > + goto err_free_reuse_maps; > > + > > + map_replace =3D reallocarray(map_replace, old_map_fds + 1, > > + sizeof(*map_replace)); > > + if (!map_replace) { > > + p_err("mem alloc failed"); > > + goto err_free_reuse_maps; =20 >=20 > Series in general looks good to me. However, above reallocarray() doesn't > exist and hence build fails, please see below. Is that from newest glibc? >=20 > You probably need some fallback implementation or in general have somethi= ng > bpftool internal that doesn't make a bet on its availability. >=20 > # make >=20 > Auto-detecting system features: > ... libbfd: [ on ] > ... disassembler-four-args: [ OFF ] >=20 > CC bpf_jit_disasm.o > LINK bpf_jit_disasm > CC bpf_dbg.o > LINK bpf_dbg > CC bpf_asm.o > BISON bpf_exp.yacc.c > CC bpf_exp.yacc.o > FLEX bpf_exp.lex.c > CC bpf_exp.lex.o > LINK bpf_asm > DESCEND bpftool >=20 > Auto-detecting system features: > ... libbfd: [ on ] > ... disassembler-four-args: [ OFF ] >=20 > CC map_perf_ring.o > CC xlated_dumper.o > CC perf.o > CC prog.o > prog.c: In function =E2=80=98do_load=E2=80=99: > prog.c:785:18: warning: implicit declaration of function =E2=80=98realloc= array=E2=80=99 [-Wimplicit-function-declaration] > map_replace =3D reallocarray(map_replace, old_map_fds + 1, > ^~~~~~~~~~~~ > prog.c:785:16: warning: assignment makes pointer from integer without a c= ast [-Wint-conversion] > map_replace =3D reallocarray(map_replace, old_map_fds + 1, > ^ > CC common.o > CC cgroup.o > CC main.o > CC json_writer.o > CC cfg.o > CC map.o > CC jit_disasm.o > CC disasm.o >=20 > Auto-detecting system features: > ... libelf: [ on ] > ... bpf: [ on ] >=20 > Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs fr= om latest version at 'include/uapi/linux/bpf.h' > CC libbpf.o > CC bpf.o > CC nlattr.o > CC btf.o > LD libbpf-in.o > LINK libbpf.a > LINK bpftool > prog.o: In function `do_load': > prog.c:(.text+0x23d): undefined reference to `reallocarray' > collect2: error: ld returned 1 exit status > Makefile:89: recipe for target 'bpftool' failed > make[1]: *** [bpftool] Error 1 > Makefile:99: recipe for target 'bpftool' failed > make: *** [bpftool] Error 2 Oh no.. Sorry & thanks for catching this. It would be nice to not depend on Glibc version defines, in case someone is using a different library. Jiong suggested we can just use the feature detection, so I have something like this: --- diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 0911b00b25cc..20a691659381 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -52,8 +52,8 @@ INSTALL ?=3D install RM ?=3D rm -f =20 FEATURE_USER =3D .bpftool -FEATURE_TESTS =3D libbfd disassembler-four-args -FEATURE_DISPLAY =3D libbfd disassembler-four-args +FEATURE_TESTS =3D libbfd disassembler-four-args reallocarray +FEATURE_DISPLAY =3D libbfd disassembler-four-args reallocarray =20 check_feat :=3D 1 NON_CHECK_FEAT_TARGETS :=3D clean uninstall doc doc-clean doc-install doc-= uninstall diff --git a/tools/bpf/bpftool/compat.h b/tools/bpf/bpftool/compat.h new file mode 100644 index 000000000000..7885cedc9efe --- /dev/null +++ b/tools/bpf/bpftool/compat.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Copyright (C) 2018 Netronome Systems, Inc. */ + +#ifndef __BPF_TOOL_COMPAT_H +#define __BPF_TOOL_COMPAT_H + +#define _GNU_SOURCE +#include + +static inline void *reallocarray(void *ptr, size_t nmemb, size_t size) +{ + return realloc(ptr, nmemb * size); +} +#endif diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index 1a9a2aefa014..2106adb73631 100644 --- a/tools/bpf/bpftool/main.h +++ b/tools/bpf/bpftool/main.h @@ -43,6 +43,7 @@ #include #include =20 +#include "compat.h" #include "json_writer.h" =20 #define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr)) diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index dac9563b5470..0516259be70f 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -14,6 +14,7 @@ FILES=3D \ test-libaudit.bin \ test-libbfd.bin \ test-disassembler-four-args.bin \ + test-reallocarray.bin \ test-liberty.bin \ test-liberty-z.bin \ test-cplus-demangle.bin \ @@ -204,6 +205,9 @@ FLAGS_PERL_EMBED=3D$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LD= OPTS) $(OUTPUT)test-disassembler-four-args.bin: $(BUILD) -DPACKAGE=3D'"perf"' -lbfd -lopcodes =20 +$(OUTPUT)test-reallocarray.bin: + $(BUILD) + $(OUTPUT)test-liberty.bin: $(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE=3D'"perf"' $(= LDFLAGS) -lbfd -ldl -liberty =20 diff --git a/tools/build/feature/test-reallocarray.c b/tools/build/feature/= test-reallocarray.c new file mode 100644 index 000000000000..8170de35150d --- /dev/null +++ b/tools/build/feature/test-reallocarray.c @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE +#include + +int main(void) +{ + return !!reallocarray(NULL, 1, 1); +}