* [PATCH v4 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
v4 changes:
* addressed another round of comments/style issues from Jakub Kicinski &
Quentin Monnet (thanks!)
* implemented bpf_object__pin_maps and bpf_object__pin_programs helpers and
used them in bpf_program__pin
* added new pin_name to bpf_program so bpf_program__pin
works with sections that contain '/'
* moved *loadall* command implementation into a separate patch
* added patch that implements *pinmaps* to pin maps when doing
load/loadall
v3 changes:
* (maybe) better cleanup for partial failure in bpf_object__pin
* added special case in bpf_program__pin for programs with single
instances
v2 changes:
* addressed comments/style issues from Jakub Kicinski & Quentin Monnet
* removed logic that populates jump table
* added cleanup for partial failure in bpf_object__pin
This patch series adds support for loading and attaching flow dissector
programs from the bpftool:
* first patch fixes flow dissector section name in the selftests (so
libbpf auto-detection works)
* second patch adds proper cleanup to bpf_object__pin, parts of which are now
being used to attach all flow dissector progs/maps
* third patch adds special case in bpf_program__pin for programs with
single instances (we don't create <prog>/0 pin anymore, just <prog>)
* forth patch adds pin_name to the bpf_program struct
which is now used as a pin name in bpf_program__pin et al
* fifth patch adds *loadall* command that pins all programs, not just
the first one
* sixth patch adds *pinmaps* argument to load/loadall to let users pin
all maps of the obj file
* seventh patch adds actual flow_dissector support to the bpftool and
an example
Stanislav Fomichev (7):
selftests/bpf: rename flow dissector section to flow_dissector
libbpf: cleanup after partial failure in bpf_object__pin
libbpf: bpf_program__pin: add special case for instances.nr == 1
libbpf: add internal pin_name
bpftool: add loadall command
bpftool: add pinmaps argument to the load/loadall
bpftool: support loading flow dissector
.../bpftool/Documentation/bpftool-prog.rst | 42 +-
tools/bpf/bpftool/bash-completion/bpftool | 21 +-
tools/bpf/bpftool/common.c | 31 +-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 185 ++++++---
tools/lib/bpf/libbpf.c | 364 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 546 insertions(+), 120 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
v4 changes:
* addressed another round of comments/style issues from Jakub Kicinski &
Quentin Monnet (thanks!)
* implemented bpf_object__pin_maps and bpf_object__pin_programs helpers and
used them in bpf_program__pin
* added new pin_name to bpf_program so bpf_program__pin
works with sections that contain '/'
* moved *loadall* command implementation into a separate patch
* added patch that implements *pinmaps* to pin maps when doing
load/loadall
v3 changes:
* (maybe) better cleanup for partial failure in bpf_object__pin
* added special case in bpf_program__pin for programs with single
instances
v2 changes:
* addressed comments/style issues from Jakub Kicinski & Quentin Monnet
* removed logic that populates jump table
* added cleanup for partial failure in bpf_object__pin
This patch series adds support for loading and attaching flow dissector
programs from the bpftool:
* first patch fixes flow dissector section name in the selftests (so
libbpf auto-detection works)
* second patch adds proper cleanup to bpf_object__pin, parts of which are now
being used to attach all flow dissector progs/maps
* third patch adds special case in bpf_program__pin for programs with
single instances (we don't create <prog>/0 pin anymore, just <prog>)
* forth patch adds pin_name to the bpf_program struct
which is now used as a pin name in bpf_program__pin et al
* fifth patch adds *loadall* command that pins all programs, not just
the first one
* sixth patch adds *pinmaps* argument to load/loadall to let users pin
all maps of the obj file
* seventh patch adds actual flow_dissector support to the bpftool and
an example
Stanislav Fomichev (7):
selftests/bpf: rename flow dissector section to flow_dissector
libbpf: cleanup after partial failure in bpf_object__pin
libbpf: bpf_program__pin: add special case for instances.nr == 1
libbpf: add internal pin_name
bpftool: add loadall command
bpftool: add pinmaps argument to the load/loadall
bpftool: support loading flow dissector
.../bpftool/Documentation/bpftool-prog.rst | 42 +-
tools/bpf/bpftool/bash-completion/bpftool | 21 +-
tools/bpf/bpftool/common.c | 31 +-
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 185 ++++++---
tools/lib/bpf/libbpf.c | 364 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 546 insertions(+), 120 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
Makes it compatible with the logic that derives program type
from section name in libbpf_prog_type_by_name.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
tools/testing/selftests/bpf/test_flow_dissector.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_flow.c b/tools/testing/selftests/bpf/bpf_flow.c
index 107350a7821d..b9798f558ca7 100644
--- a/tools/testing/selftests/bpf/bpf_flow.c
+++ b/tools/testing/selftests/bpf/bpf_flow.c
@@ -116,7 +116,7 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
return BPF_DROP;
}
-SEC("dissect")
+SEC("flow_dissector")
int _dissect(struct __sk_buff *skb)
{
if (!skb->vlan_present)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index c0fb073b5eab..d23d4da66b83 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -59,7 +59,7 @@ else
fi
# Attach BPF program
-./flow_dissector_load -p bpf_flow.o -s dissect
+./flow_dissector_load -p bpf_flow.o -s flow_dissector
# Setup
tc qdisc add dev lo ingress
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
Makes it compatible with the logic that derives program type
from section name in libbpf_prog_type_by_name.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
tools/testing/selftests/bpf/test_flow_dissector.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_flow.c b/tools/testing/selftests/bpf/bpf_flow.c
index 107350a7821d..b9798f558ca7 100644
--- a/tools/testing/selftests/bpf/bpf_flow.c
+++ b/tools/testing/selftests/bpf/bpf_flow.c
@@ -116,7 +116,7 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
return BPF_DROP;
}
-SEC("dissect")
+SEC("flow_dissector")
int _dissect(struct __sk_buff *skb)
{
if (!skb->vlan_present)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index c0fb073b5eab..d23d4da66b83 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -59,7 +59,7 @@ else
fi
# Attach BPF program
-./flow_dissector_load -p bpf_flow.o -s dissect
+./flow_dissector_load -p bpf_flow.o -s flow_dissector
# Setup
tc qdisc add dev lo ingress
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
Makes it compatible with the logic that derives program type
from section name in libbpf_prog_type_by_name.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
tools/testing/selftests/bpf/test_flow_dissector.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_flow.c b/tools/testing/selftests/bpf/bpf_flow.c
index 107350a7821d..b9798f558ca7 100644
--- a/tools/testing/selftests/bpf/bpf_flow.c
+++ b/tools/testing/selftests/bpf/bpf_flow.c
@@ -116,7 +116,7 @@ static __always_inline int parse_eth_proto(struct __sk_buff *skb, __be16 proto)
return BPF_DROP;
}
-SEC("dissect")
+SEC("flow_dissector")
int _dissect(struct __sk_buff *skb)
{
if (!skb->vlan_present)
diff --git a/tools/testing/selftests/bpf/test_flow_dissector.sh b/tools/testing/selftests/bpf/test_flow_dissector.sh
index c0fb073b5eab..d23d4da66b83 100755
--- a/tools/testing/selftests/bpf/test_flow_dissector.sh
+++ b/tools/testing/selftests/bpf/test_flow_dissector.sh
@@ -59,7 +59,7 @@ else
fi
# Attach BPF program
-./flow_dissector_load -p bpf_flow.o -s dissect
+./flow_dissector_load -p bpf_flow.o -s flow_dissector
# Setup
tc qdisc add dev lo ingress
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
bpftool will use bpf_object__pin in the next commits to pin all programs
and maps from the file; in case of a partial failure, we need to get
back to the clean state (undo previous program/map pins).
As part of a cleanup, I've added and exported separate routines to
pin all maps (bpf_object__pin_maps) and progs (bpf_object__pin_programs)
of an object.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 328 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 323 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..f8590490a9dd 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1699,6 +1699,34 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
return 0;
}
+int bpf_program__unpin_instance(struct bpf_program *prog, const char *path,
+ int instance)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (instance < 0 || instance >= prog->instances.nr) {
+ pr_warning("invalid prog instance %d of prog %s (max %d)\n",
+ instance, prog->section_name, prog->instances.nr);
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned program '%s'\n", path);
+
+ return 0;
+}
+
static int make_dir(const char *path)
{
char *cp, errmsg[STRERR_BUFSIZE];
@@ -1737,6 +1765,64 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
if (err)
return err;
+ for (i = 0; i < prog->instances.nr; i++) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin;
+ }
+
+ err = bpf_program__pin_instance(prog, buf, i);
+ if (err)
+ goto err_unpin;
+ }
+
+ return 0;
+
+err_unpin:
+ for (i = i - 1; i >= 0; i--) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin_instance(prog, buf, i);
+ }
+
+ rmdir(path);
+
+ return err;
+}
+
+int bpf_program__unpin(struct bpf_program *prog, const char *path)
+{
+ int i, err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (prog->instances.nr <= 0) {
+ pr_warning("no instances of prog %s to pin\n",
+ prog->section_name);
+ return -EINVAL;
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
@@ -1747,11 +1833,15 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin_instance(prog, buf, i);
+ err = bpf_program__unpin_instance(prog, buf, i);
if (err)
return err;
}
+ err = rmdir(path);
+ if (err)
+ return -errno;
+
return 0;
}
@@ -1776,12 +1866,33 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
pr_debug("pinned map '%s'\n", path);
+
return 0;
}
-int bpf_object__pin(struct bpf_object *obj, const char *path)
+int bpf_map__unpin(struct bpf_map *map, const char *path)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (map == NULL) {
+ pr_warning("invalid map pointer\n");
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned map '%s'\n", path);
+
+ return 0;
+}
+
+int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
{
- struct bpf_program *prog;
struct bpf_map *map;
int err;
@@ -1797,6 +1908,55 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
if (err)
return err;
+ bpf_map__for_each(map, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_maps;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_maps;
+ }
+
+ err = bpf_map__pin(map, buf);
+ if (err)
+ goto err_unpin_maps;
+ }
+
+ return 0;
+
+err_unpin_maps:
+ for (map = bpf_map__prev(map, obj);
+ map != NULL;
+ map = bpf_map__prev(map, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_map__unpin(map, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
+{
+ struct bpf_map *map;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_map__for_each(map, obj) {
char buf[PATH_MAX];
int len;
@@ -1808,11 +1968,80 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_map__pin(map, buf);
+ err = bpf_map__unpin(map, buf);
if (err)
return err;
}
+ return 0;
+}
+
+int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
+ if (!obj->loaded) {
+ pr_warning("object not yet loaded; load it first\n");
+ return -ENOENT;
+ }
+
+ err = make_dir(path);
+ if (err)
+ return err;
+
+ bpf_object__for_each_program(prog, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_programs;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_programs;
+ }
+
+ err = bpf_program__pin(prog, buf);
+ if (err)
+ goto err_unpin_programs;
+ }
+
+ return 0;
+
+err_unpin_programs:
+ for (prog = bpf_program__prev(prog, obj);
+ prog != NULL;
+ prog = bpf_program__prev(prog, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin(prog, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_object__for_each_program(prog, obj) {
char buf[PATH_MAX];
int len;
@@ -1824,7 +2053,7 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin(prog, buf);
+ err = bpf_program__unpin(prog, buf);
if (err)
return err;
}
@@ -1832,6 +2061,23 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0;
}
+int bpf_object__pin(struct bpf_object *obj, const char *path)
+{
+ int err;
+
+ err = bpf_object__pin_maps(obj, path);
+ if (err)
+ return err;
+
+ err = bpf_object__pin_programs(obj, path);
+ if (err) {
+ bpf_object__unpin_maps(obj, path);
+ return err;
+ }
+
+ return 0;
+}
+
void bpf_object__close(struct bpf_object *obj)
{
size_t i;
@@ -1918,23 +2164,20 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
if (!obj->programs)
return NULL;
- /* First handler */
- if (prev == NULL)
- return &obj->programs[0];
- if (prev->obj != obj) {
+ if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (prev - obj->programs) + 1;
- if (idx >= obj->nr_programs)
+ idx = (p - obj->programs) + i;
+ if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
}
@@ -1944,8 +2187,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
+ if (prev == NULL)
+ return obj->programs;
+
do {
- prog = __bpf_program__next(prog, obj);
+ prog = __bpf_program__iter(prog, obj, 1);
+ } while (prog && bpf_program__is_function_storage(prog, obj));
+
+ return prog;
+}
+
+struct bpf_program *
+bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
+{
+ struct bpf_program *prog = next;
+
+ if (next == NULL) {
+ if (!obj->nr_programs)
+ return NULL;
+ return obj->programs + obj->nr_programs - 1;
+ }
+
+ do {
+ prog = __bpf_program__iter(prog, obj, -1);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2272,10 +2536,10 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
map->map_ifindex = ifindex;
}
-struct bpf_map *
-bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+static struct bpf_map *
+__bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
struct bpf_map *s, *e;
if (!obj || !obj->maps)
@@ -2284,21 +2548,39 @@ bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
s = obj->maps;
e = obj->maps + obj->nr_maps;
- if (prev == NULL)
- return s;
-
- if ((prev < s) || (prev >= e)) {
+ if ((m < s) || (m >= e)) {
pr_warning("error in %s: map handler doesn't belong to object\n",
__func__);
return NULL;
}
- idx = (prev - obj->maps) + 1;
- if (idx >= obj->nr_maps)
+ idx = (m - obj->maps) + i;
+ if (idx >= obj->nr_maps || idx < 0)
return NULL;
return &obj->maps[idx];
}
+struct bpf_map *
+bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+{
+ if (prev == NULL)
+ return obj->maps;
+
+ return __bpf_map__iter(prev, obj, 1);
+}
+
+struct bpf_map *
+bpf_map__prev(struct bpf_map *next, struct bpf_object *obj)
+{
+ if (next == NULL) {
+ if (!obj->nr_maps)
+ return NULL;
+ return obj->maps + obj->nr_maps - 1;
+ }
+
+ return __bpf_map__iter(next, obj, -1);
+}
+
struct bpf_map *
bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1f3468dad8b2..b1686a787102 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -71,6 +71,13 @@ struct bpf_object *__bpf_object__open_xattr(struct bpf_object_open_attr *attr,
LIBBPF_API struct bpf_object *bpf_object__open_buffer(void *obj_buf,
size_t obj_buf_sz,
const char *name);
+LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
+LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
+ const char *path);
LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path);
LIBBPF_API void bpf_object__close(struct bpf_object *object);
@@ -112,6 +119,9 @@ LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
(pos) != NULL; \
(pos) = bpf_program__next((pos), (obj)))
+LIBBPF_API struct bpf_program *bpf_program__prev(struct bpf_program *prog,
+ struct bpf_object *obj);
+
typedef void (*bpf_program_clear_priv_t)(struct bpf_program *,
void *);
@@ -131,7 +141,11 @@ LIBBPF_API int bpf_program__fd(struct bpf_program *prog);
LIBBPF_API int bpf_program__pin_instance(struct bpf_program *prog,
const char *path,
int instance);
+LIBBPF_API int bpf_program__unpin_instance(struct bpf_program *prog,
+ const char *path,
+ int instance);
LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
+LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
struct bpf_insn;
@@ -260,6 +274,9 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
(pos) != NULL; \
(pos) = bpf_map__next((pos), (obj)))
+LIBBPF_API struct bpf_map *
+bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
+
LIBBPF_API int bpf_map__fd(struct bpf_map *map);
LIBBPF_API const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
LIBBPF_API const char *bpf_map__name(struct bpf_map *map);
@@ -274,6 +291,7 @@ LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
LIBBPF_API bool bpf_map__is_offload_neutral(struct bpf_map *map);
LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
+LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
LIBBPF_API long libbpf_get_error(const void *ptr);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
bpftool will use bpf_object__pin in the next commits to pin all programs
and maps from the file; in case of a partial failure, we need to get
back to the clean state (undo previous program/map pins).
As part of a cleanup, I've added and exported separate routines to
pin all maps (bpf_object__pin_maps) and progs (bpf_object__pin_programs)
of an object.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 328 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 323 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..f8590490a9dd 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1699,6 +1699,34 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
return 0;
}
+int bpf_program__unpin_instance(struct bpf_program *prog, const char *path,
+ int instance)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (instance < 0 || instance >= prog->instances.nr) {
+ pr_warning("invalid prog instance %d of prog %s (max %d)\n",
+ instance, prog->section_name, prog->instances.nr);
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned program '%s'\n", path);
+
+ return 0;
+}
+
static int make_dir(const char *path)
{
char *cp, errmsg[STRERR_BUFSIZE];
@@ -1737,6 +1765,64 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
if (err)
return err;
+ for (i = 0; i < prog->instances.nr; i++) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin;
+ }
+
+ err = bpf_program__pin_instance(prog, buf, i);
+ if (err)
+ goto err_unpin;
+ }
+
+ return 0;
+
+err_unpin:
+ for (i = i - 1; i >= 0; i--) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin_instance(prog, buf, i);
+ }
+
+ rmdir(path);
+
+ return err;
+}
+
+int bpf_program__unpin(struct bpf_program *prog, const char *path)
+{
+ int i, err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (prog->instances.nr <= 0) {
+ pr_warning("no instances of prog %s to pin\n",
+ prog->section_name);
+ return -EINVAL;
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
@@ -1747,11 +1833,15 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin_instance(prog, buf, i);
+ err = bpf_program__unpin_instance(prog, buf, i);
if (err)
return err;
}
+ err = rmdir(path);
+ if (err)
+ return -errno;
+
return 0;
}
@@ -1776,12 +1866,33 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
pr_debug("pinned map '%s'\n", path);
+
return 0;
}
-int bpf_object__pin(struct bpf_object *obj, const char *path)
+int bpf_map__unpin(struct bpf_map *map, const char *path)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (map == NULL) {
+ pr_warning("invalid map pointer\n");
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned map '%s'\n", path);
+
+ return 0;
+}
+
+int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
{
- struct bpf_program *prog;
struct bpf_map *map;
int err;
@@ -1797,6 +1908,55 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
if (err)
return err;
+ bpf_map__for_each(map, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_maps;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_maps;
+ }
+
+ err = bpf_map__pin(map, buf);
+ if (err)
+ goto err_unpin_maps;
+ }
+
+ return 0;
+
+err_unpin_maps:
+ for (map = bpf_map__prev(map, obj);
+ map != NULL;
+ map = bpf_map__prev(map, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_map__unpin(map, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
+{
+ struct bpf_map *map;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_map__for_each(map, obj) {
char buf[PATH_MAX];
int len;
@@ -1808,11 +1968,80 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_map__pin(map, buf);
+ err = bpf_map__unpin(map, buf);
if (err)
return err;
}
+ return 0;
+}
+
+int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
+ if (!obj->loaded) {
+ pr_warning("object not yet loaded; load it first\n");
+ return -ENOENT;
+ }
+
+ err = make_dir(path);
+ if (err)
+ return err;
+
+ bpf_object__for_each_program(prog, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_programs;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_programs;
+ }
+
+ err = bpf_program__pin(prog, buf);
+ if (err)
+ goto err_unpin_programs;
+ }
+
+ return 0;
+
+err_unpin_programs:
+ for (prog = bpf_program__prev(prog, obj);
+ prog != NULL;
+ prog = bpf_program__prev(prog, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin(prog, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_object__for_each_program(prog, obj) {
char buf[PATH_MAX];
int len;
@@ -1824,7 +2053,7 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin(prog, buf);
+ err = bpf_program__unpin(prog, buf);
if (err)
return err;
}
@@ -1832,6 +2061,23 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0;
}
+int bpf_object__pin(struct bpf_object *obj, const char *path)
+{
+ int err;
+
+ err = bpf_object__pin_maps(obj, path);
+ if (err)
+ return err;
+
+ err = bpf_object__pin_programs(obj, path);
+ if (err) {
+ bpf_object__unpin_maps(obj, path);
+ return err;
+ }
+
+ return 0;
+}
+
void bpf_object__close(struct bpf_object *obj)
{
size_t i;
@@ -1918,23 +2164,20 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
if (!obj->programs)
return NULL;
- /* First handler */
- if (prev == NULL)
- return &obj->programs[0];
- if (prev->obj != obj) {
+ if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (prev - obj->programs) + 1;
- if (idx >= obj->nr_programs)
+ idx = (p - obj->programs) + i;
+ if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
}
@@ -1944,8 +2187,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
+ if (prev == NULL)
+ return obj->programs;
+
do {
- prog = __bpf_program__next(prog, obj);
+ prog = __bpf_program__iter(prog, obj, 1);
+ } while (prog && bpf_program__is_function_storage(prog, obj));
+
+ return prog;
+}
+
+struct bpf_program *
+bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
+{
+ struct bpf_program *prog = next;
+
+ if (next == NULL) {
+ if (!obj->nr_programs)
+ return NULL;
+ return obj->programs + obj->nr_programs - 1;
+ }
+
+ do {
+ prog = __bpf_program__iter(prog, obj, -1);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2272,10 +2536,10 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
map->map_ifindex = ifindex;
}
-struct bpf_map *
-bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+static struct bpf_map *
+__bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
struct bpf_map *s, *e;
if (!obj || !obj->maps)
@@ -2284,21 +2548,39 @@ bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
s = obj->maps;
e = obj->maps + obj->nr_maps;
- if (prev == NULL)
- return s;
-
- if ((prev < s) || (prev >= e)) {
+ if ((m < s) || (m >= e)) {
pr_warning("error in %s: map handler doesn't belong to object\n",
__func__);
return NULL;
}
- idx = (prev - obj->maps) + 1;
- if (idx >= obj->nr_maps)
+ idx = (m - obj->maps) + i;
+ if (idx >= obj->nr_maps || idx < 0)
return NULL;
return &obj->maps[idx];
}
+struct bpf_map *
+bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+{
+ if (prev == NULL)
+ return obj->maps;
+
+ return __bpf_map__iter(prev, obj, 1);
+}
+
+struct bpf_map *
+bpf_map__prev(struct bpf_map *next, struct bpf_object *obj)
+{
+ if (next == NULL) {
+ if (!obj->nr_maps)
+ return NULL;
+ return obj->maps + obj->nr_maps - 1;
+ }
+
+ return __bpf_map__iter(next, obj, -1);
+}
+
struct bpf_map *
bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1f3468dad8b2..b1686a787102 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -71,6 +71,13 @@ struct bpf_object *__bpf_object__open_xattr(struct bpf_object_open_attr *attr,
LIBBPF_API struct bpf_object *bpf_object__open_buffer(void *obj_buf,
size_t obj_buf_sz,
const char *name);
+LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
+LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
+ const char *path);
LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path);
LIBBPF_API void bpf_object__close(struct bpf_object *object);
@@ -112,6 +119,9 @@ LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
(pos) != NULL; \
(pos) = bpf_program__next((pos), (obj)))
+LIBBPF_API struct bpf_program *bpf_program__prev(struct bpf_program *prog,
+ struct bpf_object *obj);
+
typedef void (*bpf_program_clear_priv_t)(struct bpf_program *,
void *);
@@ -131,7 +141,11 @@ LIBBPF_API int bpf_program__fd(struct bpf_program *prog);
LIBBPF_API int bpf_program__pin_instance(struct bpf_program *prog,
const char *path,
int instance);
+LIBBPF_API int bpf_program__unpin_instance(struct bpf_program *prog,
+ const char *path,
+ int instance);
LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
+LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
struct bpf_insn;
@@ -260,6 +274,9 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
(pos) != NULL; \
(pos) = bpf_map__next((pos), (obj)))
+LIBBPF_API struct bpf_map *
+bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
+
LIBBPF_API int bpf_map__fd(struct bpf_map *map);
LIBBPF_API const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
LIBBPF_API const char *bpf_map__name(struct bpf_map *map);
@@ -274,6 +291,7 @@ LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
LIBBPF_API bool bpf_map__is_offload_neutral(struct bpf_map *map);
LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
+LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
LIBBPF_API long libbpf_get_error(const void *ptr);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
bpftool will use bpf_object__pin in the next commits to pin all programs
and maps from the file; in case of a partial failure, we need to get
back to the clean state (undo previous program/map pins).
As part of a cleanup, I've added and exported separate routines to
pin all maps (bpf_object__pin_maps) and progs (bpf_object__pin_programs)
of an object.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 328 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 323 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..f8590490a9dd 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1699,6 +1699,34 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
return 0;
}
+int bpf_program__unpin_instance(struct bpf_program *prog, const char *path,
+ int instance)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (instance < 0 || instance >= prog->instances.nr) {
+ pr_warning("invalid prog instance %d of prog %s (max %d)\n",
+ instance, prog->section_name, prog->instances.nr);
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned program '%s'\n", path);
+
+ return 0;
+}
+
static int make_dir(const char *path)
{
char *cp, errmsg[STRERR_BUFSIZE];
@@ -1737,6 +1765,64 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
if (err)
return err;
+ for (i = 0; i < prog->instances.nr; i++) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin;
+ }
+
+ err = bpf_program__pin_instance(prog, buf, i);
+ if (err)
+ goto err_unpin;
+ }
+
+ return 0;
+
+err_unpin:
+ for (i = i - 1; i >= 0; i--) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%d", path, i);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin_instance(prog, buf, i);
+ }
+
+ rmdir(path);
+
+ return err;
+}
+
+int bpf_program__unpin(struct bpf_program *prog, const char *path)
+{
+ int i, err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (prog == NULL) {
+ pr_warning("invalid program pointer\n");
+ return -EINVAL;
+ }
+
+ if (prog->instances.nr <= 0) {
+ pr_warning("no instances of prog %s to pin\n",
+ prog->section_name);
+ return -EINVAL;
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
@@ -1747,11 +1833,15 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin_instance(prog, buf, i);
+ err = bpf_program__unpin_instance(prog, buf, i);
if (err)
return err;
}
+ err = rmdir(path);
+ if (err)
+ return -errno;
+
return 0;
}
@@ -1776,12 +1866,33 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
pr_debug("pinned map '%s'\n", path);
+
return 0;
}
-int bpf_object__pin(struct bpf_object *obj, const char *path)
+int bpf_map__unpin(struct bpf_map *map, const char *path)
+{
+ int err;
+
+ err = check_path(path);
+ if (err)
+ return err;
+
+ if (map == NULL) {
+ pr_warning("invalid map pointer\n");
+ return -EINVAL;
+ }
+
+ err = unlink(path);
+ if (err != 0)
+ return -errno;
+ pr_debug("unpinned map '%s'\n", path);
+
+ return 0;
+}
+
+int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
{
- struct bpf_program *prog;
struct bpf_map *map;
int err;
@@ -1797,6 +1908,55 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
if (err)
return err;
+ bpf_map__for_each(map, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_maps;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_maps;
+ }
+
+ err = bpf_map__pin(map, buf);
+ if (err)
+ goto err_unpin_maps;
+ }
+
+ return 0;
+
+err_unpin_maps:
+ for (map = bpf_map__prev(map, obj);
+ map != NULL;
+ map = bpf_map__prev(map, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ bpf_map__name(map));
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_map__unpin(map, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
+{
+ struct bpf_map *map;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_map__for_each(map, obj) {
char buf[PATH_MAX];
int len;
@@ -1808,11 +1968,80 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_map__pin(map, buf);
+ err = bpf_map__unpin(map, buf);
if (err)
return err;
}
+ return 0;
+}
+
+int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
+ if (!obj->loaded) {
+ pr_warning("object not yet loaded; load it first\n");
+ return -ENOENT;
+ }
+
+ err = make_dir(path);
+ if (err)
+ return err;
+
+ bpf_object__for_each_program(prog, obj) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0) {
+ err = -EINVAL;
+ goto err_unpin_programs;
+ } else if (len >= PATH_MAX) {
+ err = -ENAMETOOLONG;
+ goto err_unpin_programs;
+ }
+
+ err = bpf_program__pin(prog, buf);
+ if (err)
+ goto err_unpin_programs;
+ }
+
+ return 0;
+
+err_unpin_programs:
+ for (prog = bpf_program__prev(prog, obj);
+ prog != NULL;
+ prog = bpf_program__prev(prog, obj)) {
+ char buf[PATH_MAX];
+ int len;
+
+ len = snprintf(buf, PATH_MAX, "%s/%s", path,
+ prog->section_name);
+ if (len < 0)
+ continue;
+ else if (len >= PATH_MAX)
+ continue;
+
+ bpf_program__unpin(prog, buf);
+ }
+
+ return err;
+}
+
+int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
+{
+ struct bpf_program *prog;
+ int err;
+
+ if (!obj)
+ return -ENOENT;
+
bpf_object__for_each_program(prog, obj) {
char buf[PATH_MAX];
int len;
@@ -1824,7 +2053,7 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
else if (len >= PATH_MAX)
return -ENAMETOOLONG;
- err = bpf_program__pin(prog, buf);
+ err = bpf_program__unpin(prog, buf);
if (err)
return err;
}
@@ -1832,6 +2061,23 @@ int bpf_object__pin(struct bpf_object *obj, const char *path)
return 0;
}
+int bpf_object__pin(struct bpf_object *obj, const char *path)
+{
+ int err;
+
+ err = bpf_object__pin_maps(obj, path);
+ if (err)
+ return err;
+
+ err = bpf_object__pin_programs(obj, path);
+ if (err) {
+ bpf_object__unpin_maps(obj, path);
+ return err;
+ }
+
+ return 0;
+}
+
void bpf_object__close(struct bpf_object *obj)
{
size_t i;
@@ -1918,23 +2164,20 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
if (!obj->programs)
return NULL;
- /* First handler */
- if (prev == NULL)
- return &obj->programs[0];
- if (prev->obj != obj) {
+ if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (prev - obj->programs) + 1;
- if (idx >= obj->nr_programs)
+ idx = (p - obj->programs) + i;
+ if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
}
@@ -1944,8 +2187,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
+ if (prev == NULL)
+ return obj->programs;
+
do {
- prog = __bpf_program__next(prog, obj);
+ prog = __bpf_program__iter(prog, obj, 1);
+ } while (prog && bpf_program__is_function_storage(prog, obj));
+
+ return prog;
+}
+
+struct bpf_program *
+bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
+{
+ struct bpf_program *prog = next;
+
+ if (next == NULL) {
+ if (!obj->nr_programs)
+ return NULL;
+ return obj->programs + obj->nr_programs - 1;
+ }
+
+ do {
+ prog = __bpf_program__iter(prog, obj, -1);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2272,10 +2536,10 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
map->map_ifindex = ifindex;
}
-struct bpf_map *
-bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+static struct bpf_map *
+__bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
{
- size_t idx;
+ ssize_t idx;
struct bpf_map *s, *e;
if (!obj || !obj->maps)
@@ -2284,21 +2548,39 @@ bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
s = obj->maps;
e = obj->maps + obj->nr_maps;
- if (prev == NULL)
- return s;
-
- if ((prev < s) || (prev >= e)) {
+ if ((m < s) || (m >= e)) {
pr_warning("error in %s: map handler doesn't belong to object\n",
__func__);
return NULL;
}
- idx = (prev - obj->maps) + 1;
- if (idx >= obj->nr_maps)
+ idx = (m - obj->maps) + i;
+ if (idx >= obj->nr_maps || idx < 0)
return NULL;
return &obj->maps[idx];
}
+struct bpf_map *
+bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
+{
+ if (prev == NULL)
+ return obj->maps;
+
+ return __bpf_map__iter(prev, obj, 1);
+}
+
+struct bpf_map *
+bpf_map__prev(struct bpf_map *next, struct bpf_object *obj)
+{
+ if (next == NULL) {
+ if (!obj->nr_maps)
+ return NULL;
+ return obj->maps + obj->nr_maps - 1;
+ }
+
+ return __bpf_map__iter(next, obj, -1);
+}
+
struct bpf_map *
bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 1f3468dad8b2..b1686a787102 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -71,6 +71,13 @@ struct bpf_object *__bpf_object__open_xattr(struct bpf_object_open_attr *attr,
LIBBPF_API struct bpf_object *bpf_object__open_buffer(void *obj_buf,
size_t obj_buf_sz,
const char *name);
+LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
+LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
+ const char *path);
+LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
+ const char *path);
LIBBPF_API int bpf_object__pin(struct bpf_object *object, const char *path);
LIBBPF_API void bpf_object__close(struct bpf_object *object);
@@ -112,6 +119,9 @@ LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
(pos) != NULL; \
(pos) = bpf_program__next((pos), (obj)))
+LIBBPF_API struct bpf_program *bpf_program__prev(struct bpf_program *prog,
+ struct bpf_object *obj);
+
typedef void (*bpf_program_clear_priv_t)(struct bpf_program *,
void *);
@@ -131,7 +141,11 @@ LIBBPF_API int bpf_program__fd(struct bpf_program *prog);
LIBBPF_API int bpf_program__pin_instance(struct bpf_program *prog,
const char *path,
int instance);
+LIBBPF_API int bpf_program__unpin_instance(struct bpf_program *prog,
+ const char *path,
+ int instance);
LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
+LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
struct bpf_insn;
@@ -260,6 +274,9 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
(pos) != NULL; \
(pos) = bpf_map__next((pos), (obj)))
+LIBBPF_API struct bpf_map *
+bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
+
LIBBPF_API int bpf_map__fd(struct bpf_map *map);
LIBBPF_API const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
LIBBPF_API const char *bpf_map__name(struct bpf_map *map);
@@ -274,6 +291,7 @@ LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
LIBBPF_API bool bpf_map__is_offload_neutral(struct bpf_map *map);
LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
+LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
LIBBPF_API long libbpf_get_error(const void *ptr);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 3:37 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:37 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> + for (map = bpf_map__prev(map, obj);
> + map != NULL;
> + map = bpf_map__prev(map, obj)) {
nit pick: if you need to respin all these for loops on error paths could
have been more concise while loops
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 3:37 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:37 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> + for (map = bpf_map__prev(map, obj);
> + map != NULL;
> + map = bpf_map__prev(map, obj)) {
nit pick: if you need to respin all these for loops on error paths could
have been more concise while loops
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 3:37 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 3:37 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> + for (map = bpf_map__prev(map, obj);
> + map != NULL;
> + map = bpf_map__prev(map, obj)) {
nit pick: if you need to respin all these for loops on error paths could
have been more concise while loops
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-09 3:37 ` jakub.kicinski
(?)
@ 2018-11-09 15:41 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 15:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On 11/08, Jakub Kicinski wrote:
> On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> > + for (map = bpf_map__prev(map, obj);
> > + map != NULL;
> > + map = bpf_map__prev(map, obj)) {
>
> nit pick: if you need to respin all these for loops on error paths could
> have been more concise while loops
Agreed with everything, will address this one and the other comments in v5.
Thank you for another thorough review!
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 15:41 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 15:41 UTC (permalink / raw)
On 11/08, Jakub Kicinski wrote:
> On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> > + for (map = bpf_map__prev(map, obj);
> > + map != NULL;
> > + map = bpf_map__prev(map, obj)) {
>
> nit pick: if you need to respin all these for loops on error paths could
> have been more concise while loops
Agreed with everything, will address this one and the other comments in v5.
Thank you for another thorough review!
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 15:41 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 15:41 UTC (permalink / raw)
On 11/08, Jakub Kicinski wrote:
> On Thu, 8 Nov 2018 16:22:08 -0800, Stanislav Fomichev wrote:
> > + for (map = bpf_map__prev(map, obj);
> > + map != NULL;
> > + map = bpf_map__prev(map, obj)) {
>
> nit pick: if you need to respin all these for loops on error paths could
> have been more concise while loops
Agreed with everything, will address this one and the other comments in v5.
Thank you for another thorough review!
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
When bpf_program has only one instance, don't create a subdirectory with
per-instance pin files (<prog>/0). Instead, just create a single pin file
for that single instance. This simplifies object pinning by not creating
unnecessary subdirectories.
This can potentially break existing users that depend on the case
where '/0' is always created. However, I couldn't find any serious
usage of bpf_program__pin inside the kernel tree and I suppose there
should be none outside.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f8590490a9dd..cfa269c91e11 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1761,6 +1761,11 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__pin_instance(prog, path, 0);
+ }
+
err = make_dir(path);
if (err)
return err;
@@ -1823,6 +1828,11 @@ int bpf_program__unpin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__unpin_instance(prog, path, 0);
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
When bpf_program has only one instance, don't create a subdirectory with
per-instance pin files (<prog>/0). Instead, just create a single pin file
for that single instance. This simplifies object pinning by not creating
unnecessary subdirectories.
This can potentially break existing users that depend on the case
where '/0' is always created. However, I couldn't find any serious
usage of bpf_program__pin inside the kernel tree and I suppose there
should be none outside.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f8590490a9dd..cfa269c91e11 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1761,6 +1761,11 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__pin_instance(prog, path, 0);
+ }
+
err = make_dir(path);
if (err)
return err;
@@ -1823,6 +1828,11 @@ int bpf_program__unpin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__unpin_instance(prog, path, 0);
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
When bpf_program has only one instance, don't create a subdirectory with
per-instance pin files (<prog>/0). Instead, just create a single pin file
for that single instance. This simplifies object pinning by not creating
unnecessary subdirectories.
This can potentially break existing users that depend on the case
where '/0' is always created. However, I couldn't find any serious
usage of bpf_program__pin inside the kernel tree and I suppose there
should be none outside.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f8590490a9dd..cfa269c91e11 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1761,6 +1761,11 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__pin_instance(prog, path, 0);
+ }
+
err = make_dir(path);
if (err)
return err;
@@ -1823,6 +1828,11 @@ int bpf_program__unpin(struct bpf_program *prog, const char *path)
return -EINVAL;
}
+ if (prog->instances.nr == 1) {
+ /* don't create subdirs when pinning single instance */
+ return bpf_program__unpin_instance(prog, path, 0);
+ }
+
for (i = 0; i < prog->instances.nr; i++) {
char buf[PATH_MAX];
int len;
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
pin_name is the same as section_name where '/' is replaced
by '_'. bpf_object__pin_programs is converted to use pin_name
to avoid the situation where section_name would require creating another
subdirectory for a pin (as, for example, when calling bpf_object__pin_programs
for programs in sections like "cgroup/connect6").
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cfa269c91e11..38dbeb113eeb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -124,6 +124,10 @@ struct bpf_program {
char *name;
int prog_ifindex;
char *section_name;
+ /* section_name with / replaced by _; makes recursive pinning
+ * in bpf_object__pin_programs easier
+ */
+ char *pin_name;
struct bpf_insn *insns;
size_t insns_cnt, main_prog_cnt;
enum bpf_prog_type type;
@@ -253,6 +257,7 @@ static void bpf_program__exit(struct bpf_program *prog)
bpf_program__unload(prog);
zfree(&prog->name);
zfree(&prog->section_name);
+ zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);
@@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name;
+
+ name = strdup(prog->section_name);
+ for (char *p = name; p && *p; p++)
+ if (*p == '/')
+ *p = '_';
+
+ return name;
+}
+
static int
bpf_program__init(void *data, size_t size, char *section_name, int idx,
struct bpf_program *prog)
@@ -279,6 +296,13 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
goto errout;
}
+ prog->pin_name = __bpf_program__pin_name(prog);
+ if (!prog->pin_name) {
+ pr_warning("failed to alloc pin name for prog under section(%d) %s\n",
+ idx, section_name);
+ goto errout;
+ }
+
prog->insns = malloc(size);
if (!prog->insns) {
pr_warning("failed to alloc insns for prog under section %s\n",
@@ -2008,7 +2032,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0) {
err = -EINVAL;
goto err_unpin_programs;
@@ -2032,7 +2056,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
continue;
else if (len >= PATH_MAX)
@@ -2057,7 +2081,7 @@ int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
return -EINVAL;
else if (len >= PATH_MAX)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
pin_name is the same as section_name where '/' is replaced
by '_'. bpf_object__pin_programs is converted to use pin_name
to avoid the situation where section_name would require creating another
subdirectory for a pin (as, for example, when calling bpf_object__pin_programs
for programs in sections like "cgroup/connect6").
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cfa269c91e11..38dbeb113eeb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -124,6 +124,10 @@ struct bpf_program {
char *name;
int prog_ifindex;
char *section_name;
+ /* section_name with / replaced by _; makes recursive pinning
+ * in bpf_object__pin_programs easier
+ */
+ char *pin_name;
struct bpf_insn *insns;
size_t insns_cnt, main_prog_cnt;
enum bpf_prog_type type;
@@ -253,6 +257,7 @@ static void bpf_program__exit(struct bpf_program *prog)
bpf_program__unload(prog);
zfree(&prog->name);
zfree(&prog->section_name);
+ zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);
@@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name;
+
+ name = strdup(prog->section_name);
+ for (char *p = name; p && *p; p++)
+ if (*p == '/')
+ *p = '_';
+
+ return name;
+}
+
static int
bpf_program__init(void *data, size_t size, char *section_name, int idx,
struct bpf_program *prog)
@@ -279,6 +296,13 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
goto errout;
}
+ prog->pin_name = __bpf_program__pin_name(prog);
+ if (!prog->pin_name) {
+ pr_warning("failed to alloc pin name for prog under section(%d) %s\n",
+ idx, section_name);
+ goto errout;
+ }
+
prog->insns = malloc(size);
if (!prog->insns) {
pr_warning("failed to alloc insns for prog under section %s\n",
@@ -2008,7 +2032,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0) {
err = -EINVAL;
goto err_unpin_programs;
@@ -2032,7 +2056,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
continue;
else if (len >= PATH_MAX)
@@ -2057,7 +2081,7 @@ int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
return -EINVAL;
else if (len >= PATH_MAX)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
pin_name is the same as section_name where '/' is replaced
by '_'. bpf_object__pin_programs is converted to use pin_name
to avoid the situation where section_name would require creating another
subdirectory for a pin (as, for example, when calling bpf_object__pin_programs
for programs in sections like "cgroup/connect6").
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
tools/lib/bpf/libbpf.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cfa269c91e11..38dbeb113eeb 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -124,6 +124,10 @@ struct bpf_program {
char *name;
int prog_ifindex;
char *section_name;
+ /* section_name with / replaced by _; makes recursive pinning
+ * in bpf_object__pin_programs easier
+ */
+ char *pin_name;
struct bpf_insn *insns;
size_t insns_cnt, main_prog_cnt;
enum bpf_prog_type type;
@@ -253,6 +257,7 @@ static void bpf_program__exit(struct bpf_program *prog)
bpf_program__unload(prog);
zfree(&prog->name);
zfree(&prog->section_name);
+ zfree(&prog->pin_name);
zfree(&prog->insns);
zfree(&prog->reloc_desc);
@@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name;
+
+ name = strdup(prog->section_name);
+ for (char *p = name; p && *p; p++)
+ if (*p == '/')
+ *p = '_';
+
+ return name;
+}
+
static int
bpf_program__init(void *data, size_t size, char *section_name, int idx,
struct bpf_program *prog)
@@ -279,6 +296,13 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx,
goto errout;
}
+ prog->pin_name = __bpf_program__pin_name(prog);
+ if (!prog->pin_name) {
+ pr_warning("failed to alloc pin name for prog under section(%d) %s\n",
+ idx, section_name);
+ goto errout;
+ }
+
prog->insns = malloc(size);
if (!prog->insns) {
pr_warning("failed to alloc insns for prog under section %s\n",
@@ -2008,7 +2032,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0) {
err = -EINVAL;
goto err_unpin_programs;
@@ -2032,7 +2056,7 @@ int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
continue;
else if (len >= PATH_MAX)
@@ -2057,7 +2081,7 @@ int bpf_object__unpin_programs(struct bpf_object *obj, const char *path)
int len;
len = snprintf(buf, PATH_MAX, "%s/%s", path,
- prog->section_name);
+ prog->pin_name);
if (len < 0)
return -EINVAL;
else if (len >= PATH_MAX)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 3:38 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:38 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Thu, 8 Nov 2018 16:22:10 -0800, Stanislav Fomichev wrote:
> @@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
> prog->idx = -1;
> }
>
> +static char *__bpf_program__pin_name(struct bpf_program *prog)
> +{
> + char *name;
> +
> + name = strdup(prog->section_name);
> + for (char *p = name; p && *p; p++)
Useful patch! I'm not sure about libbpf but in the kernel we don't do
C99 variable declarations inside for loop init. Perhaps better to stick
to kernel rules than invent our own.
Also, I'm tempted to say:
char *name, *p;
name = p = strdup(prog->section_name);
while ((p = strchr(p, '/')))
*p = '_';
;)
> + if (*p == '/')
> + *p = '_';
> +
> + return name;
> +}
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 3:38 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:38 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:10 -0800, Stanislav Fomichev wrote:
> @@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
> prog->idx = -1;
> }
>
> +static char *__bpf_program__pin_name(struct bpf_program *prog)
> +{
> + char *name;
> +
> + name = strdup(prog->section_name);
> + for (char *p = name; p && *p; p++)
Useful patch! I'm not sure about libbpf but in the kernel we don't do
C99 variable declarations inside for loop init. Perhaps better to stick
to kernel rules than invent our own.
Also, I'm tempted to say:
char *name, *p;
name = p = strdup(prog->section_name);
while ((p = strchr(p, '/')))
*p = '_';
;)
> + if (*p == '/')
> + *p = '_';
> +
> + return name;
> +}
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 3:38 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 3:38 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:10 -0800, Stanislav Fomichev wrote:
> @@ -261,6 +266,18 @@ static void bpf_program__exit(struct bpf_program *prog)
> prog->idx = -1;
> }
>
> +static char *__bpf_program__pin_name(struct bpf_program *prog)
> +{
> + char *name;
> +
> + name = strdup(prog->section_name);
> + for (char *p = name; p && *p; p++)
Useful patch! I'm not sure about libbpf but in the kernel we don't do
C99 variable declarations inside for loop init. Perhaps better to stick
to kernel rules than invent our own.
Also, I'm tempted to say:
char *name, *p;
name = p = strdup(prog->section_name);
while ((p = strchr(p, '/')))
*p = '_';
;)
> + if (*p == '/')
> + *p = '_';
> +
> + return name;
> +}
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
This patch adds new *loadall* command which slightly differs from the
existing *load*. *load* command loads all programs from the obj file,
but pins only the first programs. *loadall* pins all programs from the
obj file under specified directory.
The intended usecase is flow_dissector, where we want to load a bunch
of progs, pin them all and after that construct a jump table.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 14 +++-
tools/bpf/bpftool/bash-completion/bpftool | 4 +-
tools/bpf/bpftool/common.c | 31 ++++----
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
5 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..d943d9b67a1d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,8 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
+ { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
+ | **loadall** | **help** }
MAP COMMANDS
=============
@@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog help**
@@ -79,8 +80,13 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **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*.
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *FILE*.
+ Both **bpftool prog load** and **bpftool prog loadall** load
+ all maps and programs from the *OBJ* and differ only in
+ pinning. **load** pins only the first program from the *OBJ*
+ as *FILE*. **loadall** pins all programs from the *OBJ*
+ under *FILE* directory.
**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
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 3f78e6404589..780ebafb756a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -243,7 +243,7 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
- if [[ $command != "load" ]]; then
+ if [[ $command != "load" && $command != "loadall" ]]; then
case $prev in
id)
_bpftool_get_prog_ids
@@ -309,7 +309,7 @@ _bpftool()
fi
return 0
;;
- load)
+ load|loadall)
local obj
if [[ ${#words[@]} -lt 6 ]]; then
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 25af85304ebe..21ce556c15e1 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -169,34 +169,23 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_fd(int fd, const char *name)
+int mount_bpffs_for_pin(const char *name)
{
char err_str[ERR_MAX_LEN];
char *file;
char *dir;
int err = 0;
- err = bpf_obj_pin(fd, name);
- if (!err)
- goto out;
-
file = malloc(strlen(name) + 1);
strcpy(file, name);
dir = dirname(file);
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", name, strerror(errno));
+ if (is_bpffs(dir))
+ /* nothing to do if already mounted */
goto out_free;
- }
- /* Attempt to mount bpffs, then retry pinning. */
err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, name);
- if (err)
- p_err("can't pin the object (%s): %s", name,
- strerror(errno));
- } else {
+ if (err) {
err_str[ERR_MAX_LEN - 1] = '\0';
p_err("can't mount BPF file system to pin the object (%s): %s",
name, err_str);
@@ -204,10 +193,20 @@ int do_pin_fd(int fd, const char *name)
out_free:
free(file);
-out:
return err;
}
+int do_pin_fd(int fd, const char *name)
+{
+ int err;
+
+ err = mount_bpffs_for_pin(name);
+ if (err)
+ return err;
+
+ return bpf_obj_pin(fd, name);
+}
+
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
{
unsigned int id;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 28322ace2856..1383824c9baf 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -129,6 +129,7 @@ const char *get_fd_type_name(enum bpf_obj_type type);
char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
+int mount_bpffs_for_pin(const char *name);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
int do_pin_fd(int fd, const char *name);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..751a90ccfdab 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -792,15 +792,16 @@ static int do_detach(int argc, char **argv)
jsonw_null(json_wtr);
return 0;
}
-static int do_load(int argc, char **argv)
+
+static int load_with_options(int argc, char **argv, bool first_prog_only)
{
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;
+ struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
- struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -918,26 +919,25 @@ static int do_load(int argc, char **argv)
goto err_free_reuse_maps;
}
- prog = bpf_program__next(NULL, obj);
- if (!prog) {
- p_err("object file doesn't contain any bpf program");
- goto err_close_obj;
- }
+ bpf_object__for_each_program(pos, obj) {
+ enum bpf_prog_type prog_type = attr.prog_type;
- bpf_program__set_ifindex(prog, ifindex);
- if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
- const char *sec_name = bpf_program__title(prog, false);
+ if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
+ const char *sec_name = bpf_program__title(pos, 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;
+ err = libbpf_prog_type_by_name(sec_name, &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_ifindex(pos, ifindex);
+ bpf_program__set_type(pos, prog_type);
+ bpf_program__set_expected_attach_type(pos, expected_attach_type);
}
- bpf_program__set_type(prog, attr.prog_type);
- bpf_program__set_expected_attach_type(prog, expected_attach_type);
qsort(map_replace, old_map_fds, sizeof(*map_replace),
map_replace_compar);
@@ -1001,9 +1001,31 @@ static int do_load(int argc, char **argv)
goto err_close_obj;
}
- if (do_pin_fd(bpf_program__fd(prog), pinfile))
+ err = mount_bpffs_for_pin(pinfile);
+ if (err)
goto err_close_obj;
+ if (first_prog_only) {
+ prog = bpf_program__next(NULL, obj);
+ if (!prog) {
+ p_err("object file doesn't contain any bpf program");
+ goto err_close_obj;
+ }
+
+ err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
+ if (err) {
+ p_err("failed to pin program %s",
+ bpf_program__title(prog, false));
+ goto err_close_obj;
+ }
+ } else {
+ err = bpf_object__pin_programs(obj, pinfile);
+ if (err) {
+ p_err("failed to pin all programs");
+ goto err_close_obj;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1023,6 +1045,16 @@ static int do_load(int argc, char **argv)
return -1;
}
+static int do_load(int argc, char **argv)
+{
+ return load_with_options(argc, argv, true);
+}
+
+static int do_loadall(int argc, char **argv)
+{
+ return load_with_options(argc, argv, false);
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
+ " [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
@@ -1067,6 +1100,7 @@ static const struct cmd cmds[] = {
{ "dump", do_dump },
{ "pin", do_pin },
{ "load", do_load },
+ { "loadall", do_loadall },
{ "attach", do_attach },
{ "detach", do_detach },
{ 0 }
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
This patch adds new *loadall* command which slightly differs from the
existing *load*. *load* command loads all programs from the obj file,
but pins only the first programs. *loadall* pins all programs from the
obj file under specified directory.
The intended usecase is flow_dissector, where we want to load a bunch
of progs, pin them all and after that construct a jump table.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 14 +++-
tools/bpf/bpftool/bash-completion/bpftool | 4 +-
tools/bpf/bpftool/common.c | 31 ++++----
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
5 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..d943d9b67a1d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,8 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
+ { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
+ | **loadall** | **help** }
MAP COMMANDS
=============
@@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog help**
@@ -79,8 +80,13 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **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*.
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *FILE*.
+ Both **bpftool prog load** and **bpftool prog loadall** load
+ all maps and programs from the *OBJ* and differ only in
+ pinning. **load** pins only the first program from the *OBJ*
+ as *FILE*. **loadall** pins all programs from the *OBJ*
+ under *FILE* directory.
**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
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 3f78e6404589..780ebafb756a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -243,7 +243,7 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
- if [[ $command != "load" ]]; then
+ if [[ $command != "load" && $command != "loadall" ]]; then
case $prev in
id)
_bpftool_get_prog_ids
@@ -309,7 +309,7 @@ _bpftool()
fi
return 0
;;
- load)
+ load|loadall)
local obj
if [[ ${#words[@]} -lt 6 ]]; then
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 25af85304ebe..21ce556c15e1 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -169,34 +169,23 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_fd(int fd, const char *name)
+int mount_bpffs_for_pin(const char *name)
{
char err_str[ERR_MAX_LEN];
char *file;
char *dir;
int err = 0;
- err = bpf_obj_pin(fd, name);
- if (!err)
- goto out;
-
file = malloc(strlen(name) + 1);
strcpy(file, name);
dir = dirname(file);
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", name, strerror(errno));
+ if (is_bpffs(dir))
+ /* nothing to do if already mounted */
goto out_free;
- }
- /* Attempt to mount bpffs, then retry pinning. */
err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, name);
- if (err)
- p_err("can't pin the object (%s): %s", name,
- strerror(errno));
- } else {
+ if (err) {
err_str[ERR_MAX_LEN - 1] = '\0';
p_err("can't mount BPF file system to pin the object (%s): %s",
name, err_str);
@@ -204,10 +193,20 @@ int do_pin_fd(int fd, const char *name)
out_free:
free(file);
-out:
return err;
}
+int do_pin_fd(int fd, const char *name)
+{
+ int err;
+
+ err = mount_bpffs_for_pin(name);
+ if (err)
+ return err;
+
+ return bpf_obj_pin(fd, name);
+}
+
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
{
unsigned int id;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 28322ace2856..1383824c9baf 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -129,6 +129,7 @@ const char *get_fd_type_name(enum bpf_obj_type type);
char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
+int mount_bpffs_for_pin(const char *name);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
int do_pin_fd(int fd, const char *name);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..751a90ccfdab 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -792,15 +792,16 @@ static int do_detach(int argc, char **argv)
jsonw_null(json_wtr);
return 0;
}
-static int do_load(int argc, char **argv)
+
+static int load_with_options(int argc, char **argv, bool first_prog_only)
{
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;
+ struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
- struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -918,26 +919,25 @@ static int do_load(int argc, char **argv)
goto err_free_reuse_maps;
}
- prog = bpf_program__next(NULL, obj);
- if (!prog) {
- p_err("object file doesn't contain any bpf program");
- goto err_close_obj;
- }
+ bpf_object__for_each_program(pos, obj) {
+ enum bpf_prog_type prog_type = attr.prog_type;
- bpf_program__set_ifindex(prog, ifindex);
- if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
- const char *sec_name = bpf_program__title(prog, false);
+ if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
+ const char *sec_name = bpf_program__title(pos, 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;
+ err = libbpf_prog_type_by_name(sec_name, &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_ifindex(pos, ifindex);
+ bpf_program__set_type(pos, prog_type);
+ bpf_program__set_expected_attach_type(pos, expected_attach_type);
}
- bpf_program__set_type(prog, attr.prog_type);
- bpf_program__set_expected_attach_type(prog, expected_attach_type);
qsort(map_replace, old_map_fds, sizeof(*map_replace),
map_replace_compar);
@@ -1001,9 +1001,31 @@ static int do_load(int argc, char **argv)
goto err_close_obj;
}
- if (do_pin_fd(bpf_program__fd(prog), pinfile))
+ err = mount_bpffs_for_pin(pinfile);
+ if (err)
goto err_close_obj;
+ if (first_prog_only) {
+ prog = bpf_program__next(NULL, obj);
+ if (!prog) {
+ p_err("object file doesn't contain any bpf program");
+ goto err_close_obj;
+ }
+
+ err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
+ if (err) {
+ p_err("failed to pin program %s",
+ bpf_program__title(prog, false));
+ goto err_close_obj;
+ }
+ } else {
+ err = bpf_object__pin_programs(obj, pinfile);
+ if (err) {
+ p_err("failed to pin all programs");
+ goto err_close_obj;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1023,6 +1045,16 @@ static int do_load(int argc, char **argv)
return -1;
}
+static int do_load(int argc, char **argv)
+{
+ return load_with_options(argc, argv, true);
+}
+
+static int do_loadall(int argc, char **argv)
+{
+ return load_with_options(argc, argv, false);
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
+ " [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
@@ -1067,6 +1100,7 @@ static const struct cmd cmds[] = {
{ "dump", do_dump },
{ "pin", do_pin },
{ "load", do_load },
+ { "loadall", do_loadall },
{ "attach", do_attach },
{ "detach", do_detach },
{ 0 }
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
This patch adds new *loadall* command which slightly differs from the
existing *load*. *load* command loads all programs from the obj file,
but pins only the first programs. *loadall* pins all programs from the
obj file under specified directory.
The intended usecase is flow_dissector, where we want to load a bunch
of progs, pin them all and after that construct a jump table.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 14 +++-
tools/bpf/bpftool/bash-completion/bpftool | 4 +-
tools/bpf/bpftool/common.c | 31 ++++----
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
5 files changed, 82 insertions(+), 42 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..d943d9b67a1d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,8 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
+ { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load**
+ | **loadall** | **help** }
MAP COMMANDS
=============
@@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
| **bpftool** **prog help**
@@ -79,8 +80,13 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **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*.
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *FILE*.
+ Both **bpftool prog load** and **bpftool prog loadall** load
+ all maps and programs from the *OBJ* and differ only in
+ pinning. **load** pins only the first program from the *OBJ*
+ as *FILE*. **loadall** pins all programs from the *OBJ*
+ under *FILE* directory.
**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
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 3f78e6404589..780ebafb756a 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -243,7 +243,7 @@ _bpftool()
# Completion depends on object and command in use
case $object in
prog)
- if [[ $command != "load" ]]; then
+ if [[ $command != "load" && $command != "loadall" ]]; then
case $prev in
id)
_bpftool_get_prog_ids
@@ -309,7 +309,7 @@ _bpftool()
fi
return 0
;;
- load)
+ load|loadall)
local obj
if [[ ${#words[@]} -lt 6 ]]; then
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 25af85304ebe..21ce556c15e1 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -169,34 +169,23 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_fd(int fd, const char *name)
+int mount_bpffs_for_pin(const char *name)
{
char err_str[ERR_MAX_LEN];
char *file;
char *dir;
int err = 0;
- err = bpf_obj_pin(fd, name);
- if (!err)
- goto out;
-
file = malloc(strlen(name) + 1);
strcpy(file, name);
dir = dirname(file);
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", name, strerror(errno));
+ if (is_bpffs(dir))
+ /* nothing to do if already mounted */
goto out_free;
- }
- /* Attempt to mount bpffs, then retry pinning. */
err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, name);
- if (err)
- p_err("can't pin the object (%s): %s", name,
- strerror(errno));
- } else {
+ if (err) {
err_str[ERR_MAX_LEN - 1] = '\0';
p_err("can't mount BPF file system to pin the object (%s): %s",
name, err_str);
@@ -204,10 +193,20 @@ int do_pin_fd(int fd, const char *name)
out_free:
free(file);
-out:
return err;
}
+int do_pin_fd(int fd, const char *name)
+{
+ int err;
+
+ err = mount_bpffs_for_pin(name);
+ if (err)
+ return err;
+
+ return bpf_obj_pin(fd, name);
+}
+
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
{
unsigned int id;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 28322ace2856..1383824c9baf 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -129,6 +129,7 @@ const char *get_fd_type_name(enum bpf_obj_type type);
char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
+int mount_bpffs_for_pin(const char *name);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
int do_pin_fd(int fd, const char *name);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5302ee282409..751a90ccfdab 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -792,15 +792,16 @@ static int do_detach(int argc, char **argv)
jsonw_null(json_wtr);
return 0;
}
-static int do_load(int argc, char **argv)
+
+static int load_with_options(int argc, char **argv, bool first_prog_only)
{
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;
+ struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
- struct bpf_program *prog;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -918,26 +919,25 @@ static int do_load(int argc, char **argv)
goto err_free_reuse_maps;
}
- prog = bpf_program__next(NULL, obj);
- if (!prog) {
- p_err("object file doesn't contain any bpf program");
- goto err_close_obj;
- }
+ bpf_object__for_each_program(pos, obj) {
+ enum bpf_prog_type prog_type = attr.prog_type;
- bpf_program__set_ifindex(prog, ifindex);
- if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
- const char *sec_name = bpf_program__title(prog, false);
+ if (attr.prog_type == BPF_PROG_TYPE_UNSPEC) {
+ const char *sec_name = bpf_program__title(pos, 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;
+ err = libbpf_prog_type_by_name(sec_name, &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_ifindex(pos, ifindex);
+ bpf_program__set_type(pos, prog_type);
+ bpf_program__set_expected_attach_type(pos, expected_attach_type);
}
- bpf_program__set_type(prog, attr.prog_type);
- bpf_program__set_expected_attach_type(prog, expected_attach_type);
qsort(map_replace, old_map_fds, sizeof(*map_replace),
map_replace_compar);
@@ -1001,9 +1001,31 @@ static int do_load(int argc, char **argv)
goto err_close_obj;
}
- if (do_pin_fd(bpf_program__fd(prog), pinfile))
+ err = mount_bpffs_for_pin(pinfile);
+ if (err)
goto err_close_obj;
+ if (first_prog_only) {
+ prog = bpf_program__next(NULL, obj);
+ if (!prog) {
+ p_err("object file doesn't contain any bpf program");
+ goto err_close_obj;
+ }
+
+ err = bpf_obj_pin(bpf_program__fd(prog), pinfile);
+ if (err) {
+ p_err("failed to pin program %s",
+ bpf_program__title(prog, false));
+ goto err_close_obj;
+ }
+ } else {
+ err = bpf_object__pin_programs(obj, pinfile);
+ if (err) {
+ p_err("failed to pin all programs");
+ goto err_close_obj;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1023,6 +1045,16 @@ static int do_load(int argc, char **argv)
return -1;
}
+static int do_load(int argc, char **argv)
+{
+ return load_with_options(argc, argv, true);
+}
+
+static int do_loadall(int argc, char **argv)
+{
+ return load_with_options(argc, argv, false);
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
+ " [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
@@ -1067,6 +1100,7 @@ static const struct cmd cmds[] = {
{ "dump", do_dump },
{ "pin", do_pin },
{ "load", do_load },
+ { "loadall", do_loadall },
{ "attach", do_attach },
{ "detach", do_detach },
{ 0 }
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 3:41 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:41 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Thu, 8 Nov 2018 16:22:11 -0800, Stanislav Fomichev wrote:
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
As I said the fact that we load all always is a libbpf limitation,
I wouldn't put it in documentation as it may change.
With that removed looks good to me:
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 3:41 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:41 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:11 -0800, Stanislav Fomichev wrote:
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
As I said the fact that we load all always is a libbpf limitation,
I wouldn't put it in documentation as it may change.
With that removed looks good to me:
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 3:41 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 3:41 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:11 -0800, Stanislav Fomichev wrote:
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
As I said the fact that we load all always is a libbpf limitation,
I wouldn't put it in documentation as it may change.
With that removed looks good to me:
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 13:29 ` quentin.monnet
-1 siblings, 0 replies; 48+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:29 UTC (permalink / raw)
To: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> From: Stanislav Fomichev <sdf@google.com>
>
> This patch adds new *loadall* command which slightly differs from the
> existing *load*. *load* command loads all programs from the obj file,
> but pins only the first programs. *loadall* pins all programs from the
> obj file under specified directory.
>
> The intended usecase is flow_dissector, where we want to load a bunch
> of progs, pin them all and after that construct a jump table.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> tools/bpf/bpftool/common.c | 31 ++++----
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> 5 files changed, 82 insertions(+), 42 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ac4e904b10fb..d943d9b67a1d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog help**
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
Thanks a lot for all the changes! The series looks really good to me
now. The last nit I might have is that we could maybe replace "FILE"
with "PATH" (as it can now be a directory), in the doc an below. No need
to respin just for this, though.
> @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> + " [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> " %s %s attach PROG ATTACH_TYPE MAP\n"
> " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 13:29 ` quentin.monnet
0 siblings, 0 replies; 48+ messages in thread
From: Quentin Monnet @ 2018-11-09 13:29 UTC (permalink / raw)
2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf at fomichev.me>
> From: Stanislav Fomichev <sdf at google.com>
>
> This patch adds new *loadall* command which slightly differs from the
> existing *load*. *load* command loads all programs from the obj file,
> but pins only the first programs. *loadall* pins all programs from the
> obj file under specified directory.
>
> The intended usecase is flow_dissector, where we want to load a bunch
> of progs, pin them all and after that construct a jump table.
>
> Signed-off-by: Stanislav Fomichev <sdf at google.com>
> ---
> .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> tools/bpf/bpftool/common.c | 31 ++++----
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> 5 files changed, 82 insertions(+), 42 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ac4e904b10fb..d943d9b67a1d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog help**
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
Thanks a lot for all the changes! The series looks really good to me
now. The last nit I might have is that we could maybe replace "FILE"
with "PATH" (as it can now be a directory), in the doc an below. No need
to respin just for this, though.
> @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> + " [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> " %s %s attach PROG ATTACH_TYPE MAP\n"
> " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 13:29 ` quentin.monnet
0 siblings, 0 replies; 48+ messages in thread
From: quentin.monnet @ 2018-11-09 13:29 UTC (permalink / raw)
2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf at fomichev.me>
> From: Stanislav Fomichev <sdf at google.com>
>
> This patch adds new *loadall* command which slightly differs from the
> existing *load*. *load* command loads all programs from the obj file,
> but pins only the first programs. *loadall* pins all programs from the
> obj file under specified directory.
>
> The intended usecase is flow_dissector, where we want to load a bunch
> of progs, pin them all and after that construct a jump table.
>
> Signed-off-by: Stanislav Fomichev <sdf at google.com>
> ---
> .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> tools/bpf/bpftool/common.c | 31 ++++----
> tools/bpf/bpftool/main.h | 1 +
> tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> 5 files changed, 82 insertions(+), 42 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index ac4e904b10fb..d943d9b67a1d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> | **bpftool** **prog help**
> @@ -79,8 +80,13 @@ DESCRIPTION
> contain a dot character ('.'), which is reserved for future
> extensions of *bpffs*.
>
> - **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*.
> + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> + Both **bpftool prog load** and **bpftool prog loadall** load
> + all maps and programs from the *OBJ* and differ only in
> + pinning. **load** pins only the first program from the *OBJ*
> + as *FILE*. **loadall** pins all programs from the *OBJ*
> + under *FILE* directory.
> **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
Thanks a lot for all the changes! The series looks really good to me
now. The last nit I might have is that we could maybe replace "FILE"
with "PATH" (as it can now be a directory), in the doc an below. No need
to respin just for this, though.
> @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> + " [type TYPE] [dev NAME] \\\n"
> " [map { idx IDX | name NAME } MAP]\n"
> " %s %s attach PROG ATTACH_TYPE MAP\n"
> " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 5/7] bpftool: add loadall command
2018-11-09 13:29 ` quentin.monnet
(?)
@ 2018-11-09 15:42 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 15:42 UTC (permalink / raw)
To: Quentin Monnet
Cc: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On 11/09, Quentin Monnet wrote:
> 2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> > From: Stanislav Fomichev <sdf@google.com>
> >
> > This patch adds new *loadall* command which slightly differs from the
> > existing *load*. *load* command loads all programs from the obj file,
> > but pins only the first programs. *loadall* pins all programs from the
> > obj file under specified directory.
> >
> > The intended usecase is flow_dissector, where we want to load a bunch
> > of progs, pin them all and after that construct a jump table.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> > tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> > tools/bpf/bpftool/common.c | 31 ++++----
> > tools/bpf/bpftool/main.h | 1 +
> > tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> > 5 files changed, 82 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index ac4e904b10fb..d943d9b67a1d 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> > @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog help**
> > @@ -79,8 +80,13 @@ DESCRIPTION
> > contain a dot character ('.'), which is reserved for future
> > extensions of *bpffs*.
> >
> > - **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*.
> > + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> > + Both **bpftool prog load** and **bpftool prog loadall** load
> > + all maps and programs from the *OBJ* and differ only in
> > + pinning. **load** pins only the first program from the *OBJ*
> > + as *FILE*. **loadall** pins all programs from the *OBJ*
> > + under *FILE* directory.
> > **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
>
> Thanks a lot for all the changes! The series looks really good to me
> now. The last nit I might have is that we could maybe replace "FILE"
> with "PATH" (as it can now be a directory), in the doc an below. No need
> to respin just for this, though.
Agreed, makes sense, will do another respin to address Jakub's comments
anyway. Thanks for a review!
> > @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> > + " [type TYPE] [dev NAME] \\\n"
> > " [map { idx IDX | name NAME } MAP]\n"
> > " %s %s attach PROG ATTACH_TYPE MAP\n"
> > " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 15:42 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 15:42 UTC (permalink / raw)
On 11/09, Quentin Monnet wrote:
> 2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf at fomichev.me>
> > From: Stanislav Fomichev <sdf at google.com>
> >
> > This patch adds new *loadall* command which slightly differs from the
> > existing *load*. *load* command loads all programs from the obj file,
> > but pins only the first programs. *loadall* pins all programs from the
> > obj file under specified directory.
> >
> > The intended usecase is flow_dissector, where we want to load a bunch
> > of progs, pin them all and after that construct a jump table.
> >
> > Signed-off-by: Stanislav Fomichev <sdf at google.com>
> > ---
> > .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> > tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> > tools/bpf/bpftool/common.c | 31 ++++----
> > tools/bpf/bpftool/main.h | 1 +
> > tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> > 5 files changed, 82 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index ac4e904b10fb..d943d9b67a1d 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> > @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog help**
> > @@ -79,8 +80,13 @@ DESCRIPTION
> > contain a dot character ('.'), which is reserved for future
> > extensions of *bpffs*.
> >
> > - **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*.
> > + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> > + Both **bpftool prog load** and **bpftool prog loadall** load
> > + all maps and programs from the *OBJ* and differ only in
> > + pinning. **load** pins only the first program from the *OBJ*
> > + as *FILE*. **loadall** pins all programs from the *OBJ*
> > + under *FILE* directory.
> > **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
>
> Thanks a lot for all the changes! The series looks really good to me
> now. The last nit I might have is that we could maybe replace "FILE"
> with "PATH" (as it can now be a directory), in the doc an below. No need
> to respin just for this, though.
Agreed, makes sense, will do another respin to address Jakub's comments
anyway. Thanks for a review!
> > @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> > + " [type TYPE] [dev NAME] \\\n"
> > " [map { idx IDX | name NAME } MAP]\n"
> > " %s %s attach PROG ATTACH_TYPE MAP\n"
> > " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 15:42 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 15:42 UTC (permalink / raw)
On 11/09, Quentin Monnet wrote:
> 2018-11-08 16:22 UTC-0800 ~ Stanislav Fomichev <sdf at fomichev.me>
> > From: Stanislav Fomichev <sdf at google.com>
> >
> > This patch adds new *loadall* command which slightly differs from the
> > existing *load*. *load* command loads all programs from the obj file,
> > but pins only the first programs. *loadall* pins all programs from the
> > obj file under specified directory.
> >
> > The intended usecase is flow_dissector, where we want to load a bunch
> > of progs, pin them all and after that construct a jump table.
> >
> > Signed-off-by: Stanislav Fomichev <sdf at google.com>
> > ---
> > .../bpftool/Documentation/bpftool-prog.rst | 14 +++-
> > tools/bpf/bpftool/bash-completion/bpftool | 4 +-
> > tools/bpf/bpftool/common.c | 31 ++++----
> > tools/bpf/bpftool/main.h | 1 +
> > tools/bpf/bpftool/prog.c | 74 ++++++++++++++-----
> > 5 files changed, 82 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index ac4e904b10fb..d943d9b67a1d 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> > @@ -24,7 +25,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* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > +| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > | **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
> > | **bpftool** **prog help**
> > @@ -79,8 +80,13 @@ DESCRIPTION
> > contain a dot character ('.'), which is reserved for future
> > extensions of *bpffs*.
> >
> > - **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*.
> > + **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
> > + Load bpf program(s) from binary *OBJ* and pin as *FILE*.
> > + Both **bpftool prog load** and **bpftool prog loadall** load
> > + all maps and programs from the *OBJ* and differ only in
> > + pinning. **load** pins only the first program from the *OBJ*
> > + as *FILE*. **loadall** pins all programs from the *OBJ*
> > + under *FILE* directory.
> > **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
>
> Thanks a lot for all the changes! The series looks really good to me
> now. The last nit I might have is that we could maybe replace "FILE"
> with "PATH" (as it can now be a directory), in the doc an below. No need
> to respin just for this, though.
Agreed, makes sense, will do another respin to address Jakub's comments
anyway. Thanks for a review!
> > @@ -1035,7 +1067,8 @@ 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 | loadall } OBJ FILE \\\n"
> > + " [type TYPE] [dev NAME] \\\n"
> > " [map { idx IDX | name NAME } MAP]\n"
> > " %s %s attach PROG ATTACH_TYPE MAP\n"
> > " %s %s detach PROG ATTACH_TYPE MAP\n"
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
This new additional argument lets users pin all maps from the object at
specified path.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
tools/bpf/bpftool/prog.c | 24 ++++++++++++++++++-
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index d943d9b67a1d..b04c4a365739 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -80,7 +80,7 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *FILE*.
Both **bpftool prog load** and **bpftool prog loadall** load
all maps and programs from the *OBJ* and differ only in
@@ -98,6 +98,8 @@ DESCRIPTION
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).
+ Optional **pinmaps** argument can be provided to pin all
+ maps under *MAP_DIR* directory.
Note: *FILE* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 780ebafb756a..a05d0071f39f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -346,7 +346,7 @@ _bpftool()
_bpftool_get_map_ids
return 0
;;
- pinned)
+ pinned|pinmaps)
_filedir
return 0
;;
@@ -358,6 +358,7 @@ _bpftool()
COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
_bpftool_once_attr 'type'
_bpftool_once_attr 'dev'
+ _bpftool_once_attr 'pinmaps'
return 0
;;
esac
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 751a90ccfdab..4654d9450cd9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -802,6 +802,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
struct map_replace *map_replace = NULL;
struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
+ const char *pinmaps = NULL;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -906,6 +907,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
goto err_free_reuse_maps;
}
NEXT_ARG();
+ } else if (is_prefix(*argv, "pinmaps")) {
+ NEXT_ARG();
+
+ if (!REQ_ARGS(1))
+ goto err_free_reuse_maps;
+
+ pinmaps = GET_ARG();
} else {
p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
*argv);
@@ -1026,6 +1034,14 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
}
}
+ if (pinmaps) {
+ err = bpf_object__pin_maps(obj, pinmaps);
+ if (err) {
+ p_err("failed to pin all maps");
+ goto err_unpin;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1036,6 +1052,11 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
return 0;
+err_unpin:
+ if (first_prog_only)
+ unlink(pinfile);
+ else
+ bpf_object__unpin_programs(obj, pinfile);
err_close_obj:
bpf_object__close(obj);
err_free_reuse_maps:
@@ -1069,7 +1090,8 @@ static int do_help(int argc, char **argv)
" %s %s pin PROG FILE\n"
" %s %s { load | loadall } OBJ FILE \\\n"
" [type TYPE] [dev NAME] \\\n"
- " [map { idx IDX | name NAME } MAP]\n"
+ " [map { idx IDX | name NAME } MAP]\\\n"
+ " [pinmaps MAP_DIR]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
" %s %s help\n"
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
This new additional argument lets users pin all maps from the object at
specified path.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
tools/bpf/bpftool/prog.c | 24 ++++++++++++++++++-
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index d943d9b67a1d..b04c4a365739 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -80,7 +80,7 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *FILE*.
Both **bpftool prog load** and **bpftool prog loadall** load
all maps and programs from the *OBJ* and differ only in
@@ -98,6 +98,8 @@ DESCRIPTION
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).
+ Optional **pinmaps** argument can be provided to pin all
+ maps under *MAP_DIR* directory.
Note: *FILE* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 780ebafb756a..a05d0071f39f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -346,7 +346,7 @@ _bpftool()
_bpftool_get_map_ids
return 0
;;
- pinned)
+ pinned|pinmaps)
_filedir
return 0
;;
@@ -358,6 +358,7 @@ _bpftool()
COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
_bpftool_once_attr 'type'
_bpftool_once_attr 'dev'
+ _bpftool_once_attr 'pinmaps'
return 0
;;
esac
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 751a90ccfdab..4654d9450cd9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -802,6 +802,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
struct map_replace *map_replace = NULL;
struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
+ const char *pinmaps = NULL;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -906,6 +907,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
goto err_free_reuse_maps;
}
NEXT_ARG();
+ } else if (is_prefix(*argv, "pinmaps")) {
+ NEXT_ARG();
+
+ if (!REQ_ARGS(1))
+ goto err_free_reuse_maps;
+
+ pinmaps = GET_ARG();
} else {
p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
*argv);
@@ -1026,6 +1034,14 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
}
}
+ if (pinmaps) {
+ err = bpf_object__pin_maps(obj, pinmaps);
+ if (err) {
+ p_err("failed to pin all maps");
+ goto err_unpin;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1036,6 +1052,11 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
return 0;
+err_unpin:
+ if (first_prog_only)
+ unlink(pinfile);
+ else
+ bpf_object__unpin_programs(obj, pinfile);
err_close_obj:
bpf_object__close(obj);
err_free_reuse_maps:
@@ -1069,7 +1090,8 @@ static int do_help(int argc, char **argv)
" %s %s pin PROG FILE\n"
" %s %s { load | loadall } OBJ FILE \\\n"
" [type TYPE] [dev NAME] \\\n"
- " [map { idx IDX | name NAME } MAP]\n"
+ " [map { idx IDX | name NAME } MAP]\\\n"
+ " [pinmaps MAP_DIR]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
" %s %s help\n"
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
This new additional argument lets users pin all maps from the object at
specified path.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/bash-completion/bpftool | 3 ++-
tools/bpf/bpftool/prog.c | 24 ++++++++++++++++++-
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index d943d9b67a1d..b04c4a365739 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -80,7 +80,7 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *FILE*.
Both **bpftool prog load** and **bpftool prog loadall** load
all maps and programs from the *OBJ* and differ only in
@@ -98,6 +98,8 @@ DESCRIPTION
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).
+ Optional **pinmaps** argument can be provided to pin all
+ maps under *MAP_DIR* directory.
Note: *FILE* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 780ebafb756a..a05d0071f39f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -346,7 +346,7 @@ _bpftool()
_bpftool_get_map_ids
return 0
;;
- pinned)
+ pinned|pinmaps)
_filedir
return 0
;;
@@ -358,6 +358,7 @@ _bpftool()
COMPREPLY=( $( compgen -W "map" -- "$cur" ) )
_bpftool_once_attr 'type'
_bpftool_once_attr 'dev'
+ _bpftool_once_attr 'pinmaps'
return 0
;;
esac
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 751a90ccfdab..4654d9450cd9 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -802,6 +802,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
struct map_replace *map_replace = NULL;
struct bpf_program *prog = NULL, *pos;
unsigned int old_map_fds = 0;
+ const char *pinmaps = NULL;
struct bpf_object *obj;
struct bpf_map *map;
const char *pinfile;
@@ -906,6 +907,13 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
goto err_free_reuse_maps;
}
NEXT_ARG();
+ } else if (is_prefix(*argv, "pinmaps")) {
+ NEXT_ARG();
+
+ if (!REQ_ARGS(1))
+ goto err_free_reuse_maps;
+
+ pinmaps = GET_ARG();
} else {
p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
*argv);
@@ -1026,6 +1034,14 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
}
}
+ if (pinmaps) {
+ err = bpf_object__pin_maps(obj, pinmaps);
+ if (err) {
+ p_err("failed to pin all maps");
+ goto err_unpin;
+ }
+ }
+
if (json_output)
jsonw_null(json_wtr);
@@ -1036,6 +1052,11 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
return 0;
+err_unpin:
+ if (first_prog_only)
+ unlink(pinfile);
+ else
+ bpf_object__unpin_programs(obj, pinfile);
err_close_obj:
bpf_object__close(obj);
err_free_reuse_maps:
@@ -1069,7 +1090,8 @@ static int do_help(int argc, char **argv)
" %s %s pin PROG FILE\n"
" %s %s { load | loadall } OBJ FILE \\\n"
" [type TYPE] [dev NAME] \\\n"
- " [map { idx IDX | name NAME } MAP]\n"
+ " [map { idx IDX | name NAME } MAP]\\\n"
+ " [pinmaps MAP_DIR]\n"
" %s %s attach PROG ATTACH_TYPE MAP\n"
" %s %s detach PROG ATTACH_TYPE MAP\n"
" %s %s help\n"
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 3:43 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:43 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Thu, 8 Nov 2018 16:22:12 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf@google.com>
>
> This new additional argument lets users pin all maps from the object at
> specified path.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 3:43 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:43 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:12 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf at google.com>
>
> This new additional argument lets users pin all maps from the object at
> specified path.
>
> Signed-off-by: Stanislav Fomichev <sdf at google.com>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 3:43 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 3:43 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:12 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf at google.com>
>
> This new additional argument lets users pin all maps from the object at
> specified path.
>
> Signed-off-by: Stanislav Fomichev <sdf at google.com>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 0:22 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
To: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet
Cc: guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
From: Stanislav Fomichev <sdf@google.com>
This commit adds support for loading/attaching/detaching flow
dissector program. The structure of the flow dissector program is
assumed to be the same as in the selftests:
* flow_dissector section with the main entry point
* a bunch of tail call progs
* a jmp_table map that is populated with the tail call progs
When `bpftool loadall` is called with a flow_dissector prog (i.e. when the
'type flow_dissector' argument is passed), we load and pin all programs.
User is responsible to construct the jump table for the tail calls.
The last argument of `bpftool attach` is made optional for this use
case.
Example:
bpftool prog load tools/testing/selftests/bpf/bpf_flow.o \
/sys/fs/bpf/flow type flow_dissector \
pinmaps /sys/fs/bpf/flow
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 0 0 0 0 \
value pinned /sys/fs/bpf/flow/IP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 1 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 2 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6OP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 3 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6FR
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 4 0 0 0 \
value pinned /sys/fs/bpf/flow/MPLS
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 5 0 0 0 \
value pinned /sys/fs/bpf/flow/VLAN
bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector flow_dissector
Tested by using the above lines to load the prog in
the test_flow_dissector.sh selftest.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 87 +++++++++++--------
3 files changed, 77 insertions(+), 50 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index b04c4a365739..d77885176464 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -26,8 +26,8 @@ MAP COMMANDS
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
-| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
-| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
+| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
| **bpftool** **prog help**
|
| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -40,7 +40,9 @@ MAP COMMANDS
| **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
| }
-| *ATTACH_TYPE* := { **msg_verdict** | **skb_verdict** | **skb_parse** }
+| *ATTACH_TYPE* := {
+| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
+| }
DESCRIPTION
@@ -105,13 +107,17 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog attach** *PROG* *ATTACH_TYPE* *MAP*
- Attach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- to the map *MAP*.
-
- **bpftool prog detach** *PROG* *ATTACH_TYPE* *MAP*
- Detach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- from the map *MAP*.
+ **bpftool prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Attach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ attached to current networking name space.
+
+ **bpftool prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Detach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ detached from the current networking name space.
**bpftool prog help**
Print short help message.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a05d0071f39f..45c2db257d2b 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -299,7 +299,8 @@ _bpftool()
fi
if [[ ${#words[@]} == 6 ]]; then
- COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse" -- "$cur" ) )
+ COMPREPLY=( $( compgen -W "msg_verdict skb_verdict \
+ skb_parse flow_dissector" -- "$cur" ) )
return 0
fi
@@ -338,7 +339,16 @@ _bpftool()
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" -- \
+ COMPREPLY=( $( compgen -W "socket kprobe \
+ kretprobe classifier flow_dissector \
+ 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
;;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 4654d9450cd9..b808a67d1d3e 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
[BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
[BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
[BPF_SK_MSG_VERDICT] = "msg_verdict",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
@@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
return a->idx - b->idx;
}
-static int do_attach(int argc, char **argv)
+static int parse_atach_detach_args(int argc, char **argv, int *progfd,
+ enum bpf_attach_type *attach_type,
+ int *mapfd)
{
- enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map attach");
+ if (!REQ_ARGS(3)) {
+ p_err("too few parameters for attach/detach");
return -EINVAL;
}
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ *progfd = prog_parse_fd(&argc, &argv);
+ if (*progfd < 0)
+ return *progfd;
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
+ *attach_type = parse_attach_type(*argv);
+ if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach/detach type");
return -EINVAL;
}
+
+ if (*attach_type == BPF_FLOW_DISSECTOR) {
+ *mapfd = -1;
+ return 0;
+ }
+
NEXT_ARG();
+ if (!REQ_ARGS(2)) {
+ p_err("too few parameters for map attach/detach");
+ return -EINVAL;
+ }
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
+
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
if (err) {
@@ -760,27 +784,13 @@ static int do_attach(int argc, char **argv)
static int do_detach(int argc, char **argv)
{
enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map detach");
- return -EINVAL;
- }
+ int err, progfd;
+ int mapfd;
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
-
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
- return -EINVAL;
- }
- NEXT_ARG();
-
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1102,8 @@ static int do_help(int argc, char **argv)
" [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\\\n"
" [pinmaps MAP_DIR]\n"
- " %s %s attach PROG ATTACH_TYPE MAP\n"
- " %s %s detach PROG ATTACH_TYPE MAP\n"
+ " %s %s attach PROG ATTACH_TYPE [MAP]\n"
+ " %s %s detach PROG ATTACH_TYPE [MAP]\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_MAP "\n"
@@ -1105,7 +1115,8 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse }\n"
+ " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
+ " flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf@google.com>
This commit adds support for loading/attaching/detaching flow
dissector program. The structure of the flow dissector program is
assumed to be the same as in the selftests:
* flow_dissector section with the main entry point
* a bunch of tail call progs
* a jmp_table map that is populated with the tail call progs
When `bpftool loadall` is called with a flow_dissector prog (i.e. when the
'type flow_dissector' argument is passed), we load and pin all programs.
User is responsible to construct the jump table for the tail calls.
The last argument of `bpftool attach` is made optional for this use
case.
Example:
bpftool prog load tools/testing/selftests/bpf/bpf_flow.o \
/sys/fs/bpf/flow type flow_dissector \
pinmaps /sys/fs/bpf/flow
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 0 0 0 0 \
value pinned /sys/fs/bpf/flow/IP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 1 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 2 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6OP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 3 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6FR
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 4 0 0 0 \
value pinned /sys/fs/bpf/flow/MPLS
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 5 0 0 0 \
value pinned /sys/fs/bpf/flow/VLAN
bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector flow_dissector
Tested by using the above lines to load the prog in
the test_flow_dissector.sh selftest.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 87 +++++++++++--------
3 files changed, 77 insertions(+), 50 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index b04c4a365739..d77885176464 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -26,8 +26,8 @@ MAP COMMANDS
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
-| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
-| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
+| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
| **bpftool** **prog help**
|
| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -40,7 +40,9 @@ MAP COMMANDS
| **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
| }
-| *ATTACH_TYPE* := { **msg_verdict** | **skb_verdict** | **skb_parse** }
+| *ATTACH_TYPE* := {
+| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
+| }
DESCRIPTION
@@ -105,13 +107,17 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog attach** *PROG* *ATTACH_TYPE* *MAP*
- Attach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- to the map *MAP*.
-
- **bpftool prog detach** *PROG* *ATTACH_TYPE* *MAP*
- Detach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- from the map *MAP*.
+ **bpftool prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Attach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ attached to current networking name space.
+
+ **bpftool prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Detach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ detached from the current networking name space.
**bpftool prog help**
Print short help message.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a05d0071f39f..45c2db257d2b 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -299,7 +299,8 @@ _bpftool()
fi
if [[ ${#words[@]} == 6 ]]; then
- COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse" -- "$cur" ) )
+ COMPREPLY=( $( compgen -W "msg_verdict skb_verdict \
+ skb_parse flow_dissector" -- "$cur" ) )
return 0
fi
@@ -338,7 +339,16 @@ _bpftool()
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" -- \
+ COMPREPLY=( $( compgen -W "socket kprobe \
+ kretprobe classifier flow_dissector \
+ 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
;;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 4654d9450cd9..b808a67d1d3e 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
[BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
[BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
[BPF_SK_MSG_VERDICT] = "msg_verdict",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
@@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
return a->idx - b->idx;
}
-static int do_attach(int argc, char **argv)
+static int parse_atach_detach_args(int argc, char **argv, int *progfd,
+ enum bpf_attach_type *attach_type,
+ int *mapfd)
{
- enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map attach");
+ if (!REQ_ARGS(3)) {
+ p_err("too few parameters for attach/detach");
return -EINVAL;
}
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ *progfd = prog_parse_fd(&argc, &argv);
+ if (*progfd < 0)
+ return *progfd;
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
+ *attach_type = parse_attach_type(*argv);
+ if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach/detach type");
return -EINVAL;
}
+
+ if (*attach_type == BPF_FLOW_DISSECTOR) {
+ *mapfd = -1;
+ return 0;
+ }
+
NEXT_ARG();
+ if (!REQ_ARGS(2)) {
+ p_err("too few parameters for map attach/detach");
+ return -EINVAL;
+ }
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
+
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
if (err) {
@@ -760,27 +784,13 @@ static int do_attach(int argc, char **argv)
static int do_detach(int argc, char **argv)
{
enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map detach");
- return -EINVAL;
- }
+ int err, progfd;
+ int mapfd;
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
-
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
- return -EINVAL;
- }
- NEXT_ARG();
-
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1102,8 @@ static int do_help(int argc, char **argv)
" [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\\\n"
" [pinmaps MAP_DIR]\n"
- " %s %s attach PROG ATTACH_TYPE MAP\n"
- " %s %s detach PROG ATTACH_TYPE MAP\n"
+ " %s %s attach PROG ATTACH_TYPE [MAP]\n"
+ " %s %s detach PROG ATTACH_TYPE [MAP]\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_MAP "\n"
@@ -1105,7 +1115,8 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse }\n"
+ " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
+ " flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 0:22 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 0:22 UTC (permalink / raw)
From: Stanislav Fomichev <sdf at google.com>
This commit adds support for loading/attaching/detaching flow
dissector program. The structure of the flow dissector program is
assumed to be the same as in the selftests:
* flow_dissector section with the main entry point
* a bunch of tail call progs
* a jmp_table map that is populated with the tail call progs
When `bpftool loadall` is called with a flow_dissector prog (i.e. when the
'type flow_dissector' argument is passed), we load and pin all programs.
User is responsible to construct the jump table for the tail calls.
The last argument of `bpftool attach` is made optional for this use
case.
Example:
bpftool prog load tools/testing/selftests/bpf/bpf_flow.o \
/sys/fs/bpf/flow type flow_dissector \
pinmaps /sys/fs/bpf/flow
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 0 0 0 0 \
value pinned /sys/fs/bpf/flow/IP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 1 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 2 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6OP
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 3 0 0 0 \
value pinned /sys/fs/bpf/flow/IPV6FR
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 4 0 0 0 \
value pinned /sys/fs/bpf/flow/MPLS
bpftool map update pinned /sys/fs/bpf/flow/jmp_table \
key 5 0 0 0 \
value pinned /sys/fs/bpf/flow/VLAN
bpftool prog attach pinned /sys/fs/bpf/flow/flow_dissector flow_dissector
Tested by using the above lines to load the prog in
the test_flow_dissector.sh selftest.
Signed-off-by: Stanislav Fomichev <sdf at google.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 87 +++++++++++--------
3 files changed, 77 insertions(+), 50 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index b04c4a365739..d77885176464 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -26,8 +26,8 @@ MAP COMMANDS
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
| **bpftool** **prog { load | loadall }** *OBJ* *FILE* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
-| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* *MAP*
-| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* *MAP*
+| **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+| **bpftool** **prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
| **bpftool** **prog help**
|
| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
@@ -40,7 +40,9 @@ MAP COMMANDS
| **cgroup/bind4** | **cgroup/bind6** | **cgroup/post_bind4** | **cgroup/post_bind6** |
| **cgroup/connect4** | **cgroup/connect6** | **cgroup/sendmsg4** | **cgroup/sendmsg6**
| }
-| *ATTACH_TYPE* := { **msg_verdict** | **skb_verdict** | **skb_parse** }
+| *ATTACH_TYPE* := {
+| **msg_verdict** | **skb_verdict** | **skb_parse** | **flow_dissector**
+| }
DESCRIPTION
@@ -105,13 +107,17 @@ DESCRIPTION
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
- **bpftool prog attach** *PROG* *ATTACH_TYPE* *MAP*
- Attach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- to the map *MAP*.
-
- **bpftool prog detach** *PROG* *ATTACH_TYPE* *MAP*
- Detach bpf program *PROG* (with type specified by *ATTACH_TYPE*)
- from the map *MAP*.
+ **bpftool prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Attach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ attached to current networking name space.
+
+ **bpftool prog detach** *PROG* *ATTACH_TYPE* [*MAP*]
+ Detach bpf program *PROG* (with type specified by
+ *ATTACH_TYPE*). Most *ATTACH_TYPEs* require a *MAP*
+ parameter, with the exception of *flow_dissector* which is
+ detached from the current networking name space.
**bpftool prog help**
Print short help message.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a05d0071f39f..45c2db257d2b 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -299,7 +299,8 @@ _bpftool()
fi
if [[ ${#words[@]} == 6 ]]; then
- COMPREPLY=( $( compgen -W "msg_verdict skb_verdict skb_parse" -- "$cur" ) )
+ COMPREPLY=( $( compgen -W "msg_verdict skb_verdict \
+ skb_parse flow_dissector" -- "$cur" ) )
return 0
fi
@@ -338,7 +339,16 @@ _bpftool()
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" -- \
+ COMPREPLY=( $( compgen -W "socket kprobe \
+ kretprobe classifier flow_dissector \
+ 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
;;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 4654d9450cd9..b808a67d1d3e 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
[BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
[BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
[BPF_SK_MSG_VERDICT] = "msg_verdict",
+ [BPF_FLOW_DISSECTOR] = "flow_dissector",
[__MAX_BPF_ATTACH_TYPE] = NULL,
};
@@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
return a->idx - b->idx;
}
-static int do_attach(int argc, char **argv)
+static int parse_atach_detach_args(int argc, char **argv, int *progfd,
+ enum bpf_attach_type *attach_type,
+ int *mapfd)
{
- enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map attach");
+ if (!REQ_ARGS(3)) {
+ p_err("too few parameters for attach/detach");
return -EINVAL;
}
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ *progfd = prog_parse_fd(&argc, &argv);
+ if (*progfd < 0)
+ return *progfd;
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
+ *attach_type = parse_attach_type(*argv);
+ if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach/detach type");
return -EINVAL;
}
+
+ if (*attach_type == BPF_FLOW_DISSECTOR) {
+ *mapfd = -1;
+ return 0;
+ }
+
NEXT_ARG();
+ if (!REQ_ARGS(2)) {
+ p_err("too few parameters for map attach/detach");
+ return -EINVAL;
+ }
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
+
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_attach(progfd, mapfd, attach_type, 0);
if (err) {
@@ -760,27 +784,13 @@ static int do_attach(int argc, char **argv)
static int do_detach(int argc, char **argv)
{
enum bpf_attach_type attach_type;
- int err, mapfd, progfd;
-
- if (!REQ_ARGS(5)) {
- p_err("too few parameters for map detach");
- return -EINVAL;
- }
+ int err, progfd;
+ int mapfd;
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
-
- attach_type = parse_attach_type(*argv);
- if (attach_type == __MAX_BPF_ATTACH_TYPE) {
- p_err("invalid attach type");
- return -EINVAL;
- }
- NEXT_ARG();
-
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+ err = parse_atach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1102,8 @@ static int do_help(int argc, char **argv)
" [type TYPE] [dev NAME] \\\n"
" [map { idx IDX | name NAME } MAP]\\\n"
" [pinmaps MAP_DIR]\n"
- " %s %s attach PROG ATTACH_TYPE MAP\n"
- " %s %s detach PROG ATTACH_TYPE MAP\n"
+ " %s %s attach PROG ATTACH_TYPE [MAP]\n"
+ " %s %s detach PROG ATTACH_TYPE [MAP]\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_MAP "\n"
@@ -1105,7 +1115,8 @@ static int do_help(int argc, char **argv)
" cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |\n"
" cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |\n"
" cgroup/sendmsg4 | cgroup/sendmsg6 }\n"
- " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse }\n"
+ " ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |\n"
+ " flow_dissector }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
2018-11-09 0:22 ` sdf
(?)
@ 2018-11-09 3:50 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:50 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, sdf, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Thu, 8 Nov 2018 16:22:13 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf@google.com>
>
> This commit adds support for loading/attaching/detaching flow
> dissector program. The structure of the flow dissector program is
> assumed to be the same as in the selftests:
nit: I don't think we make any assumptions any more? Since the
sub-programs are added to the map explicitly by the user?
> * flow_dissector section with the main entry point
> * a bunch of tail call progs
> * a jmp_table map that is populated with the tail call progs
>
[...]
> @@ -338,7 +339,16 @@ _bpftool()
>
> 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" -- \
> + COMPREPLY=( $( compgen -W "socket kprobe \
> + kretprobe classifier flow_dissector \
> + 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" ) )
Thanks! :)
> return 0
> ;;
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 4654d9450cd9..b808a67d1d3e 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
> [BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
> [BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
> [BPF_SK_MSG_VERDICT] = "msg_verdict",
> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
> [__MAX_BPF_ATTACH_TYPE] = NULL,
> };
>
> @@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
> return a->idx - b->idx;
> }
>
> -static int do_attach(int argc, char **argv)
> +static int parse_atach_detach_args(int argc, char **argv, int *progfd,
> + enum bpf_attach_type *attach_type,
> + int *mapfd)
> {
> - enum bpf_attach_type attach_type;
> - int err, mapfd, progfd;
> -
> - if (!REQ_ARGS(5)) {
> - p_err("too few parameters for map attach");
> + if (!REQ_ARGS(3)) {
> + p_err("too few parameters for attach/detach");
I know this is not existing bug but we didn't catch it when
attach/ /detach was added :( - REQ_ARGS() already includes a p_err(),
so we would have a duplicate error in JSON. Please drop the error here
and below.
With that fix:
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks for the work!
> return -EINVAL;
> }
>
> - progfd = prog_parse_fd(&argc, &argv);
> - if (progfd < 0)
> - return progfd;
> + *progfd = prog_parse_fd(&argc, &argv);
> + if (*progfd < 0)
> + return *progfd;
>
> - attach_type = parse_attach_type(*argv);
> - if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> - p_err("invalid attach type");
> + *attach_type = parse_attach_type(*argv);
> + if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach/detach type");
> return -EINVAL;
> }
> +
> + if (*attach_type == BPF_FLOW_DISSECTOR) {
> + *mapfd = -1;
> + return 0;
> + }
> +
> NEXT_ARG();
> + if (!REQ_ARGS(2)) {
> + p_err("too few parameters for map attach/detach");
> + return -EINVAL;
> + }
>
> - mapfd = map_parse_fd(&argc, &argv);
> - if (mapfd < 0)
> - return mapfd;
> + *mapfd = map_parse_fd(&argc, &argv);
> + if (*mapfd < 0)
> + return *mapfd;
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 3:50 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 3:50 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:13 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf at google.com>
>
> This commit adds support for loading/attaching/detaching flow
> dissector program. The structure of the flow dissector program is
> assumed to be the same as in the selftests:
nit: I don't think we make any assumptions any more? Since the
sub-programs are added to the map explicitly by the user?
> * flow_dissector section with the main entry point
> * a bunch of tail call progs
> * a jmp_table map that is populated with the tail call progs
>
[...]
> @@ -338,7 +339,16 @@ _bpftool()
>
> 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" -- \
> + COMPREPLY=( $( compgen -W "socket kprobe \
> + kretprobe classifier flow_dissector \
> + 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" ) )
Thanks! :)
> return 0
> ;;
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 4654d9450cd9..b808a67d1d3e 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
> [BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
> [BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
> [BPF_SK_MSG_VERDICT] = "msg_verdict",
> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
> [__MAX_BPF_ATTACH_TYPE] = NULL,
> };
>
> @@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
> return a->idx - b->idx;
> }
>
> -static int do_attach(int argc, char **argv)
> +static int parse_atach_detach_args(int argc, char **argv, int *progfd,
> + enum bpf_attach_type *attach_type,
> + int *mapfd)
> {
> - enum bpf_attach_type attach_type;
> - int err, mapfd, progfd;
> -
> - if (!REQ_ARGS(5)) {
> - p_err("too few parameters for map attach");
> + if (!REQ_ARGS(3)) {
> + p_err("too few parameters for attach/detach");
I know this is not existing bug but we didn't catch it when
attach/ /detach was added :( - REQ_ARGS() already includes a p_err(),
so we would have a duplicate error in JSON. Please drop the error here
and below.
With that fix:
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
Thanks for the work!
> return -EINVAL;
> }
>
> - progfd = prog_parse_fd(&argc, &argv);
> - if (progfd < 0)
> - return progfd;
> + *progfd = prog_parse_fd(&argc, &argv);
> + if (*progfd < 0)
> + return *progfd;
>
> - attach_type = parse_attach_type(*argv);
> - if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> - p_err("invalid attach type");
> + *attach_type = parse_attach_type(*argv);
> + if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach/detach type");
> return -EINVAL;
> }
> +
> + if (*attach_type == BPF_FLOW_DISSECTOR) {
> + *mapfd = -1;
> + return 0;
> + }
> +
> NEXT_ARG();
> + if (!REQ_ARGS(2)) {
> + p_err("too few parameters for map attach/detach");
> + return -EINVAL;
> + }
>
> - mapfd = map_parse_fd(&argc, &argv);
> - if (mapfd < 0)
> - return mapfd;
> + *mapfd = map_parse_fd(&argc, &argv);
> + if (*mapfd < 0)
> + return *mapfd;
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 48+ messages in thread* [PATCH v4 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 3:50 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 3:50 UTC (permalink / raw)
On Thu, 8 Nov 2018 16:22:13 -0800, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <sdf at google.com>
>
> This commit adds support for loading/attaching/detaching flow
> dissector program. The structure of the flow dissector program is
> assumed to be the same as in the selftests:
nit: I don't think we make any assumptions any more? Since the
sub-programs are added to the map explicitly by the user?
> * flow_dissector section with the main entry point
> * a bunch of tail call progs
> * a jmp_table map that is populated with the tail call progs
>
[...]
> @@ -338,7 +339,16 @@ _bpftool()
>
> 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" -- \
> + COMPREPLY=( $( compgen -W "socket kprobe \
> + kretprobe classifier flow_dissector \
> + 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" ) )
Thanks! :)
> return 0
> ;;
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 4654d9450cd9..b808a67d1d3e 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -81,6 +81,7 @@ static const char * const attach_type_strings[] = {
> [BPF_SK_SKB_STREAM_PARSER] = "stream_parser",
> [BPF_SK_SKB_STREAM_VERDICT] = "stream_verdict",
> [BPF_SK_MSG_VERDICT] = "msg_verdict",
> + [BPF_FLOW_DISSECTOR] = "flow_dissector",
> [__MAX_BPF_ATTACH_TYPE] = NULL,
> };
>
> @@ -721,30 +722,53 @@ int map_replace_compar(const void *p1, const void *p2)
> return a->idx - b->idx;
> }
>
> -static int do_attach(int argc, char **argv)
> +static int parse_atach_detach_args(int argc, char **argv, int *progfd,
> + enum bpf_attach_type *attach_type,
> + int *mapfd)
> {
> - enum bpf_attach_type attach_type;
> - int err, mapfd, progfd;
> -
> - if (!REQ_ARGS(5)) {
> - p_err("too few parameters for map attach");
> + if (!REQ_ARGS(3)) {
> + p_err("too few parameters for attach/detach");
I know this is not existing bug but we didn't catch it when
attach/ /detach was added :( - REQ_ARGS() already includes a p_err(),
so we would have a duplicate error in JSON. Please drop the error here
and below.
With that fix:
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
Thanks for the work!
> return -EINVAL;
> }
>
> - progfd = prog_parse_fd(&argc, &argv);
> - if (progfd < 0)
> - return progfd;
> + *progfd = prog_parse_fd(&argc, &argv);
> + if (*progfd < 0)
> + return *progfd;
>
> - attach_type = parse_attach_type(*argv);
> - if (attach_type == __MAX_BPF_ATTACH_TYPE) {
> - p_err("invalid attach type");
> + *attach_type = parse_attach_type(*argv);
> + if (*attach_type == __MAX_BPF_ATTACH_TYPE) {
> + p_err("invalid attach/detach type");
> return -EINVAL;
> }
> +
> + if (*attach_type == BPF_FLOW_DISSECTOR) {
> + *mapfd = -1;
> + return 0;
> + }
> +
> NEXT_ARG();
> + if (!REQ_ARGS(2)) {
> + p_err("too few parameters for map attach/detach");
> + return -EINVAL;
> + }
>
> - mapfd = map_parse_fd(&argc, &argv);
> - if (mapfd < 0)
> - return mapfd;
> + *mapfd = map_parse_fd(&argc, &argv);
> + if (*mapfd < 0)
> + return *mapfd;
> +
> + return 0;
> +}
> +
^ permalink raw reply [flat|nested] 48+ messages in thread