netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load
@ 2018-07-10 21:42 Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 01/13] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
                   ` (13 more replies)
  0 siblings, 14 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Hi!

This series starts with two minor clean ups to test_offload.py
selftest script.

The next 11 patches extend the abilities of bpftool prog load
beyond the simple cgroup use cases.  Three new parameters are
added:

 - type - allows specifying program type, independent of how
   code sections are named;
 - map  - allows reusing existing maps, instead of creating a new
   map on every program load;
 - dev  - offload/binding to a device.

A number of changes to libbpf is required to accomplish the task.
The section - program type logic mapping is exposed.  We should
probably aim to use the libbpf program section naming everywhere.
For reuse of maps we need to allow users to set FD for bpf map
object in libbpf.

Examples

Load program my_xdp.o and pin it as /sys/fs/bpf/my_xdp, for xdp
program type:

$ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
  type xdp

As above but for offload:

$ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
  type xdp \
  dev netdevsim0

Load program my_maps.o, but for the first map reuse map id 17,
and for the map called "other_map" reuse pinned map /sys/fs/bpf/map0:

$ bpftool prog load my_maps.o /sys/fs/bpf/prog \
  map idx 0 id 17 \
  map name other_map pinned /sys/fs/bpf/map0

---
v3:
 - fix return codes in patch 5;
 - rename libbpf_prog_type_by_string() -> libbpf_prog_type_by_name();
 - fold file path into xattr in patch 8;
 - add patch 10;
 - use dup3() in patch 12;
 - depend on fd value in patch 12;
 - close old fd in patch 12.
v2:
 - add compat for reallocarray().

Jakub Kicinski (13):
  selftests/bpf: remove duplicated word from test offloads
  selftests/bpf: add Error: prefix in check_extack helper
  tools: bpftool: refactor argument parsing for prog load
  tools: bpftool: add support for loading programs for offload
  tools: libbpf: expose the prog type guessing from section name logic
  tools: bpftool: allow users to specify program type for prog load
  tools: libbpf: recognize offload neutral maps
  tools: libbpf: add extended attributes version of bpf_object__open()
  tools: bpftool: reimplement bpf_prog_load() for prog load
  tools: libbpf: move library error code into a separate file
  tools: bpf: make use of reallocarray
  tools: libbpf: allow map reuse
  tools: bpftool: allow reuse of maps with bpftool prog load

 .../bpftool/Documentation/bpftool-prog.rst    |  33 ++-
 tools/bpf/bpftool/Makefile                    |   6 +-
 tools/bpf/bpftool/bash-completion/bpftool     |  96 +++++-
 tools/bpf/bpftool/main.h                      |  19 ++
 tools/bpf/bpftool/map.c                       |   4 +-
 tools/bpf/bpftool/prog.c                      | 245 ++++++++++++++-
 tools/bpf/bpftool/xlated_dumper.c             |   6 +-
 tools/build/feature/Makefile                  |   4 +
 tools/build/feature/test-reallocarray.c       |   8 +
 tools/include/linux/compiler-gcc.h            |   4 +
 tools/include/linux/overflow.h                | 278 ++++++++++++++++++
 tools/include/tools/libc_compat.h             |  20 ++
 tools/lib/bpf/Build                           |   2 +-
 tools/lib/bpf/Makefile                        |   6 +-
 tools/lib/bpf/libbpf.c                        | 188 +++++++-----
 tools/lib/bpf/libbpf.h                        |  11 +
 tools/lib/bpf/libbpf_errno.c                  |  74 +++++
 tools/testing/selftests/bpf/test_offload.py   |  10 +-
 18 files changed, 906 insertions(+), 108 deletions(-)
 create mode 100644 tools/build/feature/test-reallocarray.c
 create mode 100644 tools/include/linux/overflow.h
 create mode 100644 tools/include/tools/libc_compat.h
 create mode 100644 tools/lib/bpf/libbpf_errno.c

-- 
2.17.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 01/13] selftests/bpf: remove duplicated word from test offloads
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
@ 2018-07-10 21:42 ` Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 02/13] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Trivial removal of duplicated "mode" in error message.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index be800d0e7a84..a257e4b08392 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -830,7 +830,7 @@ netns = []
     check_extack_nsim(err, "program loaded with different flags.", args)
     ret, _, err = sim.unset_xdp("", force=True,
                                 fail=False, include_stderr=True)
-    fail(ret == 0, "Removed program with a bad mode mode")
+    fail(ret == 0, "Removed program with a bad mode")
     check_extack_nsim(err, "program loaded with different flags.", args)
 
     start_test("Test MTU restrictions...")
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 02/13] selftests/bpf: add Error: prefix in check_extack helper
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 01/13] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
@ 2018-07-10 21:42 ` Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 03/13] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Currently the test only checks errors, not warnings, so save typing
and prefix the extack messages with "Error:" inside the check helper.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index a257e4b08392..f8d9bd81d9a4 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -547,11 +547,11 @@ netns = [] # net namespaces to be removed
     if skip_extack:
         return
     lines = output.split("\n")
-    comp = len(lines) >= 2 and lines[1] == reference
+    comp = len(lines) >= 2 and lines[1] == 'Error: ' + reference
     fail(not comp, "Missing or incorrect netlink extack message")
 
 def check_extack_nsim(output, reference, args):
-    check_extack(output, "Error: netdevsim: " + reference, args)
+    check_extack(output, "netdevsim: " + reference, args)
 
 def check_no_extack(res, needle):
     fail((res[1] + res[2]).count(needle) or (res[1] + res[2]).count("Warning:"),
@@ -654,7 +654,7 @@ netns = []
     ret, _, err = sim.cls_bpf_add_filter(obj, skip_sw=True,
                                          fail=False, include_stderr=True)
     fail(ret == 0, "TC filter loaded without enabling TC offloads")
-    check_extack(err, "Error: TC offload is disabled on net device.", args)
+    check_extack(err, "TC offload is disabled on net device.", args)
     sim.wait_for_flush()
 
     sim.set_ethtool_tc_offloads(True)
@@ -694,7 +694,7 @@ netns = []
                                          skip_sw=True,
                                          fail=False, include_stderr=True)
     fail(ret == 0, "Offloaded a filter to chain other than 0")
-    check_extack(err, "Error: Driver supports only offload of chain 0.", args)
+    check_extack(err, "Driver supports only offload of chain 0.", args)
     sim.tc_flush_filters()
 
     start_test("Test TC replace...")
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 03/13] tools: bpftool: refactor argument parsing for prog load
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 01/13] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 02/13] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
@ 2018-07-10 21:42 ` Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 04/13] tools: bpftool: add support for loading programs for offload Jakub Kicinski
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Add a new macro for printing more informative message than straight
usage() when parameters are missing, and use it for prog do_load().
Save the object and pin path argument to variables for clarity.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/main.h | 15 +++++++++++++++
 tools/bpf/bpftool/prog.c | 11 +++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index d39f7ef01d23..15b6c49ae533 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -50,6 +50,21 @@
 #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
 #define NEXT_ARGP()	({ (*argc)--; (*argv)++; if (*argc < 0) usage(); })
 #define BAD_ARG()	({ p_err("what is '%s'?", *argv); -1; })
