* [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load
@ 2018-07-04 2:54 Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 01/11] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
` (10 more replies)
0 siblings, 11 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
Hi!
This series starts with two minor clean ups to test_offload.py
selftest script.
The next 9 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
---
This set needs a net-next -> bpf-next merge back, which I believe is
imminent. It should not conflict with Okash's work on BTF.
Jakub Kicinski (11):
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: allow map reuse
tools: bpftool: allow reuse of maps with bpftool prog load
.../bpftool/Documentation/bpftool-prog.rst | 33 ++-
tools/bpf/bpftool/bash-completion/bpftool | 96 ++++++-
tools/bpf/bpftool/main.h | 18 ++
tools/bpf/bpftool/map.c | 4 +-
tools/bpf/bpftool/prog.c | 245 +++++++++++++++++-
tools/lib/bpf/libbpf.c | 107 ++++++--
tools/lib/bpf/libbpf.h | 11 +
tools/testing/selftests/bpf/test_offload.py | 10 +-
8 files changed, 476 insertions(+), 48 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH bpf-next 01/11] selftests/bpf: remove duplicated word from test offloads
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 02/11] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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] 16+ messages in thread
* [PATCH bpf-next 02/11] selftests/bpf: add Error: prefix in check_extack helper
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 01/11] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 03/11] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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] 16+ messages in thread
* [PATCH bpf-next 03/11] tools: bpftool: refactor argument parsing for prog load
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 01/11] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 02/11] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 04/11] tools: bpftool: add support for loading programs for offload Jakub Kicinski
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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] 16+ messages in thread
* [PATCH bpf-next 04/11] tools: bpftool: add support for loading programs for offload
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (2 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 03/11] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 05/11] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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 fffd76f4998b..ab044f528a85 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] 16+ messages in thread
* [PATCH bpf-next 05/11] tools: libbpf: expose the prog type guessing from section name logic
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (3 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 04/11] tools: bpftool: add support for loading programs for offload Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 06/11] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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>
---
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..30f3e58bd563 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2081,25 +2081,33 @@ 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_string(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;
-
-err:
- pr_warning("failed to guess program type based on section name %s\n",
- prog->section_name);
+ if (!name)
+ return -1;
+ 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 -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_string(prog->section_name, prog_type,
+ expected_attach_type);
+}
+
int bpf_map__fd(struct bpf_map *map)
{
return map ? map->fd : -EINVAL;
@@ -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..617dacfc6704 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_string(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] 16+ messages in thread
* [PATCH bpf-next 06/11] tools: bpftool: allow users to specify program type for prog load
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (4 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 05/11] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 07/11] tools: libbpf: recognize offload neutral maps Jakub Kicinski
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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 ab044f528a85..9ae33a73d732 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..7a06fd4c5d27 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_string(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] 16+ messages in thread
* [PATCH bpf-next 07/11] tools: libbpf: recognize offload neutral maps
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (5 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 06/11] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 08/11] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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 30f3e58bd563..edc3b0b3737d 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 617dacfc6704..3122d74f2643 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] 16+ messages in thread
* [PATCH bpf-next 08/11] tools: libbpf: add extended attributes version of bpf_object__open()
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (6 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 07/11] tools: libbpf: recognize offload neutral maps Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 09/11] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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>
---
tools/lib/bpf/libbpf.c | 21 +++++++++++++++++----
tools/lib/bpf/libbpf.h | 6 ++++++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index edc3b0b3737d..5b0e84fbcf71 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1520,7 +1520,8 @@ __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(const char *path,
+ struct bpf_object_open_attr *attr)
{
/* param validation */
if (!path)
@@ -1528,7 +1529,17 @@ struct bpf_object *bpf_object__open(const char *path)
pr_debug("loading %s\n", path);
- return __bpf_object__open(path, NULL, 0, true);
+ return __bpf_object__open(path, 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 = {
+ .prog_type = BPF_PROG_TYPE_UNSPEC,
+ };
+
+ return bpf_object__open_xattr(path, &attr);
}
struct bpf_object *bpf_object__open_buffer(void *obj_buf,
@@ -2238,6 +2249,9 @@ 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 = {
+ .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 +2264,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(attr->file, &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 3122d74f2643..60593ac44700 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 {
+ enum bpf_prog_type prog_type;
+};
+
struct bpf_object *bpf_object__open(const char *path);
+struct bpf_object *bpf_object__open_xattr(const char *path,
+ 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] 16+ messages in thread
* [PATCH bpf-next 09/11] tools: bpftool: reimplement bpf_prog_load() for prog load
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (7 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 08/11] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 10/11] tools: libbpf: allow map reuse Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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>
---
tools/bpf/bpftool/prog.c | 57 ++++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 7a06fd4c5d27..267d653c93f5 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,12 +684,15 @@ 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;
+ __u32 ifindex = 0;
int err;
if (!REQ_ARGS(2))
@@ -719,7 +724,7 @@ static int do_load(int argc, char **argv)
strcat(type, "/");
err = libbpf_prog_type_by_string(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(objfile, &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_string(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] 16+ messages in thread
* [PATCH bpf-next 10/11] tools: libbpf: allow map reuse
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (8 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 09/11] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
10 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +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>
---
tools/lib/bpf/libbpf.c | 35 +++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 1 +
2 files changed, 36 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5b0e84fbcf71..916810207c2a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -214,6 +214,7 @@ struct bpf_map {
int fd;
char *name;
size_t offset;
+ bool fd_preset;
int map_ifindex;
struct bpf_map_def def;
uint32_t btf_key_type_id;
@@ -1081,6 +1082,34 @@ 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 err;
+
+ err = bpf_obj_get_info_by_fd(fd, &info, &len);
+ if (err)
+ return err;
+
+ map->fd = dup(fd);
+ if (map->fd < 0)
+ return map->fd;
+ map->fd_preset = true;
+
+ free(map->name);
+ map->name = strdup(info.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;
+}
+
static int
bpf_object__create_maps(struct bpf_object *obj)
{
@@ -1093,6 +1122,12 @@ bpf_object__create_maps(struct bpf_object *obj)
struct bpf_map_def *def = &map->def;
int *pfd = &map->fd;
+ if (map->fd_preset) {
+ 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 60593ac44700..8e709a74f47c 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] 16+ messages in thread
* [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
` (9 preceding siblings ...)
2018-07-04 2:54 ` [PATCH bpf-next 10/11] tools: libbpf: allow map reuse Jakub Kicinski
@ 2018-07-04 2:54 ` Jakub Kicinski
2018-07-05 8:35 ` Daniel Borkmann
10 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-04 2:54 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
Add map parameter to prog load which will allow reuse of existing
maps instead of creating new ones.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
.../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 9ae33a73d732..626598964cee 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 15b6c49ae533..1a9a2aefa014 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -74,6 +74,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,
@@ -135,6 +137,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 267d653c93f5..0f8bdab62864 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;
const char *objfile, *pinfile;
+ unsigned int old_map_fds = 0;
struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
+ 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(objfile, &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] 16+ messages in thread
* Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load
2018-07-04 2:54 ` [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
@ 2018-07-05 8:35 ` Daniel Borkmann
2018-07-05 22:57 ` Jakub Kicinski
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Borkmann @ 2018-07-05 8:35 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov; +Cc: oss-drivers, netdev
On 07/04/2018 04:54 AM, Jakub Kicinski wrote:
> Add map parameter to prog load which will allow reuse of existing
> maps instead of creating new ones.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
[...]
> +
> + fd = map_parse_fd(&argc, &argv);
> + if (fd < 0)
> + goto err_free_reuse_maps;
> +
> + map_replace = reallocarray(map_replace, old_map_fds + 1,
> + sizeof(*map_replace));
> + if (!map_replace) {
> + p_err("mem alloc failed");
> + goto err_free_reuse_maps;
Series in general looks good to me. However, above reallocarray() doesn't
exist and hence build fails, please see below. Is that from newest glibc?
You probably need some fallback implementation or in general have something
bpftool internal that doesn't make a bet on its availability.
# make
Auto-detecting system features:
... libbfd: [ on ]
... disassembler-four-args: [ OFF ]
CC bpf_jit_disasm.o
LINK bpf_jit_disasm
CC bpf_dbg.o
LINK bpf_dbg
CC bpf_asm.o
BISON bpf_exp.yacc.c
CC bpf_exp.yacc.o
FLEX bpf_exp.lex.c
CC bpf_exp.lex.o
LINK bpf_asm
DESCEND bpftool
Auto-detecting system features:
... libbfd: [ on ]
... disassembler-four-args: [ OFF ]
CC map_perf_ring.o
CC xlated_dumper.o
CC perf.o
CC prog.o
prog.c: In function ‘do_load’:
prog.c:785:18: warning: implicit declaration of function ‘reallocarray’ [-Wimplicit-function-declaration]
map_replace = reallocarray(map_replace, old_map_fds + 1,
^~~~~~~~~~~~
prog.c:785:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
map_replace = reallocarray(map_replace, old_map_fds + 1,
^
CC common.o
CC cgroup.o
CC main.o
CC json_writer.o
CC cfg.o
CC map.o
CC jit_disasm.o
CC disasm.o
Auto-detecting system features:
... libelf: [ on ]
... bpf: [ on ]
Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'
CC libbpf.o
CC bpf.o
CC nlattr.o
CC btf.o
LD libbpf-in.o
LINK libbpf.a
LINK bpftool
prog.o: In function `do_load':
prog.c:(.text+0x23d): undefined reference to `reallocarray'
collect2: error: ld returned 1 exit status
Makefile:89: recipe for target 'bpftool' failed
make[1]: *** [bpftool] Error 1
Makefile:99: recipe for target 'bpftool' failed
make: *** [bpftool] Error 2
Thanks,
Daniel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load
2018-07-05 8:35 ` Daniel Borkmann
@ 2018-07-05 22:57 ` Jakub Kicinski
2018-07-06 7:16 ` Daniel Borkmann
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-05 22:57 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: alexei.starovoitov, oss-drivers, netdev
On Thu, 5 Jul 2018 10:35:24 +0200, Daniel Borkmann wrote:
> On 07/04/2018 04:54 AM, Jakub Kicinski wrote:
> > Add map parameter to prog load which will allow reuse of existing
> > maps instead of creating new ones.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> [...]
> > +
> > + fd = map_parse_fd(&argc, &argv);
> > + if (fd < 0)
> > + goto err_free_reuse_maps;
> > +
> > + map_replace = reallocarray(map_replace, old_map_fds + 1,
> > + sizeof(*map_replace));
> > + if (!map_replace) {
> > + p_err("mem alloc failed");
> > + goto err_free_reuse_maps;
>
> Series in general looks good to me. However, above reallocarray() doesn't
> exist and hence build fails, please see below. Is that from newest glibc?
>
> You probably need some fallback implementation or in general have something
> bpftool internal that doesn't make a bet on its availability.
>
> # make
>
> Auto-detecting system features:
> ... libbfd: [ on ]
> ... disassembler-four-args: [ OFF ]
>
> CC bpf_jit_disasm.o
> LINK bpf_jit_disasm
> CC bpf_dbg.o
> LINK bpf_dbg
> CC bpf_asm.o
> BISON bpf_exp.yacc.c
> CC bpf_exp.yacc.o
> FLEX bpf_exp.lex.c
> CC bpf_exp.lex.o
> LINK bpf_asm
> DESCEND bpftool
>
> Auto-detecting system features:
> ... libbfd: [ on ]
> ... disassembler-four-args: [ OFF ]
>
> CC map_perf_ring.o
> CC xlated_dumper.o
> CC perf.o
> CC prog.o
> prog.c: In function ‘do_load’:
> prog.c:785:18: warning: implicit declaration of function ‘reallocarray’ [-Wimplicit-function-declaration]
> map_replace = reallocarray(map_replace, old_map_fds + 1,
> ^~~~~~~~~~~~
> prog.c:785:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
> map_replace = reallocarray(map_replace, old_map_fds + 1,
> ^
> CC common.o
> CC cgroup.o
> CC main.o
> CC json_writer.o
> CC cfg.o
> CC map.o
> CC jit_disasm.o
> CC disasm.o
>
> Auto-detecting system features:
> ... libelf: [ on ]
> ... bpf: [ on ]
>
> Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'
> CC libbpf.o
> CC bpf.o
> CC nlattr.o
> CC btf.o
> LD libbpf-in.o
> LINK libbpf.a
> LINK bpftool
> prog.o: In function `do_load':
> prog.c:(.text+0x23d): undefined reference to `reallocarray'
> collect2: error: ld returned 1 exit status
> Makefile:89: recipe for target 'bpftool' failed
> make[1]: *** [bpftool] Error 1
> Makefile:99: recipe for target 'bpftool' failed
> make: *** [bpftool] Error 2
Oh no.. Sorry & thanks for catching this. It would be nice to not
depend on Glibc version defines, in case someone is using a different
library. Jiong suggested we can just use the feature detection, so I
have something like this:
---
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 0911b00b25cc..20a691659381 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -52,8 +52,8 @@ INSTALL ?= install
RM ?= rm -f
FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray
+FEATURE_DISPLAY = libbfd disassembler-four-args reallocarray
check_feat := 1
NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
diff --git a/tools/bpf/bpftool/compat.h b/tools/bpf/bpftool/compat.h
new file mode 100644
index 000000000000..7885cedc9efe
--- /dev/null
+++ b/tools/bpf/bpftool/compat.h
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2018 Netronome Systems, Inc. */
+
+#ifndef __BPF_TOOL_COMPAT_H
+#define __BPF_TOOL_COMPAT_H
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+
+static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
+{
+ return realloc(ptr, nmemb * size);
+}
+#endif
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 1a9a2aefa014..2106adb73631 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -43,6 +43,7 @@
#include <linux/kernel.h>
#include <linux/hashtable.h>
+#include "compat.h"
#include "json_writer.h"
#define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index dac9563b5470..0516259be70f 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -14,6 +14,7 @@ FILES= \
test-libaudit.bin \
test-libbfd.bin \
test-disassembler-four-args.bin \
+ test-reallocarray.bin \
test-liberty.bin \
test-liberty-z.bin \
test-cplus-demangle.bin \
@@ -204,6 +205,9 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
$(OUTPUT)test-disassembler-four-args.bin:
$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
+$(OUTPUT)test-reallocarray.bin:
+ $(BUILD)
+
$(OUTPUT)test-liberty.bin:
$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
diff --git a/tools/build/feature/test-reallocarray.c b/tools/build/feature/test-reallocarray.c
new file mode 100644
index 000000000000..8170de35150d
--- /dev/null
+++ b/tools/build/feature/test-reallocarray.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <stdlib.h>
+
+int main(void)
+{
+ return !!reallocarray(NULL, 1, 1);
+}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load
2018-07-05 22:57 ` Jakub Kicinski
@ 2018-07-06 7:16 ` Daniel Borkmann
2018-07-06 15:30 ` Jakub Kicinski
0 siblings, 1 reply; 16+ messages in thread
From: Daniel Borkmann @ 2018-07-06 7:16 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: alexei.starovoitov, oss-drivers, netdev
On 07/06/2018 12:57 AM, Jakub Kicinski wrote:
> On Thu, 5 Jul 2018 10:35:24 +0200, Daniel Borkmann wrote:
>> On 07/04/2018 04:54 AM, Jakub Kicinski wrote:
>>> Add map parameter to prog load which will allow reuse of existing
>>> maps instead of creating new ones.
>>>
>>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
>> [...]
>>> +
>>> + fd = map_parse_fd(&argc, &argv);
>>> + if (fd < 0)
>>> + goto err_free_reuse_maps;
>>> +
>>> + map_replace = reallocarray(map_replace, old_map_fds + 1,
>>> + sizeof(*map_replace));
>>> + if (!map_replace) {
>>> + p_err("mem alloc failed");
>>> + goto err_free_reuse_maps;
>>
>> Series in general looks good to me. However, above reallocarray() doesn't
>> exist and hence build fails, please see below. Is that from newest glibc?
>>
>> You probably need some fallback implementation or in general have something
>> bpftool internal that doesn't make a bet on its availability.
>>
>> # make
>>
>> Auto-detecting system features:
>> ... libbfd: [ on ]
>> ... disassembler-four-args: [ OFF ]
>>
>> CC bpf_jit_disasm.o
>> LINK bpf_jit_disasm
>> CC bpf_dbg.o
>> LINK bpf_dbg
>> CC bpf_asm.o
>> BISON bpf_exp.yacc.c
>> CC bpf_exp.yacc.o
>> FLEX bpf_exp.lex.c
>> CC bpf_exp.lex.o
>> LINK bpf_asm
>> DESCEND bpftool
>>
>> Auto-detecting system features:
y>> ... libbfd: [ on ]
>> ... disassembler-four-args: [ OFF ]
>>
>> CC map_perf_ring.o
>> CC xlated_dumper.o
>> CC perf.o
>> CC prog.o
>> prog.c: In function ‘do_load’:
>> prog.c:785:18: warning: implicit declaration of function ‘reallocarray’ [-Wimplicit-function-declaration]
>> map_replace = reallocarray(map_replace, old_map_fds + 1,
>> ^~~~~~~~~~~~
>> prog.c:785:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>> map_replace = reallocarray(map_replace, old_map_fds + 1,
>> ^
>> CC common.o
>> CC cgroup.o
>> CC main.o
>> CC json_writer.o
>> CC cfg.o
>> CC map.o
>> CC jit_disasm.o
>> CC disasm.o
>>
>> Auto-detecting system features:
>> ... libelf: [ on ]
>> ... bpf: [ on ]
>>
>> Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'
>> CC libbpf.o
>> CC bpf.o
>> CC nlattr.o
>> CC btf.o
>> LD libbpf-in.o
>> LINK libbpf.a
>> LINK bpftool
>> prog.o: In function `do_load':
>> prog.c:(.text+0x23d): undefined reference to `reallocarray'
>> collect2: error: ld returned 1 exit status
>> Makefile:89: recipe for target 'bpftool' failed
>> make[1]: *** [bpftool] Error 1
>> Makefile:99: recipe for target 'bpftool' failed
>> make: *** [bpftool] Error 2
>
> Oh no.. Sorry & thanks for catching this. It would be nice to not
> depend on Glibc version defines, in case someone is using a different
> library. Jiong suggested we can just use the feature detection, so I
> have something like this:
Yeah, that would be okay to do if you want to go that route, sure. Other option
I had in mind would have been to import include/linux/overflow.h into the
tools/include/linux/ and have a minor wrapper similar to kmalloc_array() in a
utils.h in bpftool to get to the same for all users. But I think feature test
is totally fine too, and in general some form of reallocarray() would be good
to have rather than plain realloc().
So, below complies for me. Although don't we need to define a CFLAG based on
the outcome of the test similar as in feature-disassembler-four-args? Otherwise
with the below diff the test doesn't really do much, no? Meaning, adding a ...
ifeq ($(feature-reallocarray), 1)
CFLAGS += -DHAVE_REALLOCARRAY
endif
... to the Makefile and in compat.h having it enabled through:
#ifndef HAVE_REALLOCARRAY
static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
{
return realloc(ptr, nmemb * size);
}
#endif
In any case, we could use reallocarray() also in couple of other places in
bpftool and libbpf.
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 0911b00b25cc..20a691659381 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -52,8 +52,8 @@ INSTALL ?= install
> RM ?= rm -f
>
> FEATURE_USER = .bpftool
> -FEATURE_TESTS = libbfd disassembler-four-args
> -FEATURE_DISPLAY = libbfd disassembler-four-args
> +FEATURE_TESTS = libbfd disassembler-four-args reallocarray
> +FEATURE_DISPLAY = libbfd disassembler-four-args reallocarray
>
> check_feat := 1
> NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
> diff --git a/tools/bpf/bpftool/compat.h b/tools/bpf/bpftool/compat.h
> new file mode 100644
> index 000000000000..7885cedc9efe
> --- /dev/null
> +++ b/tools/bpf/bpftool/compat.h
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2018 Netronome Systems, Inc. */
> +
> +#ifndef __BPF_TOOL_COMPAT_H
> +#define __BPF_TOOL_COMPAT_H
> +
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +
> +static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
> +{
> + return realloc(ptr, nmemb * size);
> +}
> +#endif
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 1a9a2aefa014..2106adb73631 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -43,6 +43,7 @@
> #include <linux/kernel.h>
> #include <linux/hashtable.h>
>
> +#include "compat.h"
> #include "json_writer.h"
>
> #define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index dac9563b5470..0516259be70f 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -14,6 +14,7 @@ FILES= \
> test-libaudit.bin \
> test-libbfd.bin \
> test-disassembler-four-args.bin \
> + test-reallocarray.bin \
> test-liberty.bin \
> test-liberty-z.bin \
> test-cplus-demangle.bin \
> @@ -204,6 +205,9 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
> $(OUTPUT)test-disassembler-four-args.bin:
> $(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
>
> +$(OUTPUT)test-reallocarray.bin:
> + $(BUILD)
> +
> $(OUTPUT)test-liberty.bin:
> $(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
>
> diff --git a/tools/build/feature/test-reallocarray.c b/tools/build/feature/test-reallocarray.c
> new file mode 100644
> index 000000000000..8170de35150d
> --- /dev/null
> +++ b/tools/build/feature/test-reallocarray.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> + return !!reallocarray(NULL, 1, 1);
> +}
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load
2018-07-06 7:16 ` Daniel Borkmann
@ 2018-07-06 15:30 ` Jakub Kicinski
0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2018-07-06 15:30 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: alexei.starovoitov, oss-drivers, netdev
On Fri, 6 Jul 2018 09:16:24 +0200, Daniel Borkmann wrote:
> On 07/06/2018 12:57 AM, Jakub Kicinski wrote:
> > On Thu, 5 Jul 2018 10:35:24 +0200, Daniel Borkmann wrote:
> >> On 07/04/2018 04:54 AM, Jakub Kicinski wrote:
> >>> Add map parameter to prog load which will allow reuse of existing
> >>> maps instead of creating new ones.
> >>>
> >>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> >>> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> >> [...]
> >>> +
> >>> + fd = map_parse_fd(&argc, &argv);
> >>> + if (fd < 0)
> >>> + goto err_free_reuse_maps;
> >>> +
> >>> + map_replace = reallocarray(map_replace, old_map_fds + 1,
> >>> + sizeof(*map_replace));
> >>> + if (!map_replace) {
> >>> + p_err("mem alloc failed");
> >>> + goto err_free_reuse_maps;
> >>
> >> Series in general looks good to me. However, above reallocarray() doesn't
> >> exist and hence build fails, please see below. Is that from newest glibc?
> >>
> >> You probably need some fallback implementation or in general have something
> >> bpftool internal that doesn't make a bet on its availability.
> >>
> >> # make
> >>
> >> Auto-detecting system features:
> >> ... libbfd: [ on ]
> >> ... disassembler-four-args: [ OFF ]
> >>
> >> CC bpf_jit_disasm.o
> >> LINK bpf_jit_disasm
> >> CC bpf_dbg.o
> >> LINK bpf_dbg
> >> CC bpf_asm.o
> >> BISON bpf_exp.yacc.c
> >> CC bpf_exp.yacc.o
> >> FLEX bpf_exp.lex.c
> >> CC bpf_exp.lex.o
> >> LINK bpf_asm
> >> DESCEND bpftool
> >>
> >> Auto-detecting system features:
> y>> ... libbfd: [ on ]
> >> ... disassembler-four-args: [ OFF ]
> >>
> >> CC map_perf_ring.o
> >> CC xlated_dumper.o
> >> CC perf.o
> >> CC prog.o
> >> prog.c: In function ‘do_load’:
> >> prog.c:785:18: warning: implicit declaration of function ‘reallocarray’ [-Wimplicit-function-declaration]
> >> map_replace = reallocarray(map_replace, old_map_fds + 1,
> >> ^~~~~~~~~~~~
> >> prog.c:785:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
> >> map_replace = reallocarray(map_replace, old_map_fds + 1,
> >> ^
> >> CC common.o
> >> CC cgroup.o
> >> CC main.o
> >> CC json_writer.o
> >> CC cfg.o
> >> CC map.o
> >> CC jit_disasm.o
> >> CC disasm.o
> >>
> >> Auto-detecting system features:
> >> ... libelf: [ on ]
> >> ... bpf: [ on ]
> >>
> >> Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'
> >> CC libbpf.o
> >> CC bpf.o
> >> CC nlattr.o
> >> CC btf.o
> >> LD libbpf-in.o
> >> LINK libbpf.a
> >> LINK bpftool
> >> prog.o: In function `do_load':
> >> prog.c:(.text+0x23d): undefined reference to `reallocarray'
> >> collect2: error: ld returned 1 exit status
> >> Makefile:89: recipe for target 'bpftool' failed
> >> make[1]: *** [bpftool] Error 1
> >> Makefile:99: recipe for target 'bpftool' failed
> >> make: *** [bpftool] Error 2
> >
> > Oh no.. Sorry & thanks for catching this. It would be nice to not
> > depend on Glibc version defines, in case someone is using a different
> > library. Jiong suggested we can just use the feature detection, so I
> > have something like this:
>
> Yeah, that would be okay to do if you want to go that route, sure. Other option
> I had in mind would have been to import include/linux/overflow.h into the
> tools/include/linux/ and have a minor wrapper similar to kmalloc_array() in a
> utils.h in bpftool to get to the same for all users. But I think feature test
> is totally fine too, and in general some form of reallocarray() would be good
> to have rather than plain realloc().
>
> So, below complies for me. Although don't we need to define a CFLAG based on
> the outcome of the test similar as in feature-disassembler-four-args? Otherwise
> with the below diff the test doesn't really do much, no? Meaning, adding a ...
>
> ifeq ($(feature-reallocarray), 1)
> CFLAGS += -DHAVE_REALLOCARRAY
> endif
>
> ... to the Makefile and in compat.h having it enabled through:
>
> #ifndef HAVE_REALLOCARRAY
> static inline void *reallocarray(void *ptr, size_t nmemb, size_t size)
> {
> return realloc(ptr, nmemb * size);
> }
> #endif
Ugh, you're very right, reallocarray is a weak alias in glibc, which is
probably why I didn't see any errors.
> In any case, we could use reallocarray() also in couple of other places in
> bpftool and libbpf.
I'll look into it, too.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-07-06 15:30 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04 2:54 [PATCH bpf-next 00/11] tools: bpf: extend bpftool prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 01/11] selftests/bpf: remove duplicated word from test offloads Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 02/11] selftests/bpf: add Error: prefix in check_extack helper Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 03/11] tools: bpftool: refactor argument parsing for prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 04/11] tools: bpftool: add support for loading programs for offload Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 05/11] tools: libbpf: expose the prog type guessing from section name logic Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 06/11] tools: bpftool: allow users to specify program type for prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 07/11] tools: libbpf: recognize offload neutral maps Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 08/11] tools: libbpf: add extended attributes version of bpf_object__open() Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 09/11] tools: bpftool: reimplement bpf_prog_load() for prog load Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 10/11] tools: libbpf: allow map reuse Jakub Kicinski
2018-07-04 2:54 ` [PATCH bpf-next 11/11] tools: bpftool: allow reuse of maps with bpftool prog load Jakub Kicinski
2018-07-05 8:35 ` Daniel Borkmann
2018-07-05 22:57 ` Jakub Kicinski
2018-07-06 7:16 ` Daniel Borkmann
2018-07-06 15:30 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.