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.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.mon...@netronome.com>
> [...]
> > +
> > + fd = map_parse_fd(&argc, &argv);
> > + if (fd < 0)
> > + goto err_free_reuse_maps;
> > +
> > + map_replace = reallocarray(map_replace, old_map_fds + 1,
> > + sizeof(*map_replace));
> > + if (!map_replace) {
> > + p_err("mem alloc failed");
> > + goto err_free_reuse_maps;
>
> 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?
>
> You probably need some fallback implementation or in general have something
> bpftool internal that doesn't make a bet on its availability.
>
> # make
>
> Auto-detecting system features:
> ... libbfd: [ on ]
> ... disassembler-four-args: [ OFF ]
>
> 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
>
> Auto-detecting system features:
> ... libbfd: [ on ]
> ... disassembler-four-args: [ OFF ]
>
> CC map_perf_ring.o
> CC xlated_dumper.o
> CC perf.o
> CC prog.o
> prog.c: In function ‘do_load’:
> prog.c:785:18: warning: implicit declaration of function ‘reallocarray’
> [-Wimplicit-function-declaration]
> map_replace = reallocarray(map_replace, old_map_fds + 1,
> ^~~~~~~~~~~~
> prog.c:785:16: warning: assignment makes pointer from integer without a cast
> [-Wint-conversion]
> map_replace = 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
>
> Auto-detecting system features:
> ... libelf: [ on ]
> ... bpf: [ on ]
>
> Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from
> 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 ?= install
RM ?= rm -f
FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray
+FEATURE_DISPLAY = libbfd disassembler-four-args reallocarray
check_feat := 1
NON_CHECK_FEAT_TARGETS := 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 <stdlib.h>
+
+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 <linux/kernel.h>
#include <linux/hashtable.h>
+#include "compat.h"
#include "json_writer.h"
#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= \
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=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
$(OUTPUT)test-disassembler-four-args.bin:
$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
+$(OUTPUT)test-reallocarray.bin:
+ $(BUILD)
+
$(OUTPUT)test-liberty.bin:
$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"'
$(LDFLAGS) -lbfd -ldl -liberty
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 <stdlib.h>
+
+int main(void)
+{
+ return !!reallocarray(NULL, 1, 1);
+}