+#define GET_ARG()	({ argc--; *argv++; })
+#define REQ_ARGS(cnt)							\
+	({								\
+		int _cnt = (cnt);					\
+		bool _res;						\
+									\
+		if (argc < _cnt) {					\
+			p_err("'%s' needs at least %d arguments, %d found", \
+			      argv[-1], _cnt, argc);			\
+			_res = false;					\
+		} else {						\
+			_res = true;					\
+		}							\
+		_res;							\
+	})
 
 #define ERR_MAX_LEN	1024
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index a740da99d477..a5ef46c59029 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -681,18 +681,21 @@ static int do_pin(int argc, char **argv)
 
 static int do_load(int argc, char **argv)
 {
+	const char *objfile, *pinfile;
 	struct bpf_object *obj;
 	int prog_fd;
 
-	if (argc != 2)
-		usage();
+	if (!REQ_ARGS(2))
+		return -1;
+	objfile = GET_ARG();
+	pinfile = GET_ARG();
 
-	if (bpf_prog_load(argv[0], BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
+	if (bpf_prog_load(objfile, BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
 		p_err("failed to load program");
 		return -1;
 	}
 
-	if (do_pin_fd(prog_fd, argv[1]))
+	if (do_pin_fd(prog_fd, pinfile))
 		goto err_close_obj;
 
 	if (json_output)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 04/13] tools: bpftool: add support for loading programs for offload
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (2 preceding siblings ...)
  2018-07-10 21:42 ` [PATCH bpf-next v3 03/13] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
@ 2018-07-10 21:42 ` Jakub Kicinski
  2018-07-10 21:42 ` [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Extend the bpftool prog load command to also accept "dev"
parameter, which will allow us to load programs onto devices.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 .../bpftool/Documentation/bpftool-prog.rst    |  6 ++--
 tools/bpf/bpftool/bash-completion/bpftool     | 23 ++++++++++--
 tools/bpf/bpftool/prog.c                      | 35 +++++++++++++++++--
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 43d34a5c3ec5..41723c6acaa6 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -24,7 +24,7 @@ MAP COMMANDS
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog pin** *PROG* *FILE*
-|	**bpftool** **prog load** *OBJ* *FILE*
+|	**bpftool** **prog load** *OBJ* *FILE* [**dev** *NAME*]
 |	**bpftool** **prog help**
 |
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
@@ -64,8 +64,10 @@ DESCRIPTION
 
 		  Note: *FILE* must be located in *bpffs* mount.
 
-	**bpftool prog load** *OBJ* *FILE*
+	**bpftool prog load** *OBJ* *FILE* [**dev** *NAME*]
 		  Load bpf program from binary *OBJ* and pin as *FILE*.
+		  If **dev** *NAME* is specified program will be loaded onto
+		  given networking device (offload).
 
 		  Note: *FILE* must be located in *bpffs* mount.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index ce0bc0cda361..238c2f80092a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -99,6 +99,12 @@ _bpftool_get_prog_tags()
         command sed -n 's/.*"tag": "\(.*\)",$/\1/p' )" -- "$cur" ) )
 }
 
+_sysfs_get_netdevs()
+{
+    COMPREPLY+=( $( compgen -W "$( ls /sys/class/net 2>/dev/null )" -- \
+        "$cur" ) )
+}
+
 # For bpftool map update: retrieve type of the map to update.
 _bpftool_map_update_map_type()
 {
@@ -262,8 +268,21 @@ _bpftool()
                     return 0
                     ;;
                 load)
-                    _filedir
-                    return 0
+                    if [[ ${#words[@]} -lt 6 ]]; then
+                        _filedir
+                        return 0
+                    fi
+
+                    case $prev in
+                        dev)
+                            _sysfs_get_netdevs
+                            return 0
+                            ;;
+                        *)
+                            _bpftool_once_attr 'dev'
+                            return 0
+                            ;;
+                    esac
                     ;;
                 *)
                     [[ $prev == $object ]] && \
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index a5ef46c59029..21c74de7156f 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -39,6 +39,7 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
+#include <net/if.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -681,6 +682,9 @@ static int do_pin(int argc, char **argv)
 
 static int do_load(int argc, char **argv)
 {
+	struct bpf_prog_load_attr attr = {
+		.prog_type	= BPF_PROG_TYPE_UNSPEC,
+	};
 	const char *objfile, *pinfile;
 	struct bpf_object *obj;
 	int prog_fd;
@@ -690,7 +694,34 @@ static int do_load(int argc, char **argv)
 	objfile = GET_ARG();
 	pinfile = GET_ARG();
 
-	if (bpf_prog_load(objfile, BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
+	while (argc) {
+		if (is_prefix(*argv, "dev")) {
+			NEXT_ARG();
+
+			if (attr.ifindex) {
+				p_err("offload device already specified");
+				return -1;
+			}
+			if (!REQ_ARGS(1))
+				return -1;
+
+			attr.ifindex = if_nametoindex(*argv);
+			if (!attr.ifindex) {
+				p_err("unrecognized netdevice '%s': %s",
+				      *argv, strerror(errno));
+				return -1;
+			}
+			NEXT_ARG();
+		} else {
+			p_err("expected no more arguments or 'dev', got: '%s'?",
+			      *argv);
+			return -1;
+		}
+	}
+
+	attr.file = objfile;
+
+	if (bpf_prog_load_xattr(&attr, &obj, &prog_fd)) {
 		p_err("failed to load program");
 		return -1;
 	}
@@ -722,7 +753,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
 		"       %s %s dump jited  PROG [{ file FILE | opcodes }]\n"
 		"       %s %s pin   PROG FILE\n"
-		"       %s %s load  OBJ  FILE\n"
+		"       %s %s load  OBJ  FILE [dev NAME]\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_PROGRAM "\n"
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (3 preceding siblings ...)
  2018-07-10 21:42 ` [PATCH bpf-next v3 04/13] tools: bpftool: add support for loading programs for offload Jakub Kicinski
@ 2018-07-10 21:42 ` Jakub Kicinski
  2018-07-11  3:01   ` Andrey Ignatov
  2018-07-10 21:43 ` [PATCH bpf-next v3 06/13] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:42 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

libbpf can guess program type based on ELF section names.  As libbpf
becomes more popular its association between section name strings and
types becomes more of a standard.  Allow libbpf users to use the same
logic for matching strings to types, e.g. when the string originates
from command line.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
v3 (Andrey):
 - use -EINVAL error code;
 - rename helper to libbpf_prog_type_by_name().
---
 tools/lib/bpf/libbpf.c | 43 ++++++++++++++++++++++++------------------
 tools/lib/bpf/libbpf.h |  3 +++
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 38ed3e92e393..42f31eff5f3f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2081,23 +2081,31 @@ static const struct {
 #undef BPF_S_PROG_SEC
 #undef BPF_SA_PROG_SEC
 
-static int bpf_program__identify_section(struct bpf_program *prog)
+int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
+			     enum bpf_attach_type *expected_attach_type)
 {
 	int i;
 
-	if (!prog->section_name)
-		goto err;
-
-	for (i = 0; i < ARRAY_SIZE(section_names); i++)
-		if (strncmp(prog->section_name, section_names[i].sec,
-			    section_names[i].len) == 0)
-			return i;
+	if (!name)
+		return -EINVAL;
 
-err:
-	pr_warning("failed to guess program type based on section name %s\n",
-		   prog->section_name);
+	for (i = 0; i < ARRAY_SIZE(section_names); i++) {
+		if (strncmp(name, section_names[i].sec, section_names[i].len))
+			continue;
+		*prog_type = section_names[i].prog_type;
+		*expected_attach_type = section_names[i].expected_attach_type;
+		return 0;
+	}
+	return -EINVAL;
+}
 
-	return -1;
+static int
+bpf_program__identify_section(struct bpf_program *prog,
+			      enum bpf_prog_type *prog_type,
+			      enum bpf_attach_type *expected_attach_type)
+{
+	return libbpf_prog_type_by_name(prog->section_name, prog_type,
+					expected_attach_type);
 }
 
 int bpf_map__fd(struct bpf_map *map)
@@ -2230,7 +2238,6 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 	enum bpf_prog_type prog_type;
 	struct bpf_object *obj;
 	struct bpf_map *map;
-	int section_idx;
 	int err;
 
 	if (!attr)
@@ -2252,14 +2259,14 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 		prog->prog_ifindex = attr->ifindex;
 		expected_attach_type = attr->expected_attach_type;
 		if (prog_type == BPF_PROG_TYPE_UNSPEC) {
-			section_idx = bpf_program__identify_section(prog);
-			if (section_idx < 0) {
+			err = bpf_program__identify_section(prog, &prog_type,
+							    &expected_attach_type);
+			if (err < 0) {
+				pr_warning("failed to guess program type based on section name %s\n",
+					   prog->section_name);
 				bpf_object__close(obj);
 				return -EINVAL;
 			}
-			prog_type = section_names[section_idx].prog_type;
-			expected_attach_type =
-				section_names[section_idx].expected_attach_type;
 		}
 
 		bpf_program__set_type(prog, prog_type);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 564f4be9bae0..d1ce5c828e2e 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -92,6 +92,9 @@ int bpf_object__set_priv(struct bpf_object *obj, void *priv,
 			 bpf_object_clear_priv_t clear_priv);
 void *bpf_object__priv(struct bpf_object *prog);
 
+int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
+			     enum bpf_attach_type *expected_attach_type);
+
 /* Accessors of bpf_program */
 struct bpf_program;
 struct bpf_program *bpf_program__next(struct bpf_program *prog,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 06/13] tools: bpftool: allow users to specify program type for prog load
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (4 preceding siblings ...)
  2018-07-10 21:42 ` [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-10 21:43 ` [PATCH bpf-next v3 07/13] tools: libbpf: recognize offload neutral maps Jakub Kicinski
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Sometimes program section names don't match with libbpf's expectation.
In particular XDP's default section names differ between libbpf and
iproute2.  Allow users to pass program type on command line.  Name
the types like the libbpf expected section names.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 .../bpftool/Documentation/bpftool-prog.rst    | 15 ++++++-
 tools/bpf/bpftool/bash-completion/bpftool     |  6 +++
 tools/bpf/bpftool/prog.c                      | 44 +++++++++++++++++--
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 41723c6acaa6..e53e1ad2caf0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -24,10 +24,19 @@ MAP COMMANDS
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog pin** *PROG* *FILE*
-|	**bpftool** **prog load** *OBJ* *FILE* [**dev** *NAME*]
+|	**bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
 |	**bpftool** **prog help**
 |
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
+|	*TYPE* := {
+|		**socket** | **kprobe** | **kretprobe** | **classifier** | **action** |
+|		**tracepoint** | **raw_tracepoint** | **xdp** | **perf_event** | **cgroup/skb** |
+|		**cgroup/sock** | **cgroup/dev** | **lwt_in** | **lwt_out** | **lwt_xmit** |
+|		**lwt_seg6local** | **sockops** | **sk_skb** | **sk_msg** | **lirc_mode2** |
+|		**cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
+|		**cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
+|	}
+
 
 DESCRIPTION
 ===========
@@ -64,8 +73,10 @@ DESCRIPTION
 
 		  Note: *FILE* must be located in *bpffs* mount.
 
-	**bpftool prog load** *OBJ* *FILE* [**dev** *NAME*]
+	**bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
 		  Load bpf program from binary *OBJ* and pin as *FILE*.
+		  **type** is optional, if not specified program type will be
+		  inferred from section names.
 		  If **dev** *NAME* is specified program will be loaded onto
 		  given networking device (offload).
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 238c2f80092a..caf8711993be 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -274,11 +274,17 @@ _bpftool()
                     fi
 
                     case $prev in
+                        type)
+                            COMPREPLY=( $( compgen -W "socket kprobe kretprobe classifier action tracepoint raw_tracepoint xdp perf_event cgroup/skb cgroup/sock cgroup/dev lwt_in lwt_out lwt_xmit lwt_seg6local sockops sk_skb sk_msg lirc_mode2 cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6 cgroup/post_bind4 cgroup/post_bind6" -- \
+                                                   "$cur" ) )
+                            return 0
+                            ;;
                         dev)
                             _sysfs_get_netdevs
                             return 0
                             ;;
                         *)
+                            _bpftool_once_attr 'type'
                             _bpftool_once_attr 'dev'
                             return 0
                             ;;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 21c74de7156f..98695585bbb6 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -688,6 +688,7 @@ static int do_load(int argc, char **argv)
 	const char *objfile, *pinfile;
 	struct bpf_object *obj;
 	int prog_fd;
+	int err;
 
 	if (!REQ_ARGS(2))
 		return -1;
@@ -695,7 +696,37 @@ static int do_load(int argc, char **argv)
 	pinfile = GET_ARG();
 
 	while (argc) {
-		if (is_prefix(*argv, "dev")) {
+		if (is_prefix(*argv, "type")) {
+			char *type;
+
+			NEXT_ARG();
+
+			if (attr.prog_type != BPF_PROG_TYPE_UNSPEC) {
+				p_err("program type already specified");
+				return -1;
+			}
+			if (!REQ_ARGS(1))
+				return -1;
+
+			/* Put a '/' at the end of type to appease libbpf */
+			type = malloc(strlen(*argv) + 2);
+			if (!type) {
+				p_err("mem alloc failed");
+				return -1;
+			}
+			*type = 0;
+			strcat(type, *argv);
+			strcat(type, "/");
+
+			err = libbpf_prog_type_by_name(type, &attr.prog_type,
+						       &attr.expected_attach_type);
+			free(type);
+			if (err < 0) {
+				p_err("unknown program type '%s'", *argv);
+				return err;
+			}
+			NEXT_ARG();
+		} else if (is_prefix(*argv, "dev")) {
 			NEXT_ARG();
 
 			if (attr.ifindex) {
@@ -713,7 +744,7 @@ static int do_load(int argc, char **argv)
 			}
 			NEXT_ARG();
 		} else {
-			p_err("expected no more arguments or 'dev', got: '%s'?",
+			p_err("expected no more arguments, 'type' or 'dev', got: '%s'?",
 			      *argv);
 			return -1;
 		}
@@ -753,10 +784,17 @@ static int do_help(int argc, char **argv)
 		"       %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
 		"       %s %s dump jited  PROG [{ file FILE | opcodes }]\n"
 		"       %s %s pin   PROG FILE\n"
-		"       %s %s load  OBJ  FILE [dev NAME]\n"
+		"       %s %s load  OBJ  FILE [type TYPE] [dev NAME]\n"
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_PROGRAM "\n"
+		"       TYPE := { socket | kprobe | kretprobe | classifier | action |\n"
+		"                 tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |\n"
+		"                 cgroup/sock | cgroup/dev | lwt_in | lwt_out | lwt_xmit |\n"
+		"                 lwt_seg6local | sockops | sk_skb | sk_msg | lirc_mode2 |\n"
+		"                 cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
+		"                 cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
+		"                 cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
 		"       " HELP_SPEC_OPTIONS "\n"
 		"",
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 07/13] tools: libbpf: recognize offload neutral maps
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (5 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 06/13] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-10 21:43 ` [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Add helper to libbpf for recognizing maps which should not have
ifindex set when program is loaded.  These maps only contain
host metadata and therefore are not marked for offload, e.g.
the perf event map.

Use this helper in bpf_prog_load_xattr().

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/lib/bpf/libbpf.c | 8 +++++++-
 tools/lib/bpf/libbpf.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 42f31eff5f3f..30992305f4c1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2154,6 +2154,11 @@ void *bpf_map__priv(struct bpf_map *map)
 	return map ? map->priv : ERR_PTR(-EINVAL);
 }
 
+bool bpf_map__is_offload_neutral(struct bpf_map *map)
+{
+	return map->def.type == BPF_MAP_TYPE_PERF_EVENT_ARRAY;
+}
+
 void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
 {
 	map->map_ifindex = ifindex;
@@ -2278,7 +2283,8 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 	}
 
 	bpf_map__for_each(map, obj) {
-		map->map_ifindex = attr->ifindex;
+		if (!bpf_map__is_offload_neutral(map))
+			map->map_ifindex = attr->ifindex;
 	}
 
 	if (!first_prog) {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d1ce5c828e2e..749acf58a5da 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -255,6 +255,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
 		      bpf_map_clear_priv_t clear_priv);
 void *bpf_map__priv(struct bpf_map *map);
+bool bpf_map__is_offload_neutral(struct bpf_map *map);
 void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
 int bpf_map__pin(struct bpf_map *map, const char *path);
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open()
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (6 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 07/13] tools: libbpf: recognize offload neutral maps Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-11  3:03   ` Andrey Ignatov
  2018-07-10 21:43 ` [PATCH bpf-next v3 09/13] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Similarly to bpf_prog_load() users of bpf_object__open() may need
to specify the expected program type.  Program type is needed at
open to avoid the kernel version check for program types which don't
require it.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
v3 (Andrey):
 - put file path into the attr struct.
---
 tools/lib/bpf/libbpf.c | 26 ++++++++++++++++++++------
 tools/lib/bpf/libbpf.h |  6 ++++++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 30992305f4c1..06cd534d2fba 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1520,15 +1520,26 @@ __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
 	return ERR_PTR(err);
 }
 
-struct bpf_object *bpf_object__open(const char *path)
+struct bpf_object *bpf_object__open_xattr(struct bpf_object_open_attr *attr)
 {
 	/* param validation */
-	if (!path)
+	if (!attr->file)
 		return NULL;
 
-	pr_debug("loading %s\n", path);
+	pr_debug("loading %s\n", attr->file);
+
+	return __bpf_object__open(attr->file, NULL, 0,
+				  bpf_prog_type__needs_kver(attr->prog_type));
+}
+
+struct bpf_object *bpf_object__open(const char *path)
+{
+	struct bpf_object_open_attr attr = {
+		.file		= path,
+		.prog_type	= BPF_PROG_TYPE_UNSPEC,
+	};
 
-	return __bpf_object__open(path, NULL, 0, true);
+	return bpf_object__open_xattr(&attr);
 }
 
 struct bpf_object *bpf_object__open_buffer(void *obj_buf,
@@ -2238,6 +2249,10 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
 int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 			struct bpf_object **pobj, int *prog_fd)
 {
+	struct bpf_object_open_attr open_attr = {
+		.file		= attr->file,
+		.prog_type	= attr->prog_type,
+	};
 	struct bpf_program *prog, *first_prog = NULL;
 	enum bpf_attach_type expected_attach_type;
 	enum bpf_prog_type prog_type;
@@ -2250,8 +2265,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 	if (!attr->file)
 		return -EINVAL;
 
-	obj = __bpf_object__open(attr->file, NULL, 0,
-				 bpf_prog_type__needs_kver(attr->prog_type));
+	obj = bpf_object__open_xattr(&open_attr);
 	if (IS_ERR_OR_NULL(obj))
 		return -ENOENT;
 
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 749acf58a5da..e911ad32d02e 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -66,7 +66,13 @@ void libbpf_set_print(libbpf_print_fn_t warn,
 /* Hide internal to user */
 struct bpf_object;
 
+struct bpf_object_open_attr {
+	const char *file;
+	enum bpf_prog_type prog_type;
+};
+
 struct bpf_object *bpf_object__open(const char *path);
+struct bpf_object *bpf_object__open_xattr(struct bpf_object_open_attr *attr);
 struct bpf_object *bpf_object__open_buffer(void *obj_buf,
 					   size_t obj_buf_sz,
 					   const char *name);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 09/13] tools: bpftool: reimplement bpf_prog_load() for prog load
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (7 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-10 21:43 ` [PATCH bpf-next v3 10/13] tools: libbpf: move library error code into a separate file Jakub Kicinski
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

bpf_prog_load() is a very useful helper but it doesn't give us full
flexibility of modifying the BPF objects before loading.  Open code
bpf_prog_load() in bpftool so we can add extra logic in following
commits.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/bpf/bpftool/prog.c | 61 +++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 98695585bbb6..2bdd5ecd1aad 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -43,6 +43,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include <linux/err.h>
+
 #include <bpf.h>
 #include <libbpf.h>
 
@@ -682,17 +684,20 @@ static int do_pin(int argc, char **argv)
 
 static int do_load(int argc, char **argv)
 {
-	struct bpf_prog_load_attr attr = {
+	enum bpf_attach_type expected_attach_type;
+	struct bpf_object_open_attr attr = {
 		.prog_type	= BPF_PROG_TYPE_UNSPEC,
 	};
-	const char *objfile, *pinfile;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
-	int prog_fd;
+	struct bpf_map *map;
+	const char *pinfile;
+	__u32 ifindex = 0;
 	int err;
 
 	if (!REQ_ARGS(2))
 		return -1;
-	objfile = GET_ARG();
+	attr.file = GET_ARG();
 	pinfile = GET_ARG();
 
 	while (argc) {
@@ -719,7 +724,7 @@ static int do_load(int argc, char **argv)
 			strcat(type, "/");
 
 			err = libbpf_prog_type_by_name(type, &attr.prog_type,
-						       &attr.expected_attach_type);
+						       &expected_attach_type);
 			free(type);
 			if (err < 0) {
 				p_err("unknown program type '%s'", *argv);
@@ -729,15 +734,15 @@ static int do_load(int argc, char **argv)
 		} else if (is_prefix(*argv, "dev")) {
 			NEXT_ARG();
 
-			if (attr.ifindex) {
+			if (ifindex) {
 				p_err("offload device already specified");
 				return -1;
 			}
 			if (!REQ_ARGS(1))
 				return -1;
 
-			attr.ifindex = if_nametoindex(*argv);
-			if (!attr.ifindex) {
+			ifindex = if_nametoindex(*argv);
+			if (!ifindex) {
 				p_err("unrecognized netdevice '%s': %s",
 				      *argv, strerror(errno));
 				return -1;
@@ -750,14 +755,44 @@ static int do_load(int argc, char **argv)
 		}
 	}
 
-	attr.file = objfile;
-
-	if (bpf_prog_load_xattr(&attr, &obj, &prog_fd)) {
-		p_err("failed to load program");
+	obj = bpf_object__open_xattr(&attr);
+	if (IS_ERR_OR_NULL(obj)) {
+		p_err("failed to open object file");
 		return -1;
 	}
 
-	if (do_pin_fd(prog_fd, pinfile))
+	prog = bpf_program__next(NULL, obj);
+	if (!prog) {
+		p_err("object file doesn't contain any bpf program");
+		goto err_close_obj;
+	}
+
+	bpf_program__set_ifindex(prog, ifindex);
+	if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
+		const char *sec_name = bpf_program__title(prog, false);
+
+		err = libbpf_prog_type_by_name(sec_name, &attr.prog_type,
+					       &expected_attach_type);
+		if (err < 0) {
+			p_err("failed to guess program type based on section name %s\n",
+			      sec_name);
+			goto err_close_obj;
+		}
+	}
+	bpf_program__set_type(prog, attr.prog_type);
+	bpf_program__set_expected_attach_type(prog, expected_attach_type);
+
+	bpf_map__for_each(map, obj)
+		if (!bpf_map__is_offload_neutral(map))
+			bpf_map__set_ifindex(map, ifindex);
+
+	err = bpf_object__load(obj);
+	if (err) {
+		p_err("failed to load object file");
+		goto err_close_obj;
+	}
+
+	if (do_pin_fd(bpf_program__fd(prog), pinfile))
 		goto err_close_obj;
 
 	if (json_output)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 10/13] tools: libbpf: move library error code into a separate file
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (8 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 09/13] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-10 21:43 ` [PATCH bpf-next v3 11/13] tools: bpf: make use of reallocarray Jakub Kicinski
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

libbpf_strerror() depends on XSI-compliant (POSIX) version of
strerror_r(), which prevents us from using GNU-extensions in
libbpf.c, like reallocarray() or dup3().  Move error printing
code into a separate file to allow it to continue using POSIX
strerror_r().

No functional changes.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/lib/bpf/Build          |  2 +-
 tools/lib/bpf/libbpf.c       | 48 -----------------------
 tools/lib/bpf/libbpf_errno.c | 74 ++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 49 deletions(-)
 create mode 100644 tools/lib/bpf/libbpf_errno.c

diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
index 6070e655042d..13a861135127 100644
--- a/tools/lib/bpf/Build
+++ b/tools/lib/bpf/Build
@@ -1 +1 @@
-libbpf-y := libbpf.o bpf.o nlattr.o btf.o
+libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 06cd534d2fba..f732237610e5 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -95,54 +95,6 @@ void libbpf_set_print(libbpf_print_fn_t warn,
 
 #define STRERR_BUFSIZE  128
 
-#define ERRNO_OFFSET(e)		((e) - __LIBBPF_ERRNO__START)
-#define ERRCODE_OFFSET(c)	ERRNO_OFFSET(LIBBPF_ERRNO__##c)
-#define NR_ERRNO	(__LIBBPF_ERRNO__END - __LIBBPF_ERRNO__START)
-
-static const char *libbpf_strerror_table[NR_ERRNO] = {
-	[ERRCODE_OFFSET(LIBELF)]	= "Something wrong in libelf",
-	[ERRCODE_OFFSET(FORMAT)]	= "BPF object format invalid",
-	[ERRCODE_OFFSET(KVERSION)]	= "'version' section incorrect or lost",
-	[ERRCODE_OFFSET(ENDIAN)]	= "Endian mismatch",
-	[ERRCODE_OFFSET(INTERNAL)]	= "Internal error in libbpf",
-	[ERRCODE_OFFSET(RELOC)]		= "Relocation failed",
-	[ERRCODE_OFFSET(VERIFY)]	= "Kernel verifier blocks program loading",
-	[ERRCODE_OFFSET(PROG2BIG)]	= "Program too big",
-	[ERRCODE_OFFSET(KVER)]		= "Incorrect kernel version",
-	[ERRCODE_OFFSET(PROGTYPE)]	= "Kernel doesn't support this program type",
-	[ERRCODE_OFFSET(WRNGPID)]	= "Wrong pid in netlink message",
-	[ERRCODE_OFFSET(INVSEQ)]	= "Invalid netlink sequence",
-};
-
-int libbpf_strerror(int err, char *buf, size_t size)
-{
-	if (!buf || !size)
-		return -1;
-
-	err = err > 0 ? err : -err;
-
-	if (err < __LIBBPF_ERRNO__START) {
-		int ret;
-
-		ret = strerror_r(err, buf, size);
-		buf[size - 1] = '\0';
-		return ret;
-	}
-
-	if (err < __LIBBPF_ERRNO__END) {
-		const char *msg;
-
-		msg = libbpf_strerror_table[ERRNO_OFFSET(err)];
-		snprintf(buf, size, "%s", msg);
-		buf[size - 1] = '\0';
-		return 0;
-	}
-
-	snprintf(buf, size, "Unknown libbpf error %d", err);
-	buf[size - 1] = '\0';
-	return -1;
-}
-
 #define CHECK_ERR(action, err, out) do {	\
 	err = action;			\
 	if (err)			\
diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c
new file mode 100644
index 000000000000..d9ba851bd7f9
--- /dev/null
+++ b/tools/lib/bpf/libbpf_errno.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: LGPL-2.1
+
+/*
+ * Copyright (C) 2013-2015 Alexei Starovoitov <ast@kernel.org>
+ * Copyright (C) 2015 Wang Nan <wangnan0@huawei.com>
+ * Copyright (C) 2015 Huawei Inc.
+ * Copyright (C) 2017 Nicira, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License (not later!)
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not,  see <http://www.gnu.org/licenses>
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "libbpf.h"
+
+#define ERRNO_OFFSET(e)		((e) - __LIBBPF_ERRNO__START)
+#define ERRCODE_OFFSET(c)	ERRNO_OFFSET(LIBBPF_ERRNO__##c)
+#define NR_ERRNO	(__LIBBPF_ERRNO__END - __LIBBPF_ERRNO__START)
+
+static const char *libbpf_strerror_table[NR_ERRNO] = {
+	[ERRCODE_OFFSET(LIBELF)]	= "Something wrong in libelf",
+	[ERRCODE_OFFSET(FORMAT)]	= "BPF object format invalid",
+	[ERRCODE_OFFSET(KVERSION)]	= "'version' section incorrect or lost",
+	[ERRCODE_OFFSET(ENDIAN)]	= "Endian mismatch",
+	[ERRCODE_OFFSET(INTERNAL)]	= "Internal error in libbpf",
+	[ERRCODE_OFFSET(RELOC)]		= "Relocation failed",
+	[ERRCODE_OFFSET(VERIFY)]	= "Kernel verifier blocks program loading",
+	[ERRCODE_OFFSET(PROG2BIG)]	= "Program too big",
+	[ERRCODE_OFFSET(KVER)]		= "Incorrect kernel version",
+	[ERRCODE_OFFSET(PROGTYPE)]	= "Kernel doesn't support this program type",
+	[ERRCODE_OFFSET(WRNGPID)]	= "Wrong pid in netlink message",
+	[ERRCODE_OFFSET(INVSEQ)]	= "Invalid netlink sequence",
+};
+
+int libbpf_strerror(int err, char *buf, size_t size)
+{
+	if (!buf || !size)
+		return -1;
+
+	err = err > 0 ? err : -err;
+
+	if (err < __LIBBPF_ERRNO__START) {
+		int ret;
+
+		ret = strerror_r(err, buf, size);
+		buf[size - 1] = '\0';
+		return ret;
+	}
+
+	if (err < __LIBBPF_ERRNO__END) {
+		const char *msg;
+
+		msg = libbpf_strerror_table[ERRNO_OFFSET(err)];
+		snprintf(buf, size, "%s", msg);
+		buf[size - 1] = '\0';
+		return 0;
+	}
+
+	snprintf(buf, size, "Unknown libbpf error %d", err);
+	buf[size - 1] = '\0';
+	return -1;
+}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 11/13] tools: bpf: make use of reallocarray
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (9 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 10/13] tools: libbpf: move library error code into a separate file Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-13 23:53   ` [bpf-next,v3,11/13] " Guenter Roeck
  2018-07-10 21:43 ` [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse Jakub Kicinski
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

reallocarray() is a safer variant of realloc which checks for
multiplication overflow in case of array allocation.  Since it's
not available in Glibc < 2.26 import kernel's overflow.h and
add a static inline implementation when needed.  Use feature
detection to probe for existence of reallocarray.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jiong Wang <jiong.wang@netronome.com>
---
 tools/bpf/bpftool/Makefile              |   6 +-
 tools/bpf/bpftool/main.h                |   1 +
 tools/bpf/bpftool/xlated_dumper.c       |   6 +-
 tools/build/feature/Makefile            |   4 +
 tools/build/feature/test-reallocarray.c |   8 +
 tools/include/linux/compiler-gcc.h      |   4 +
 tools/include/linux/overflow.h          | 278 ++++++++++++++++++++++++
 tools/include/tools/libc_compat.h       |  20 ++
 tools/lib/bpf/Makefile                  |   6 +-
 tools/lib/bpf/libbpf.c                  |  10 +-
 10 files changed, 334 insertions(+), 9 deletions(-)
 create mode 100644 tools/build/feature/test-reallocarray.c
 create mode 100644 tools/include/linux/overflow.h
 create mode 100644 tools/include/tools/libc_compat.h

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 0911b00b25cc..6c4830e18879 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -52,7 +52,7 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray
 FEATURE_DISPLAY = libbfd disassembler-four-args
 
 check_feat := 1
@@ -75,6 +75,10 @@ ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
+ifeq ($(feature-reallocarray), 0)
+CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
+endif
+
 include $(wildcard $(OUTPUT)*.d)
 
 all: $(OUTPUT)bpftool
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 15b6c49ae533..1e02e4031693 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -42,6 +42,7 @@
 #include <linux/compiler.h>
 #include <linux/kernel.h>
 #include <linux/hashtable.h>
+#include <tools/libc_compat.h>
 
 #include "json_writer.h"
 
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index b97f1da60dd1..3284759df98a 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -35,6 +35,7 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#define _GNU_SOURCE
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -66,9 +67,8 @@ void kernel_syms_load(struct dump_data *dd)
 	while (!feof(fp)) {
 		if (!fgets(buff, sizeof(buff), fp))
 			break;
-		tmp = realloc(dd->sym_mapping,
-			      (dd->sym_count + 1) *
-			      sizeof(*dd->sym_mapping));
+		tmp = reallocarray(dd->sym_mapping, dd->sym_count + 1,
+				   sizeof(*dd->sym_mapping));
 		if (!tmp) {
 out:
 			free(dd->sym_mapping);
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);
+}
diff --git a/tools/include/linux/compiler-gcc.h b/tools/include/linux/compiler-gcc.h
index 70fe61295733..0d35f18006a1 100644
--- a/tools/include/linux/compiler-gcc.h
+++ b/tools/include/linux/compiler-gcc.h
@@ -36,3 +36,7 @@
 #endif
 #define __printf(a, b)	__attribute__((format(printf, a, b)))
 #define __scanf(a, b)	__attribute__((format(scanf, a, b)))
+
+#if GCC_VERSION >= 50100
+#define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
+#endif
diff --git a/tools/include/linux/overflow.h b/tools/include/linux/overflow.h
new file mode 100644
index 000000000000..8712ff70995f
--- /dev/null
+++ b/tools/include/linux/overflow.h
@@ -0,0 +1,278 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+#ifndef __LINUX_OVERFLOW_H
+#define __LINUX_OVERFLOW_H
+
+#include <linux/compiler.h>
+
+/*
+ * In the fallback code below, we need to compute the minimum and
+ * maximum values representable in a given type. These macros may also
+ * be useful elsewhere, so we provide them outside the
+ * COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW block.
+ *
+ * It would seem more obvious to do something like
+ *
+ * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
+ * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
+ *
+ * Unfortunately, the middle expressions, strictly speaking, have
+ * undefined behaviour, and at least some versions of gcc warn about
+ * the type_max expression (but not if -fsanitize=undefined is in
+ * effect; in that case, the warning is deferred to runtime...).
+ *
+ * The slightly excessive casting in type_min is to make sure the
+ * macros also produce sensible values for the exotic type _Bool. [The
+ * overflow checkers only almost work for _Bool, but that's
+ * a-feature-not-a-bug, since people shouldn't be doing arithmetic on
+ * _Bools. Besides, the gcc builtins don't allow _Bool* as third
+ * argument.]
+ *
+ * Idea stolen from
+ * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
+ * credit to Christian Biere.
+ */
+#define is_signed_type(type)       (((type)(-1)) < (type)1)
+#define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
+#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
+#define type_min(T) ((T)((T)-type_max(T)-(T)1))
+
+
+#ifdef COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW
+/*
+ * For simplicity and code hygiene, the fallback code below insists on
+ * a, b and *d having the same type (similar to the min() and max()
+ * macros), whereas gcc's type-generic overflow checkers accept
+ * different types. Hence we don't just make check_add_overflow an
+ * alias for __builtin_add_overflow, but add type checks similar to
+ * below.
+ */
+#define check_add_overflow(a, b, d) ({		\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	__builtin_add_overflow(__a, __b, __d);	\
+})
+
+#define check_sub_overflow(a, b, d) ({		\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	__builtin_sub_overflow(__a, __b, __d);	\
+})
+
+#define check_mul_overflow(a, b, d) ({		\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	__builtin_mul_overflow(__a, __b, __d);	\
+})
+
+#else
+
+
+/* Checking for unsigned overflow is relatively easy without causing UB. */
+#define __unsigned_add_overflow(a, b, d) ({	\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	*__d = __a + __b;			\
+	*__d < __a;				\
+})
+#define __unsigned_sub_overflow(a, b, d) ({	\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	*__d = __a - __b;			\
+	__a < __b;				\
+})
+/*
+ * If one of a or b is a compile-time constant, this avoids a division.
+ */
+#define __unsigned_mul_overflow(a, b, d) ({		\
+	typeof(a) __a = (a);				\
+	typeof(b) __b = (b);				\
+	typeof(d) __d = (d);				\
+	(void) (&__a == &__b);				\
+	(void) (&__a == __d);				\
+	*__d = __a * __b;				\
+	__builtin_constant_p(__b) ?			\
+	  __b > 0 && __a > type_max(typeof(__a)) / __b : \
+	  __a > 0 && __b > type_max(typeof(__b)) / __a;	 \
+})
+
+/*
+ * For signed types, detecting overflow is much harder, especially if
+ * we want to avoid UB. But the interface of these macros is such that
+ * we must provide a result in *d, and in fact we must produce the
+ * result promised by gcc's builtins, which is simply the possibly
+ * wrapped-around value. Fortunately, we can just formally do the
+ * operations in the widest relevant unsigned type (u64) and then
+ * truncate the result - gcc is smart enough to generate the same code
+ * with and without the (u64) casts.
+ */
+
+/*
+ * Adding two signed integers can overflow only if they have the same
+ * sign, and overflow has happened iff the result has the opposite
+ * sign.
+ */
+#define __signed_add_overflow(a, b, d) ({	\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	*__d = (u64)__a + (u64)__b;		\
+	(((~(__a ^ __b)) & (*__d ^ __a))	\
+		& type_min(typeof(__a))) != 0;	\
+})
+
+/*
+ * Subtraction is similar, except that overflow can now happen only
+ * when the signs are opposite. In this case, overflow has happened if
+ * the result has the opposite sign of a.
+ */
+#define __signed_sub_overflow(a, b, d) ({	\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	typeof(d) __d = (d);			\
+	(void) (&__a == &__b);			\
+	(void) (&__a == __d);			\
+	*__d = (u64)__a - (u64)__b;		\
+	((((__a ^ __b)) & (*__d ^ __a))		\
+		& type_min(typeof(__a))) != 0;	\
+})
+
+/*
+ * Signed multiplication is rather hard. gcc always follows C99, so
+ * division is truncated towards 0. This means that we can write the
+ * overflow check like this:
+ *
+ * (a > 0 && (b > MAX/a || b < MIN/a)) ||
+ * (a < -1 && (b > MIN/a || b < MAX/a) ||
+ * (a == -1 && b == MIN)
+ *
+ * The redundant casts of -1 are to silence an annoying -Wtype-limits
+ * (included in -Wextra) warning: When the type is u8 or u16, the
+ * __b_c_e in check_mul_overflow obviously selects
+ * __unsigned_mul_overflow, but unfortunately gcc still parses this
+ * code and warns about the limited range of __b.
+ */
+
+#define __signed_mul_overflow(a, b, d) ({				\
+	typeof(a) __a = (a);						\
+	typeof(b) __b = (b);						\
+	typeof(d) __d = (d);						\
+	typeof(a) __tmax = type_max(typeof(a));				\
+	typeof(a) __tmin = type_min(typeof(a));				\
+	(void) (&__a == &__b);						\
+	(void) (&__a == __d);						\
+	*__d = (u64)__a * (u64)__b;					\
+	(__b > 0   && (__a > __tmax/__b || __a < __tmin/__b)) ||	\
+	(__b < (typeof(__b))-1  && (__a > __tmin/__b || __a < __tmax/__b)) || \
+	(__b == (typeof(__b))-1 && __a == __tmin);			\
+})
+
+
+#define check_add_overflow(a, b, d)					\
+	__builtin_choose_expr(is_signed_type(typeof(a)),		\
+			__signed_add_overflow(a, b, d),			\
+			__unsigned_add_overflow(a, b, d))
+
+#define check_sub_overflow(a, b, d)					\
+	__builtin_choose_expr(is_signed_type(typeof(a)),		\
+			__signed_sub_overflow(a, b, d),			\
+			__unsigned_sub_overflow(a, b, d))
+
+#define check_mul_overflow(a, b, d)					\
+	__builtin_choose_expr(is_signed_type(typeof(a)),		\
+			__signed_mul_overflow(a, b, d),			\
+			__unsigned_mul_overflow(a, b, d))
+
+
+#endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
+
+/**
+ * array_size() - Calculate size of 2-dimensional array.
+ *
+ * @a: dimension one
+ * @b: dimension two
+ *
+ * Calculates size of 2-dimensional array: @a * @b.
+ *
+ * Returns: number of bytes needed to represent the array or SIZE_MAX on
+ * overflow.
+ */
+static inline __must_check size_t array_size(size_t a, size_t b)
+{
+	size_t bytes;
+
+	if (check_mul_overflow(a, b, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+/**
+ * array3_size() - Calculate size of 3-dimensional array.
+ *
+ * @a: dimension one
+ * @b: dimension two
+ * @c: dimension three
+ *
+ * Calculates size of 3-dimensional array: @a * @b * @c.
+ *
+ * Returns: number of bytes needed to represent the array or SIZE_MAX on
+ * overflow.
+ */
+static inline __must_check size_t array3_size(size_t a, size_t b, size_t c)
+{
+	size_t bytes;
+
+	if (check_mul_overflow(a, b, &bytes))
+		return SIZE_MAX;
+	if (check_mul_overflow(bytes, c, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+static inline __must_check size_t __ab_c_size(size_t n, size_t size, size_t c)
+{
+	size_t bytes;
+
+	if (check_mul_overflow(n, size, &bytes))
+		return SIZE_MAX;
+	if (check_add_overflow(bytes, c, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+/**
+ * struct_size() - Calculate size of structure with trailing array.
+ * @p: Pointer to the structure.
+ * @member: Name of the array member.
+ * @n: Number of elements in the array.
+ *
+ * Calculates size of memory needed for structure @p followed by an
+ * array of @n @member elements.
+ *
+ * Return: number of bytes needed or SIZE_MAX on overflow.
+ */
+#define struct_size(p, member, n)					\
+	__ab_c_size(n,							\
+		    sizeof(*(p)->member) + __must_be_array((p)->member),\
+		    sizeof(*(p)))
+
+#endif /* __LINUX_OVERFLOW_H */
diff --git a/tools/include/tools/libc_compat.h b/tools/include/tools/libc_compat.h
new file mode 100644
index 000000000000..664ced8cb1b0
--- /dev/null
+++ b/tools/include/tools/libc_compat.h
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2018 Netronome Systems, Inc. */
+
+#ifndef __TOOLS_LIBC_COMPAT_H
+#define __TOOLS_LIBC_COMPAT_H
+
+#include <stdlib.h>
+#include <linux/overflow.h>
+
+#ifdef COMPAT_NEED_REALLOCARRAY
+static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
+{
+	size_t bytes;
+
+	if (unlikely(check_mul_overflow(nmemb, size, &bytes)))
+		return NULL;
+	return realloc(ptr, bytes);
+}
+#endif
+#endif
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 5390e7725e43..7a8e4c98ef1a 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -66,7 +66,7 @@ ifndef VERBOSE
 endif
 
 FEATURE_USER = .libbpf
-FEATURE_TESTS = libelf libelf-getphdrnum libelf-mmap bpf
+FEATURE_TESTS = libelf libelf-getphdrnum libelf-mmap bpf reallocarray
 FEATURE_DISPLAY = libelf bpf
 
 INCLUDES = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(ARCH)/include/uapi -I$(srctree)/tools/include/uapi -I$(srctree)/tools/perf
@@ -120,6 +120,10 @@ ifeq ($(feature-libelf-getphdrnum), 1)
   override CFLAGS += -DHAVE_ELF_GETPHDRNUM_SUPPORT
 endif
 
+ifeq ($(feature-reallocarray), 0)
+  override CFLAGS += -DCOMPAT_NEED_REALLOCARRAY
+endif
+
 # Append required CFLAGS
 override CFLAGS += $(EXTRA_WARNINGS)
 override CFLAGS += -Werror -Wall
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f732237610e5..fd2c4433863d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -22,6 +22,7 @@
  * License along with this program; if not,  see <http://www.gnu.org/licenses>
  */
 
+#define _GNU_SOURCE
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
@@ -41,6 +42,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
+#include <tools/libc_compat.h>
 #include <libelf.h>
 #include <gelf.h>
 
@@ -321,7 +323,7 @@ bpf_object__add_program(struct bpf_object *obj, void *data, size_t size,
 	progs = obj->programs;
 	nr_progs = obj->nr_programs;
 
-	progs = realloc(progs, sizeof(progs[0]) * (nr_progs + 1));
+	progs = reallocarray(progs, nr_progs + 1, sizeof(progs[0]));
 	if (!progs) {
 		/*
 		 * In this case the original obj->programs
@@ -822,8 +824,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 				continue;
 			}
 
-			reloc = realloc(reloc,
-					sizeof(*obj->efile.reloc) * nr_reloc);
+			reloc = reallocarray(reloc, nr_reloc,
+					     sizeof(*obj->efile.reloc));
 			if (!reloc) {
 				pr_warning("realloc failed\n");
 				err = -ENOMEM;
@@ -1115,7 +1117,7 @@ bpf_program__reloc_text(struct bpf_program *prog, struct bpf_object *obj,
 			return -LIBBPF_ERRNO__RELOC;
 		}
 		new_cnt = prog->insns_cnt + text->insns_cnt;
-		new_insn = realloc(prog->insns, new_cnt * sizeof(*insn));
+		new_insn = reallocarray(prog->insns, new_cnt, sizeof(*insn));
 		if (!new_insn) {
 			pr_warning("oom in prog realloc\n");
 			return -ENOMEM;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (10 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 11/13] tools: bpf: make use of reallocarray Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-11  3:45   ` Andrey Ignatov
  2018-07-10 21:43 ` [PATCH bpf-next v3 13/13] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
  2018-07-11 20:18 ` [PATCH bpf-next v3 00/13] tools: bpf: extend " Daniel Borkmann
  13 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

More advanced applications may want to only replace programs without
destroying associated maps.  Allow libbpf users to achieve that.
Instead of always creating all of the maps at load time, expose to
users an API to reconstruct the map object from already existing
map.

The map parameters are read from the kernel and replace the parameters
of the ELF map.  libbpf does not restrict the map replacement, i.e.
the reused map does not have to be compatible with the ELF map
definition.  We relay on the verifier for checking the compatibility
between maps and programs.  The ELF map definition is completely
overwritten by the information read from the kernel, to make sure
libbpf's view of map object corresponds to the actual map.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
v3 (Andrey):
  - use dup3();
  - close existing fd;
  - handle errors better;
  - depend on fd value to determine pre-set.
---
 tools/lib/bpf/libbpf.c | 53 ++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |  1 +
 2 files changed, 54 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fd2c4433863d..955f8eafbf41 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1035,6 +1035,53 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf)
 	return 0;
 }
 
+int bpf_map__reuse_fd(struct bpf_map *map, int fd)
+{
+	struct bpf_map_info info = {};
+	__u32 len = sizeof(info);
+	int new_fd, err;
+	char *new_name;
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &len);
+	if (err)
+		return err;
+
+	new_name = strdup(info.name);
+	if (!new_name)
+		return -errno;
+
+	new_fd = open("/", O_RDONLY | O_CLOEXEC);
+	if (new_fd < 0)
+		goto err_free_new_name;
+
+	new_fd = dup3(fd, new_fd, O_CLOEXEC);
+	if (new_fd < 0)
+		goto err_close_new_fd;
+
+	err = zclose(map->fd);
+	if (err)
+		goto err_close_new_fd;
+	free(map->name);
+
+	map->fd = new_fd;
+	map->name = new_name;
+	map->def.type = info.type;
+	map->def.key_size = info.key_size;
+	map->def.value_size = info.value_size;
+	map->def.max_entries = info.max_entries;
+	map->def.map_flags = info.map_flags;
+	map->btf_key_type_id = info.btf_key_type_id;
+	map->btf_value_type_id = info.btf_value_type_id;
+
+	return 0;
+
+err_close_new_fd:
+	close(new_fd);
+err_free_new_name:
+	free(new_name);
+	return -errno;
+}
+
 static int
 bpf_object__create_maps(struct bpf_object *obj)
 {
@@ -1047,6 +1094,12 @@ bpf_object__create_maps(struct bpf_object *obj)
 		struct bpf_map_def *def = &map->def;
 		int *pfd = &map->fd;
 
+		if (map->fd >= 0) {
+			pr_debug("skip map create (preset) %s: fd=%d\n",
+				 map->name, map->fd);
+			continue;
+		}
+
 		create_attr.name = map->name;
 		create_attr.map_ifindex = map->map_ifindex;
 		create_attr.map_type = def->type;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e911ad32d02e..1f8fc2060460 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -261,6 +261,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
 		      bpf_map_clear_priv_t clear_priv);
 void *bpf_map__priv(struct bpf_map *map);
+int bpf_map__reuse_fd(struct bpf_map *map, int fd);
 bool bpf_map__is_offload_neutral(struct bpf_map *map);
 void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
 int bpf_map__pin(struct bpf_map *map, const char *path);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH bpf-next v3 13/13] tools: bpftool: allow reuse of maps with bpftool prog load
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (11 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse Jakub Kicinski
@ 2018-07-10 21:43 ` Jakub Kicinski
  2018-07-11 20:18 ` [PATCH bpf-next v3 00/13] tools: bpf: extend " Daniel Borkmann
  13 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-10 21:43 UTC (permalink / raw)
  To: alexei.starovoitov, daniel, Andrey Ignatov
  Cc: oss-drivers, netdev, Jakub Kicinski

Add map parameter to prog load which will allow reuse of existing
maps instead of creating new ones.

We need feature detection and compat code for reallocarray, since
it's not available in many libc versions.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 .../bpftool/Documentation/bpftool-prog.rst    |  20 ++-
 tools/bpf/bpftool/bash-completion/bpftool     |  67 +++++++-
 tools/bpf/bpftool/main.h                      |   3 +
 tools/bpf/bpftool/map.c                       |   4 +-
 tools/bpf/bpftool/prog.c                      | 148 ++++++++++++++++--
 5 files changed, 219 insertions(+), 23 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index e53e1ad2caf0..64156a16d530 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -24,9 +24,10 @@ MAP COMMANDS
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** | **visual**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog pin** *PROG* *FILE*
-|	**bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
+|	**bpftool** **prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
 |	**bpftool** **prog help**
 |
+|	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
 |	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
 |	*TYPE* := {
 |		**socket** | **kprobe** | **kretprobe** | **classifier** | **action** |
@@ -73,10 +74,17 @@ DESCRIPTION
 
 		  Note: *FILE* must be located in *bpffs* mount.
 
-	**bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**dev** *NAME*]
+	**bpftool prog load** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
 		  Load bpf program from binary *OBJ* and pin as *FILE*.
 		  **type** is optional, if not specified program type will be
 		  inferred from section names.
+		  By default bpftool will create new maps as declared in the ELF
+		  object being loaded.  **map** parameter allows for the reuse
+		  of existing maps.  It can be specified multiple times, each
+		  time for a different map.  *IDX* refers to index of the map
+		  to be replaced in the ELF file counting from 0, while *NAME*
+		  allows to replace a map by name.  *MAP* specifies the map to
+		  use, referring to it by **id** or through a **pinned** file.
 		  If **dev** *NAME* is specified program will be loaded onto
 		  given networking device (offload).
 
@@ -172,6 +180,14 @@ EXAMPLES
     mov    %rbx,0x0(%rbp)
     48 89 5d 00
 
+|
+| **# bpftool prog load xdp1_kern.o /sys/fs/bpf/xdp1 type xdp map name rxcnt id 7**
+| **# bpftool prog show pinned /sys/fs/bpf/xdp1**
+|   9: xdp  name xdp_prog1  tag 539ec6ce11b52f98  gpl
+|	loaded_at 2018-06-25T16:17:31-0700  uid 0
+|	xlated 488B  jited 336B  memlock 4096B  map_ids 7
+| **# rm /sys/fs/bpf/xdp1**
+|
 
 SEE ALSO
 ========
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index caf8711993be..598066c40191 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -99,6 +99,29 @@ _bpftool_get_prog_tags()
         command sed -n 's/.*"tag": "\(.*\)",$/\1/p' )" -- "$cur" ) )
 }
 
+_bpftool_get_obj_map_names()
+{
+    local obj
+
+    obj=$1
+
+    maps=$(objdump -j maps -t $obj 2>/dev/null | \
+        command awk '/g     . maps/ {print $NF}')
+
+    COMPREPLY+=( $( compgen -W "$maps" -- "$cur" ) )
+}
+
+_bpftool_get_obj_map_idxs()
+{
+    local obj
+
+    obj=$1
+
+    nmaps=$(objdump -j maps -t $obj 2>/dev/null | grep -c 'g     . maps')
+
+    COMPREPLY+=( $( compgen -W "$(seq 0 $((nmaps - 1)))" -- "$cur" ) )
+}
+
 _sysfs_get_netdevs()
 {
     COMPREPLY+=( $( compgen -W "$( ls /sys/class/net 2>/dev/null )" -- \
@@ -220,12 +243,14 @@ _bpftool()
     # Completion depends on object and command in use
     case $object in
         prog)
-            case $prev in
-                id)
-                    _bpftool_get_prog_ids
-                    return 0
-                    ;;
-            esac
+            if [[ $command != "load" ]]; then
+                case $prev in
+                    id)
+                        _bpftool_get_prog_ids
+                        return 0
+                        ;;
+                esac
+            fi
 
             local PROG_TYPE='id pinned tag'
             case $command in
@@ -268,22 +293,52 @@ _bpftool()
                     return 0
                     ;;
                 load)
+                    local obj
+
                     if [[ ${#words[@]} -lt 6 ]]; then
                         _filedir
                         return 0
                     fi
 
+                    obj=${words[3]}
+
+                    if [[ ${words[-4]} == "map" ]]; then
+                        COMPREPLY=( $( compgen -W "id pinned" -- "$cur" ) )
+                        return 0
+                    fi
+                    if [[ ${words[-3]} == "map" ]]; then
+                        if [[ ${words[-2]} == "idx" ]]; then
+                            _bpftool_get_obj_map_idxs $obj
+                        elif [[ ${words[-2]} == "name" ]]; then
+                            _bpftool_get_obj_map_names $obj
+                        fi
+                        return 0
+                    fi
+                    if [[ ${words[-2]} == "map" ]]; then
+                        COMPREPLY=( $( compgen -W "idx name" -- "$cur" ) )
+                        return 0
+                    fi
+
                     case $prev in
                         type)
                             COMPREPLY=( $( compgen -W "socket kprobe kretprobe classifier action tracepoint raw_tracepoint xdp perf_event cgroup/skb cgroup/sock cgroup/dev lwt_in lwt_out lwt_xmit lwt_seg6local sockops sk_skb sk_msg lirc_mode2 cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6 cgroup/post_bind4 cgroup/post_bind6" -- \
                                                    "$cur" ) )
                             return 0
                             ;;
+                        id)
+                            _bpftool_get_map_ids
+                            return 0
+                            ;;
+                        pinned)
+                            _filedir
+                            return 0
+                            ;;
                         dev)
                             _sysfs_get_netdevs
                             return 0
                             ;;
                         *)
+                            COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
                             _bpftool_once_attr 'type'
                             _bpftool_once_attr 'dev'
                             return 0
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 1e02e4031693..41004bb2644a 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -75,6 +75,8 @@
 	"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
 #define HELP_SPEC_OPTIONS						\
 	"OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} }"
+#define HELP_SPEC_MAP							\
+	"MAP := { id MAP_ID | pinned FILE }"
 
 enum bpf_obj_type {
 	BPF_OBJ_UNKNOWN,
@@ -136,6 +138,7 @@ int do_cgroup(int argc, char **arg);
 int do_perf(int argc, char **arg);
 
 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);
 
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 5989e1575ae4..e2baec1122fb 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -93,7 +93,7 @@ static void *alloc_value(struct bpf_map_info *info)
 		return malloc(info->value_size);
 }
 
-static int map_parse_fd(int *argc, char ***argv)
+int map_parse_fd(int *argc, char ***argv)
 {
 	int fd;
 
@@ -824,7 +824,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s event_pipe MAP [cpu N index M]\n"
 		"       %s %s help\n"
 		"\n"
-		"       MAP := { id MAP_ID | pinned FILE }\n"
+		"       " HELP_SPEC_MAP "\n"
 		"       DATA := { [hex] BYTES }\n"
 		"       " HELP_SPEC_PROGRAM "\n"
 		"       VALUE := { DATA | MAP | PROG }\n"
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 2bdd5ecd1aad..dce960d22106 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -31,6 +31,7 @@
  * SOFTWARE.
  */
 
+#define _GNU_SOURCE
 #include <errno.h>
 #include <fcntl.h>
 #include <stdarg.h>
@@ -682,18 +683,34 @@ static int do_pin(int argc, char **argv)
 	return err;
 }
 
+struct map_replace {
+	int idx;
+	int fd;
+	char *name;
+};
+
+int map_replace_compar(const void *p1, const void *p2)
+{
+	const struct map_replace *a = p1, *b = p2;
+
+	return a->idx - b->idx;
+}
+
 static int do_load(int argc, char **argv)
 {
 	enum bpf_attach_type expected_attach_type;
 	struct bpf_object_open_attr attr = {
 		.prog_type	= BPF_PROG_TYPE_UNSPEC,
 	};
+	struct map_replace *map_replace = NULL;
+	unsigned int old_map_fds = 0;
 	struct bpf_program *prog;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	const char *pinfile;
+	unsigned int i, j;
 	__u32 ifindex = 0;
-	int err;
+	int idx, err;
 
 	if (!REQ_ARGS(2))
 		return -1;
@@ -708,16 +725,16 @@ static int do_load(int argc, char **argv)
 
 			if (attr.prog_type != BPF_PROG_TYPE_UNSPEC) {
 				p_err("program type already specified");
-				return -1;
+				goto err_free_reuse_maps;
 			}
 			if (!REQ_ARGS(1))
-				return -1;
+				goto err_free_reuse_maps;
 
 			/* Put a '/' at the end of type to appease libbpf */
 			type = malloc(strlen(*argv) + 2);
 			if (!type) {
 				p_err("mem alloc failed");
-				return -1;
+				goto err_free_reuse_maps;
 			}
 			*type = 0;
 			strcat(type, *argv);
@@ -728,37 +745,81 @@ static int do_load(int argc, char **argv)
 			free(type);
 			if (err < 0) {
 				p_err("unknown program type '%s'", *argv);
-				return err;
+				goto err_free_reuse_maps;
 			}
 			NEXT_ARG();
+		} else if (is_prefix(*argv, "map")) {
+			char *endptr, *name;
+			int fd;
+
+			NEXT_ARG();
+
+			if (!REQ_ARGS(4))
+				goto err_free_reuse_maps;
+
+			if (is_prefix(*argv, "idx")) {
+				NEXT_ARG();
+
+				idx = strtoul(*argv, &endptr, 0);
+				if (*endptr) {
+					p_err("can't parse %s as IDX", *argv);
+					goto err_free_reuse_maps;
+				}
+				name = NULL;
+			} else if (is_prefix(*argv, "name")) {
+				NEXT_ARG();
+
+				name = *argv;
+				idx = -1;
+			} else {
+				p_err("expected 'idx' or 'name', got: '%s'?",
+				      *argv);
+				goto err_free_reuse_maps;
+			}
+			NEXT_ARG();
+
+			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;
+			}
+			map_replace[old_map_fds].idx = idx;
+			map_replace[old_map_fds].name = name;
+			map_replace[old_map_fds].fd = fd;
+			old_map_fds++;
 		} else if (is_prefix(*argv, "dev")) {
 			NEXT_ARG();
 
 			if (ifindex) {
 				p_err("offload device already specified");
-				return -1;
+				goto err_free_reuse_maps;
 			}
 			if (!REQ_ARGS(1))
-				return -1;
+				goto err_free_reuse_maps;
 
 			ifindex = if_nametoindex(*argv);
 			if (!ifindex) {
 				p_err("unrecognized netdevice '%s': %s",
 				      *argv, strerror(errno));
-				return -1;
+				goto err_free_reuse_maps;
 			}
 			NEXT_ARG();
 		} else {
-			p_err("expected no more arguments, 'type' or 'dev', got: '%s'?",
+			p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
 			      *argv);
-			return -1;
+			goto err_free_reuse_maps;
 		}
 	}
 
 	obj = bpf_object__open_xattr(&attr);
 	if (IS_ERR_OR_NULL(obj)) {
 		p_err("failed to open object file");
-		return -1;
+		goto err_free_reuse_maps;
 	}
 
 	prog = bpf_program__next(NULL, obj);
@@ -782,10 +843,62 @@ static int do_load(int argc, char **argv)
 	bpf_program__set_type(prog, attr.prog_type);
 	bpf_program__set_expected_attach_type(prog, expected_attach_type);
 
-	bpf_map__for_each(map, obj)
+	qsort(map_replace, old_map_fds, sizeof(*map_replace),
+	      map_replace_compar);
+
+	/* After the sort maps by name will be first on the list, because they
+	 * have idx == -1.  Resolve them.
+	 */
+	j = 0;
+	while (j < old_map_fds && map_replace[j].name) {
+		i = 0;
+		bpf_map__for_each(map, obj) {
+			if (!strcmp(bpf_map__name(map), map_replace[j].name)) {
+				map_replace[j].idx = i;
+				break;
+			}
+			i++;
+		}
+		if (map_replace[j].idx == -1) {
+			p_err("unable to find map '%s'", map_replace[j].name);
+			goto err_close_obj;
+		}
+		j++;
+	}
+	/* Resort if any names were resolved */
+	if (j)
+		qsort(map_replace, old_map_fds, sizeof(*map_replace),
+		      map_replace_compar);
+
+	/* Set ifindex and name reuse */
+	j = 0;
+	idx = 0;
+	bpf_map__for_each(map, obj) {
 		if (!bpf_map__is_offload_neutral(map))
 			bpf_map__set_ifindex(map, ifindex);
 
+		if (j < old_map_fds && idx == map_replace[j].idx) {
+			err = bpf_map__reuse_fd(map, map_replace[j++].fd);
+			if (err) {
+				p_err("unable to set up map reuse: %d", err);
+				goto err_close_obj;
+			}
+
+			/* Next reuse wants to apply to the same map */
+			if (j < old_map_fds && map_replace[j].idx == idx) {
+				p_err("replacement for map idx %d specified more than once",
+				      idx);
+				goto err_close_obj;
+			}
+		}
+
+		idx++;
+	}
+	if (j < old_map_fds) {
+		p_err("map idx '%d' not used", map_replace[j].idx);
+		goto err_close_obj;
+	}
+
 	err = bpf_object__load(obj);
 	if (err) {
 		p_err("failed to load object file");
@@ -799,11 +912,18 @@ static int do_load(int argc, char **argv)
 		jsonw_null(json_wtr);
 
 	bpf_object__close(obj);
+	for (i = 0; i < old_map_fds; i++)
+		close(map_replace[i].fd);
+	free(map_replace);
 
 	return 0;
 
 err_close_obj:
 	bpf_object__close(obj);
+err_free_reuse_maps:
+	for (i = 0; i < old_map_fds; i++)
+		close(map_replace[i].fd);
+	free(map_replace);
 	return -1;
 }
 
@@ -819,9 +939,11 @@ static int do_help(int argc, char **argv)
 		"       %s %s dump xlated PROG [{ file FILE | opcodes | visual }]\n"
 		"       %s %s dump jited  PROG [{ file FILE | opcodes }]\n"
 		"       %s %s pin   PROG FILE\n"
-		"       %s %s load  OBJ  FILE [type TYPE] [dev NAME]\n"
+		"       %s %s load  OBJ  FILE [type TYPE] [dev NAME] \\\n"
+		"                         [map { idx IDX | name NAME } MAP]\n"
 		"       %s %s help\n"
 		"\n"
+		"       " HELP_SPEC_MAP "\n"
 		"       " HELP_SPEC_PROGRAM "\n"
 		"       TYPE := { socket | kprobe | kretprobe | classifier | action |\n"
 		"                 tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |\n"
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic
  2018-07-10 21:42 ` [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
@ 2018-07-11  3:01   ` Andrey Ignatov
  0 siblings, 0 replies; 23+ messages in thread
From: Andrey Ignatov @ 2018-07-11  3:01 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: alexei.starovoitov, daniel, oss-drivers, netdev

Jakub Kicinski <jakub.kicinski@netronome.com> [Tue, 2018-07-10 14:43 -0700]:
> libbpf can guess program type based on ELF section names.  As libbpf
> becomes more popular its association between section name strings and
> types becomes more of a standard.  Allow libbpf users to use the same
> logic for matching strings to types, e.g. when the string originates
> from command line.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> v3 (Andrey):
>  - use -EINVAL error code;
>  - rename helper to libbpf_prog_type_by_name().
> ---
>  tools/lib/bpf/libbpf.c | 43 ++++++++++++++++++++++++------------------
>  tools/lib/bpf/libbpf.h |  3 +++
>  2 files changed, 28 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 38ed3e92e393..42f31eff5f3f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2081,23 +2081,31 @@ static const struct {
>  #undef BPF_S_PROG_SEC
>  #undef BPF_SA_PROG_SEC
>  
> -static int bpf_program__identify_section(struct bpf_program *prog)
> +int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
> +			     enum bpf_attach_type *expected_attach_type)
>  {
>  	int i;
>  
> -	if (!prog->section_name)
> -		goto err;
> -
> -	for (i = 0; i < ARRAY_SIZE(section_names); i++)
> -		if (strncmp(prog->section_name, section_names[i].sec,
> -			    section_names[i].len) == 0)
> -			return i;
> +	if (!name)
> +		return -EINVAL;
>  
> -err:
> -	pr_warning("failed to guess program type based on section name %s\n",
> -		   prog->section_name);
> +	for (i = 0; i < ARRAY_SIZE(section_names); i++) {
> +		if (strncmp(name, section_names[i].sec, section_names[i].len))
> +			continue;
> +		*prog_type = section_names[i].prog_type;
> +		*expected_attach_type = section_names[i].expected_attach_type;
> +		return 0;
> +	}
> +	return -EINVAL;
> +}
>  
> -	return -1;
> +static int
> +bpf_program__identify_section(struct bpf_program *prog,
> +			      enum bpf_prog_type *prog_type,
> +			      enum bpf_attach_type *expected_attach_type)
> +{
> +	return libbpf_prog_type_by_name(prog->section_name, prog_type,
> +					expected_attach_type);
>  }
>  
>  int bpf_map__fd(struct bpf_map *map)
> @@ -2230,7 +2238,6 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  	enum bpf_prog_type prog_type;
>  	struct bpf_object *obj;
>  	struct bpf_map *map;
> -	int section_idx;
>  	int err;
>  
>  	if (!attr)
> @@ -2252,14 +2259,14 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  		prog->prog_ifindex = attr->ifindex;
>  		expected_attach_type = attr->expected_attach_type;
>  		if (prog_type == BPF_PROG_TYPE_UNSPEC) {
> -			section_idx = bpf_program__identify_section(prog);
> -			if (section_idx < 0) {
> +			err = bpf_program__identify_section(prog, &prog_type,
> +							    &expected_attach_type);
> +			if (err < 0) {
> +				pr_warning("failed to guess program type based on section name %s\n",
> +					   prog->section_name);
>  				bpf_object__close(obj);
>  				return -EINVAL;
>  			}
> -			prog_type = section_names[section_idx].prog_type;
> -			expected_attach_type =
> -				section_names[section_idx].expected_attach_type;
>  		}
>  
>  		bpf_program__set_type(prog, prog_type);
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 564f4be9bae0..d1ce5c828e2e 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -92,6 +92,9 @@ int bpf_object__set_priv(struct bpf_object *obj, void *priv,
>  			 bpf_object_clear_priv_t clear_priv);
>  void *bpf_object__priv(struct bpf_object *prog);
>  
> +int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
> +			     enum bpf_attach_type *expected_attach_type);
> +
>  /* Accessors of bpf_program */
>  struct bpf_program;
>  struct bpf_program *bpf_program__next(struct bpf_program *prog,
> -- 
> 2.17.1
> 

Thanks for changes Jakub!

Acked-by: Andrey Ignatov <rdna@fb.com>

-- 
Andrey Ignatov

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open()
  2018-07-10 21:43 ` [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
@ 2018-07-11  3:03   ` Andrey Ignatov
  0 siblings, 0 replies; 23+ messages in thread
From: Andrey Ignatov @ 2018-07-11  3:03 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: alexei.starovoitov, daniel, oss-drivers, netdev

Jakub Kicinski <jakub.kicinski@netronome.com> [Tue, 2018-07-10 14:43 -0700]:
> Similarly to bpf_prog_load() users of bpf_object__open() may need
> to specify the expected program type.  Program type is needed at
> open to avoid the kernel version check for program types which don't
> require it.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> v3 (Andrey):
>  - put file path into the attr struct.
> ---
>  tools/lib/bpf/libbpf.c | 26 ++++++++++++++++++++------
>  tools/lib/bpf/libbpf.h |  6 ++++++
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 30992305f4c1..06cd534d2fba 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1520,15 +1520,26 @@ __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
>  	return ERR_PTR(err);
>  }
>  
> -struct bpf_object *bpf_object__open(const char *path)
> +struct bpf_object *bpf_object__open_xattr(struct bpf_object_open_attr *attr)
>  {
>  	/* param validation */
> -	if (!path)
> +	if (!attr->file)
>  		return NULL;
>  
> -	pr_debug("loading %s\n", path);
> +	pr_debug("loading %s\n", attr->file);
> +
> +	return __bpf_object__open(attr->file, NULL, 0,
> +				  bpf_prog_type__needs_kver(attr->prog_type));
> +}
> +
> +struct bpf_object *bpf_object__open(const char *path)
> +{
> +	struct bpf_object_open_attr attr = {
> +		.file		= path,
> +		.prog_type	= BPF_PROG_TYPE_UNSPEC,
> +	};
>  
> -	return __bpf_object__open(path, NULL, 0, true);
> +	return bpf_object__open_xattr(&attr);
>  }
>  
>  struct bpf_object *bpf_object__open_buffer(void *obj_buf,
> @@ -2238,6 +2249,10 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
>  int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  			struct bpf_object **pobj, int *prog_fd)
>  {
> +	struct bpf_object_open_attr open_attr = {
> +		.file		= attr->file,
> +		.prog_type	= attr->prog_type,
> +	};
>  	struct bpf_program *prog, *first_prog = NULL;
>  	enum bpf_attach_type expected_attach_type;
>  	enum bpf_prog_type prog_type;
> @@ -2250,8 +2265,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  	if (!attr->file)
>  		return -EINVAL;
>  
> -	obj = __bpf_object__open(attr->file, NULL, 0,
> -				 bpf_prog_type__needs_kver(attr->prog_type));
> +	obj = bpf_object__open_xattr(&open_attr);
>  	if (IS_ERR_OR_NULL(obj))
>  		return -ENOENT;
>  
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 749acf58a5da..e911ad32d02e 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -66,7 +66,13 @@ void libbpf_set_print(libbpf_print_fn_t warn,
>  /* Hide internal to user */
>  struct bpf_object;
>  
> +struct bpf_object_open_attr {
> +	const char *file;
> +	enum bpf_prog_type prog_type;
> +};
> +
>  struct bpf_object *bpf_object__open(const char *path);
> +struct bpf_object *bpf_object__open_xattr(struct bpf_object_open_attr *attr);
>  struct bpf_object *bpf_object__open_buffer(void *obj_buf,
>  					   size_t obj_buf_sz,
>  					   const char *name);
> -- 
> 2.17.1
> 

Acked-by: Andrey Ignatov <rdna@fb.com>

-- 
Andrey Ignatov

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse
  2018-07-10 21:43 ` [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse Jakub Kicinski
@ 2018-07-11  3:45   ` Andrey Ignatov
  2018-07-11  4:53     ` Jakub Kicinski
  0 siblings, 1 reply; 23+ messages in thread
From: Andrey Ignatov @ 2018-07-11  3:45 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: alexei.starovoitov, daniel, oss-drivers, netdev

Jakub Kicinski <jakub.kicinski@netronome.com> [Tue, 2018-07-10 14:43 -0700]:
> More advanced applications may want to only replace programs without
> destroying associated maps.  Allow libbpf users to achieve that.
> Instead of always creating all of the maps at load time, expose to
> users an API to reconstruct the map object from already existing
> map.
> 
> The map parameters are read from the kernel and replace the parameters
> of the ELF map.  libbpf does not restrict the map replacement, i.e.
> the reused map does not have to be compatible with the ELF map
> definition.  We relay on the verifier for checking the compatibility
> between maps and programs.  The ELF map definition is completely
> overwritten by the information read from the kernel, to make sure
> libbpf's view of map object corresponds to the actual map.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
> v3 (Andrey):
>   - use dup3();
>   - close existing fd;
>   - handle errors better;
>   - depend on fd value to determine pre-set.
> ---
>  tools/lib/bpf/libbpf.c | 53 ++++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h |  1 +
>  2 files changed, 54 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index fd2c4433863d..955f8eafbf41 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1035,6 +1035,53 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf)
>  	return 0;
>  }
>  
> +int bpf_map__reuse_fd(struct bpf_map *map, int fd)
> +{
> +	struct bpf_map_info info = {};
> +	__u32 len = sizeof(info);
> +	int new_fd, err;
> +	char *new_name;
> +
> +	err = bpf_obj_get_info_by_fd(fd, &info, &len);
> +	if (err)
> +		return err;
> +
> +	new_name = strdup(info.name);
> +	if (!new_name)
> +		return -errno;
> +
> +	new_fd = open("/", O_RDONLY | O_CLOEXEC);
> +	if (new_fd < 0)
> +		goto err_free_new_name;
> +
> +	new_fd = dup3(fd, new_fd, O_CLOEXEC);
> +	if (new_fd < 0)
> +		goto err_close_new_fd;
> +
> +	err = zclose(map->fd);
> +	if (err)
> +		goto err_close_new_fd;
> +	free(map->name);
> +
> +	map->fd = new_fd;
> +	map->name = new_name;
> +	map->def.type = info.type;
> +	map->def.key_size = info.key_size;
> +	map->def.value_size = info.value_size;
> +	map->def.max_entries = info.max_entries;
> +	map->def.map_flags = info.map_flags;
> +	map->btf_key_type_id = info.btf_key_type_id;
> +	map->btf_value_type_id = info.btf_value_type_id;
> +
> +	return 0;
> +
> +err_close_new_fd:
> +	close(new_fd);
> +err_free_new_name:
> +	free(new_name);
> +	return -errno;
> +}
> +
>  static int
>  bpf_object__create_maps(struct bpf_object *obj)
>  {
> @@ -1047,6 +1094,12 @@ bpf_object__create_maps(struct bpf_object *obj)
>  		struct bpf_map_def *def = &map->def;
>  		int *pfd = &map->fd;
>  
> +		if (map->fd >= 0) {
> +			pr_debug("skip map create (preset) %s: fd=%d\n",
> +				 map->name, map->fd);
> +			continue;
> +		}
> +
>  		create_attr.name = map->name;
>  		create_attr.map_ifindex = map->map_ifindex;
>  		create_attr.map_type = def->type;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e911ad32d02e..1f8fc2060460 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -261,6 +261,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
>  int bpf_map__set_priv(struct bpf_map *map, void *priv,
>  		      bpf_map_clear_priv_t clear_priv);
>  void *bpf_map__priv(struct bpf_map *map);
> +int bpf_map__reuse_fd(struct bpf_map *map, int fd);
>  bool bpf_map__is_offload_neutral(struct bpf_map *map);
>  void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
>  int bpf_map__pin(struct bpf_map *map, const char *path);
> -- 
> 2.17.1
> 

Acked-by: Andrey Ignatov <rdna@fb.com>

Thanks for all the changes Jakub! Sorry, it forced you to deal with
strerror_r().

One thing, I'm not really sure, is if there is a reason not to copy
ifindex as well from the map that corresponds to passed fd. It's fine
with me to follow-up separately though if it's needed.


-- 
Andrey Ignatov

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse
  2018-07-11  3:45   ` Andrey Ignatov
@ 2018-07-11  4:53     ` Jakub Kicinski
  0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-11  4:53 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: alexei.starovoitov, daniel, oss-drivers, netdev

On Tue, 10 Jul 2018 20:45:36 -0700, Andrey Ignatov wrote:
> Acked-by: Andrey Ignatov <rdna@fb.com>

Thank you! :)
 
> Thanks for all the changes Jakub! Sorry, it forced you to deal with
> strerror_r().

No worries, I already had issues with reallocarray().  Because we have
two reasons now the split seems more justified, and it's cleaner IMHO.

> One thing, I'm not really sure, is if there is a reason not to copy
> ifindex as well from the map that corresponds to passed fd. It's fine
> with me to follow-up separately though if it's needed.

To handle the ifindex correctly we would have to deal with net
namespaces.  Note that ifindex comes with the pair of netns_* fields
in bpf_map_info.  Netdev can also be moved to another namespace after
the information is cached in libbpf.

Right now there is no use for ifindex other than to populate the
attribute at creation time (where it always refer to current namespace).
I'd rather keep ifindex as a input only parameter until we have a clear
use for reading it, otherwise we have to decide on the semantics in the
dark and may bring in quite a bit of complexity for no good reason.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load
  2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
                   ` (12 preceding siblings ...)
  2018-07-10 21:43 ` [PATCH bpf-next v3 13/13] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
@ 2018-07-11 20:18 ` Daniel Borkmann
  13 siblings, 0 replies; 23+ messages in thread
From: Daniel Borkmann @ 2018-07-11 20:18 UTC (permalink / raw)
  To: Jakub Kicinski, alexei.starovoitov, Andrey Ignatov; +Cc: oss-drivers, netdev

On 07/10/2018 11:42 PM, Jakub Kicinski wrote:
> Hi!
> 
> This series starts with two minor clean ups to test_offload.py
> selftest script.
> 
> The next 11 patches extend the abilities of bpftool prog load
> beyond the simple cgroup use cases.  Three new parameters are
> added:
> 
>  - type - allows specifying program type, independent of how
>    code sections are named;
>  - map  - allows reusing existing maps, instead of creating a new
>    map on every program load;
>  - dev  - offload/binding to a device.
> 
> A number of changes to libbpf is required to accomplish the task.
> The section - program type logic mapping is exposed.  We should
> probably aim to use the libbpf program section naming everywhere.
> For reuse of maps we need to allow users to set FD for bpf map
> object in libbpf.
> 
> Examples
> 
> Load program my_xdp.o and pin it as /sys/fs/bpf/my_xdp, for xdp
> program type:
> 
> $ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
>   type xdp
> 
> As above but for offload:
> 
> $ bpftool prog load my_xdp.o /sys/fs/bpf/my_xdp \
>   type xdp \
>   dev netdevsim0
> 
> Load program my_maps.o, but for the first map reuse map id 17,
> and for the map called "other_map" reuse pinned map /sys/fs/bpf/map0:
> 
> $ bpftool prog load my_maps.o /sys/fs/bpf/prog \
>   map idx 0 id 17 \
>   map name other_map pinned /sys/fs/bpf/map0
> 
> ---
> v3:
>  - fix return codes in patch 5;
>  - rename libbpf_prog_type_by_string() -> libbpf_prog_type_by_name();
>  - fold file path into xattr in patch 8;
>  - add patch 10;
>  - use dup3() in patch 12;
>  - depend on fd value in patch 12;
>  - close old fd in patch 12.
> v2:
>  - add compat for reallocarray().

Applied to bpf-next, thanks Jakub!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [bpf-next,v3,11/13] tools: bpf: make use of reallocarray
  2018-07-10 21:43 ` [PATCH bpf-next v3 11/13] tools: bpf: make use of reallocarray Jakub Kicinski
@ 2018-07-13 23:53   ` Guenter Roeck
  2018-07-14  0:07     ` Jakub Kicinski
  0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2018-07-13 23:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: alexei.starovoitov, daniel, Andrey Ignatov, oss-drivers, netdev

Hi,

On Tue, Jul 10, 2018 at 02:43:05PM -0700, Jakub Kicinski wrote:
> reallocarray() is a safer variant of realloc which checks for
> multiplication overflow in case of array allocation.  Since it's
> not available in Glibc < 2.26 import kernel's overflow.h and
> add a static inline implementation when needed.  Use feature
> detection to probe for existence of reallocarray.
> 

This probe doesn't work on my system (Ubuntu 16.04).

libbpf.c: In function ‘bpf_object__add_program’:
libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’

bisect points to this patch.

Guenter

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [bpf-next,v3,11/13] tools: bpf: make use of reallocarray
  2018-07-13 23:53   ` [bpf-next,v3,11/13] " Guenter Roeck
@ 2018-07-14  0:07     ` Jakub Kicinski
  2018-07-14  0:31       ` Guenter Roeck
  0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-14  0:07 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: alexei.starovoitov, daniel, Andrey Ignatov, oss-drivers, netdev

On Fri, 13 Jul 2018 16:53:05 -0700, Guenter Roeck wrote:
> Hi,
> 
> On Tue, Jul 10, 2018 at 02:43:05PM -0700, Jakub Kicinski wrote:
> > reallocarray() is a safer variant of realloc which checks for
> > multiplication overflow in case of array allocation.  Since it's
> > not available in Glibc < 2.26 import kernel's overflow.h and
> > add a static inline implementation when needed.  Use feature
> > detection to probe for existence of reallocarray.
> >   
> 
> This probe doesn't work on my system (Ubuntu 16.04).
> 
> libbpf.c: In function ‘bpf_object__add_program’:
> libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’

No way :( :(  Maybe you have to clean the build directory hard?  
Maybe you have old feature check results or some such?

# lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.2 LTS
Release:	16.04
Codename:	xenial
# rm -rf /tmp/bpftool
# mkdir /tmp/bpftool
# make -C tools/bpf/bpftool/ O=/tmp/bpftool
make: Entering directory '/tmp/linux/tools/bpf/bpftool'

Auto-detecting system features:
...                        libbfd: [ OFF ]
...        disassembler-four-args: [ OFF ]

  CC       /tmp/bpftool/map_perf_ring.o
  CC       /tmp/bpftool/xlated_dumper.o
  CC       /tmp/bpftool/perf.o
  CC       /tmp/bpftool/prog.o
  CC       /tmp/bpftool/common.o
  CC       /tmp/bpftool/cgroup.o
  CC       /tmp/bpftool/main.o
  CC       /tmp/bpftool/json_writer.o
  CC       /tmp/bpftool/cfg.o
  CC       /tmp/bpftool/map.o
  CC       /tmp/bpftool/jit_disasm.o
  CC       /tmp/bpftool/disasm.o
make[1]: Entering directory '/tmp/linux/tools/lib/bpf'

Auto-detecting system features:
...                        libelf: [ on  ]
...                           bpf: [ on  ]

Warning: Kernel ABI header at 'tools/include/uapi/linux/if_link.h' differs from latest version at 'include/uapi/linux/if_link.h'
  CC       /tmp/bpftool/libbpf.o
  CC       /tmp/bpftool/bpf.o
  CC       /tmp/bpftool/nlattr.o
  CC       /tmp/bpftool/btf.o
  CC       /tmp/bpftool/libbpf_errno.o
  LD       /tmp/bpftool/libbpf-in.o
  LINK     /tmp/bpftool/libbpf.a
make[1]: Leaving directory '/tmp/linux/tools/lib/bpf'
  LINK     /tmp/bpftool/bpftool
make: Leaving directory '/tmp/linux/tools/bpf/bpftool'

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [bpf-next,v3,11/13] tools: bpf: make use of reallocarray
  2018-07-14  0:07     ` Jakub Kicinski
@ 2018-07-14  0:31       ` Guenter Roeck
  2018-07-14  1:16         ` Jakub Kicinski
  0 siblings, 1 reply; 23+ messages in thread
From: Guenter Roeck @ 2018-07-14  0:31 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: alexei.starovoitov, daniel, Andrey Ignatov, oss-drivers, netdev

On 07/13/2018 05:07 PM, Jakub Kicinski wrote:
> On Fri, 13 Jul 2018 16:53:05 -0700, Guenter Roeck wrote:
>> Hi,
>>
>> On Tue, Jul 10, 2018 at 02:43:05PM -0700, Jakub Kicinski wrote:
>>> reallocarray() is a safer variant of realloc which checks for
>>> multiplication overflow in case of array allocation.  Since it's
>>> not available in Glibc < 2.26 import kernel's overflow.h and
>>> add a static inline implementation when needed.  Use feature
>>> detection to probe for existence of reallocarray.
>>>    
>>
>> This probe doesn't work on my system (Ubuntu 16.04).
>>
>> libbpf.c: In function ‘bpf_object__add_program’:
>> libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’
> 
> No way :( :(  Maybe you have to clean the build directory hard?
> Maybe you have old feature check results or some such?
> 

Unlikely. This is seen by my test builders which always start from a clean state.

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.4 LTS
Release:	16.04
Codename:	xenial
$ gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ ld --version
GNU ld (GNU Binutils for Ubuntu) 2.26.1
Copyright (C) 2015 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.
$ git describe
next-20180713
$ git clean -d -x -f -q
$ make allmodconfig
$ make tools/perf
...
libbpf.c: In function ‘bpf_object__add_program’:
libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’

Guenter

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [bpf-next,v3,11/13] tools: bpf: make use of reallocarray
  2018-07-14  0:31       ` Guenter Roeck
@ 2018-07-14  1:16         ` Jakub Kicinski
  0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2018-07-14  1:16 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: alexei.starovoitov, daniel, Andrey Ignatov, oss-drivers, netdev

On Fri, 13 Jul 2018 17:31:34 -0700, Guenter Roeck wrote:
> On 07/13/2018 05:07 PM, Jakub Kicinski wrote:
> > On Fri, 13 Jul 2018 16:53:05 -0700, Guenter Roeck wrote:  
> >> Hi,
> >>
> >> On Tue, Jul 10, 2018 at 02:43:05PM -0700, Jakub Kicinski wrote:  
> >>> reallocarray() is a safer variant of realloc which checks for
> >>> multiplication overflow in case of array allocation.  Since it's
> >>> not available in Glibc < 2.26 import kernel's overflow.h and
> >>> add a static inline implementation when needed.  Use feature
> >>> detection to probe for existence of reallocarray.
> >>>      
> >>
> >> This probe doesn't work on my system (Ubuntu 16.04).
> >>
> >> libbpf.c: In function ‘bpf_object__add_program’:
> >> libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’  
> > 
> > No way :( :(  Maybe you have to clean the build directory hard?
> > Maybe you have old feature check results or some such?
> >   
> 
> Unlikely. This is seen by my test builders which always start from a clean state.
> 
> $ lsb_release -a
> No LSB modules are available.
> Distributor ID:	Ubuntu
> Description:	Ubuntu 16.04.4 LTS
> Release:	16.04
> Codename:	xenial
> $ gcc --version
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> $ ld --version
> GNU ld (GNU Binutils for Ubuntu) 2.26.1
> Copyright (C) 2015 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or (at your option) a later version.
> This program has absolutely no warranty.
> $ git describe
> next-20180713
> $ git clean -d -x -f -q
> $ make allmodconfig
> $ make tools/perf
> ...
> libbpf.c: In function ‘bpf_object__add_program’:
> libbpf.c:326:10: error: implicit declaration of function ‘reallocarray’

Oh, you're not building perf!  Looks like I missed adding reallocarray
to the default feature set on which perf depends, and since given that
features get reused (libbpf won't get to run its own feature detection)
we have to test for reallocarray there:

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 5b6dda3b1ca8..f216b2f5c3d7 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -57,6 +57,7 @@ FEATURE_TESTS_BASIC :=                  \
         libunwind-aarch64               \
         pthread-attr-setaffinity-np     \
         pthread-barrier                \
+        reallocarray                    \
         stackprotector-all              \
         timerfd                         \
         libdw-dwarf-unwind              \

^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-07-14  1:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-10 21:42 [PATCH bpf-next v3 00/13] tools: bpf: extend bpftool prog load Jakub Kicinski
2018-07-10 21:42 ` [PATCH bpf-next v3 01/13] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
2018-07-10 21:42 ` [PATCH bpf-next v3 02/13] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
2018-07-10 21:42 ` [PATCH bpf-next v3 03/13] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
2018-07-10 21:42 ` [PATCH bpf-next v3 04/13] tools: bpftool: add support for loading programs for offload Jakub Kicinski
2018-07-10 21:42 ` [PATCH bpf-next v3 05/13] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
2018-07-11  3:01   ` Andrey Ignatov
2018-07-10 21:43 ` [PATCH bpf-next v3 06/13] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 07/13] tools: libbpf: recognize offload neutral maps Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 08/13] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
2018-07-11  3:03   ` Andrey Ignatov
2018-07-10 21:43 ` [PATCH bpf-next v3 09/13] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 10/13] tools: libbpf: move library error code into a separate file Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 11/13] tools: bpf: make use of reallocarray Jakub Kicinski
2018-07-13 23:53   ` [bpf-next,v3,11/13] " Guenter Roeck
2018-07-14  0:07     ` Jakub Kicinski
2018-07-14  0:31       ` Guenter Roeck
2018-07-14  1:16         ` Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 12/13] tools: libbpf: allow map reuse Jakub Kicinski
2018-07-11  3:45   ` Andrey Ignatov
2018-07-11  4:53     ` Jakub Kicinski
2018-07-10 21:43 ` [PATCH bpf-next v3 13/13] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
2018-07-11 20:18 ` [PATCH bpf-next v3 00/13] tools: bpf: extend " Daniel Borkmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).