* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
v5 changes:
* FILE -> PATH for load/loadall (can be either file or directory now)
* simpler implementation for __bpf_program__pin_name
* removed p_err for REQ_ARGS checks
* parse_atach_detach_args -> parse_attach_detach_args
* for -> while in bpf_object__pin_{programs,maps} recovery
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 | 183 ++++++---
tools/lib/bpf/libbpf.c | 359 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 537 insertions(+), 122 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
v5 changes:
* FILE -> PATH for load/loadall (can be either file or directory now)
* simpler implementation for __bpf_program__pin_name
* removed p_err for REQ_ARGS checks
* parse_atach_detach_args -> parse_attach_detach_args
* for -> while in bpf_object__pin_{programs,maps} recovery
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 | 183 ++++++---
tools/lib/bpf/libbpf.c | 359 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 537 insertions(+), 122 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
v5 changes:
* FILE -> PATH for load/loadall (can be either file or directory now)
* simpler implementation for __bpf_program__pin_name
* removed p_err for REQ_ARGS checks
* parse_atach_detach_args -> parse_attach_detach_args
* for -> while in bpf_object__pin_{programs,maps} recovery
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 | 183 ++++++---
tools/lib/bpf/libbpf.c | 359 ++++++++++++++++--
tools/lib/bpf/libbpf.h | 18 +
tools/testing/selftests/bpf/bpf_flow.c | 2 +-
.../selftests/bpf/test_flow_dissector.sh | 2 +-
9 files changed, 537 insertions(+), 122 deletions(-)
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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 v5 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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 v5 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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 v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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 | 324 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 319 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..341008f47c8a 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,53 @@ 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:
+ while ((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 +1966,78 @@ 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:
+ while ((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 +2049,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 +2057,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 +2160,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 +2183,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 +2532,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 +2544,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 v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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 | 324 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 319 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..341008f47c8a 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,53 @@ 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:
+ while ((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 +1966,78 @@ 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:
+ while ((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 +2049,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 +2057,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 +2160,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 +2183,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 +2532,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 +2544,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 v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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 | 324 ++++++++++++++++++++++++++++++++++++++---
tools/lib/bpf/libbpf.h | 18 +++
2 files changed, 319 insertions(+), 23 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d6e62e90e8d4..341008f47c8a 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,53 @@ 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:
+ while ((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 +1966,78 @@ 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:
+ while ((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 +2049,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 +2057,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 +2160,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 +2183,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 +2532,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 +2544,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 v5 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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 341008f47c8a..97ce9f214002 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 v5 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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 341008f47c8a..97ce9f214002 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 v5 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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 341008f47c8a..97ce9f214002 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 v5 bpf-next 4/7] libbpf: add internal pin_name
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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 | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 97ce9f214002..e827542ffa3a 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,17 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name, *p;
+
+ name = p = strdup(prog->section_name);
+ while ((p = strchr(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 +295,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",
@@ -2006,7 +2029,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;
@@ -2028,7 +2051,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)
@@ -2053,7 +2076,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 v5 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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 | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 97ce9f214002..e827542ffa3a 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,17 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name, *p;
+
+ name = p = strdup(prog->section_name);
+ while ((p = strchr(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 +295,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",
@@ -2006,7 +2029,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;
@@ -2028,7 +2051,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)
@@ -2053,7 +2076,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 v5 bpf-next 4/7] libbpf: add internal pin_name
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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 | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 97ce9f214002..e827542ffa3a 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,17 @@ static void bpf_program__exit(struct bpf_program *prog)
prog->idx = -1;
}
+static char *__bpf_program__pin_name(struct bpf_program *prog)
+{
+ char *name, *p;
+
+ name = p = strdup(prog->section_name);
+ while ((p = strchr(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 +295,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",
@@ -2006,7 +2029,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;
@@ -2028,7 +2051,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)
@@ -2053,7 +2076,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 v5 bpf-next 5/7] bpftool: add loadall command
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.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, 81 insertions(+), 43 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..984d125c507a 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* *PATH* [**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,11 @@ 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *PATH*.
+ **bpftool prog load** pins only the first program from the
+ *OBJ* as *PATH*. **bpftool prog loadall** pins all programs
+ from the *OBJ* under *PATH* 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
@@ -93,7 +97,7 @@ DESCRIPTION
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
- Note: *FILE* must be located in *bpffs* mount. It must not
+ Note: *PATH* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
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..3abfee5533e6 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 PATH \\\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 v5 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.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, 81 insertions(+), 43 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..984d125c507a 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* *PATH* [**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,11 @@ 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *PATH*.
+ **bpftool prog load** pins only the first program from the
+ *OBJ* as *PATH*. **bpftool prog loadall** pins all programs
+ from the *OBJ* under *PATH* 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
@@ -93,7 +97,7 @@ DESCRIPTION
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
- Note: *FILE* must be located in *bpffs* mount. It must not
+ Note: *PATH* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
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..3abfee5533e6 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 PATH \\\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 v5 bpf-next 5/7] bpftool: add loadall command
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.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, 81 insertions(+), 43 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ac4e904b10fb..984d125c507a 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* *PATH* [**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,11 @@ 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ Load bpf program(s) from binary *OBJ* and pin as *PATH*.
+ **bpftool prog load** pins only the first program from the
+ *OBJ* as *PATH*. **bpftool prog loadall** pins all programs
+ from the *OBJ* under *PATH* 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
@@ -93,7 +97,7 @@ DESCRIPTION
If **dev** *NAME* is specified program will be loaded onto
given networking device (offload).
- Note: *FILE* must be located in *bpffs* mount. It must not
+ Note: *PATH* must be located in *bpffs* mount. It must not
contain a dot character ('.'), which is reserved for future
extensions of *bpffs*.
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..3abfee5533e6 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 PATH \\\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 v5 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
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>
---
.../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 984d125c507a..15e9172f7e55 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *PATH*.
**bpftool prog load** pins only the first program from the
*OBJ* as *PATH*. **bpftool prog loadall** pins all programs
@@ -96,6 +96,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: *PATH* 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 3abfee5533e6..4bd6ad19fc05 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 PATH \\\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 v5 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
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>
---
.../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 984d125c507a..15e9172f7e55 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *PATH*.
**bpftool prog load** pins only the first program from the
*OBJ* as *PATH*. **bpftool prog loadall** pins all programs
@@ -96,6 +96,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: *PATH* 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 3abfee5533e6..4bd6ad19fc05 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 PATH \\\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 v5 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
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>
---
.../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 984d125c507a..15e9172f7e55 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* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
+ **bpftool prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*] [**pinmaps** *MAP_DIR*]
Load bpf program(s) from binary *OBJ* and pin as *PATH*.
**bpftool prog load** pins only the first program from the
*OBJ* as *PATH*. **bpftool prog loadall** pins all programs
@@ -96,6 +96,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: *PATH* 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 3abfee5533e6..4bd6ad19fc05 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 PATH \\\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 v5 bpf-next 7/7] bpftool: support loading flow dissector
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 16:21 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 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
This commit adds support for loading/attaching/detaching flow
dissector program.
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>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 85 ++++++++++---------
3 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 15e9172f7e55..8db78ed82a71 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* *PATH* [**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
@@ -103,13 +105,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 4bd6ad19fc05..9adc56e0885c 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,49 @@ 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_attach_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))
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))
+ return -EINVAL;
+
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_attach_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 +780,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;
- }
-
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ int err, progfd;
+ int mapfd;
- 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_attach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1098,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 +1111,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 v5 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-09 16:21 UTC (permalink / raw)
This commit adds support for loading/attaching/detaching flow
dissector program.
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>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 85 ++++++++++---------
3 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 15e9172f7e55..8db78ed82a71 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* *PATH* [**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
@@ -103,13 +105,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 4bd6ad19fc05..9adc56e0885c 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,49 @@ 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_attach_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))
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))
+ return -EINVAL;
+
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_attach_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 +780,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;
- }
-
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ int err, progfd;
+ int mapfd;
- 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_attach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1098,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 +1111,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 v5 bpf-next 7/7] bpftool: support loading flow dissector
@ 2018-11-09 16:21 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-09 16:21 UTC (permalink / raw)
This commit adds support for loading/attaching/detaching flow
dissector program.
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>
Acked-by: Jakub Kicinski <jakub.kicinski at netronome.com>
---
.../bpftool/Documentation/bpftool-prog.rst | 26 +++---
tools/bpf/bpftool/bash-completion/bpftool | 14 ++-
tools/bpf/bpftool/prog.c | 85 ++++++++++---------
3 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 15e9172f7e55..8db78ed82a71 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* *PATH* [**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
@@ -103,13 +105,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 4bd6ad19fc05..9adc56e0885c 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,49 @@ 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_attach_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))
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))
+ return -EINVAL;
+
+ *mapfd = map_parse_fd(&argc, &argv);
+ if (*mapfd < 0)
+ return *mapfd;
+
+ return 0;
+}
- mapfd = map_parse_fd(&argc, &argv);
- if (mapfd < 0)
- return mapfd;
+static int do_attach(int argc, char **argv)
+{
+ enum bpf_attach_type attach_type;
+ int err, progfd;
+ int mapfd;
+
+ err = parse_attach_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 +780,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;
- }
-
- progfd = prog_parse_fd(&argc, &argv);
- if (progfd < 0)
- return progfd;
+ int err, progfd;
+ int mapfd;
- 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_attach_detach_args(argc, argv,
+ &progfd, &attach_type, &mapfd);
+ if (err)
+ return err;
err = bpf_prog_detach2(progfd, mapfd, attach_type);
if (err) {
@@ -1092,8 +1098,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 +1111,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 v5 bpf-next 0/7] bpftool: support loading flow dissector
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 17:38 ` quentin.monnet
-1 siblings, 0 replies; 48+ messages in thread
From: Quentin Monnet @ 2018-11-09 17:38 UTC (permalink / raw)
To: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski
Cc: guro, jiong.wang, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
2018-11-09 08:21 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
>
> 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
The series look good to me, thanks!
For the bpftool parts:
Acked-by: Quentin Monnet <quentin.monnet@netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 17:38 ` quentin.monnet
0 siblings, 0 replies; 48+ messages in thread
From: quentin.monnet @ 2018-11-09 17:38 UTC (permalink / raw)
2018-11-09 08:21 UTC-0800 ~ Stanislav Fomichev <sdf at google.com>
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
>
> 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
The series look good to me, thanks!
For the bpftool parts:
Acked-by: Quentin Monnet <quentin.monnet at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 17:38 ` quentin.monnet
0 siblings, 0 replies; 48+ messages in thread
From: Quentin Monnet @ 2018-11-09 17:38 UTC (permalink / raw)
2018-11-09 08:21 UTC-0800 ~ Stanislav Fomichev <sdf at google.com>
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
>
> 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
The series look good to me, thanks!
For the bpftool parts:
Acked-by: Quentin Monnet <quentin.monnet at netronome.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-09 20:15 ` jakub.kicinski
-1 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 20:15 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet,
guro, jiong.wang, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 20:15 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: jakub.kicinski @ 2018-11-09 20:15 UTC (permalink / raw)
On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-09 20:15 ` jakub.kicinski
0 siblings, 0 replies; 48+ messages in thread
From: Jakub Kicinski @ 2018-11-09 20:15 UTC (permalink / raw)
On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> v5 changes:
> * FILE -> PATH for load/loadall (can be either file or directory now)
> * simpler implementation for __bpf_program__pin_name
> * removed p_err for REQ_ARGS checks
> * parse_atach_detach_args -> parse_attach_detach_args
> * for -> while in bpf_object__pin_{programs,maps} recovery
Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
2018-11-09 20:15 ` jakub.kicinski
(?)
@ 2018-11-11 0:00 ` alexei.starovoitov
-1 siblings, 0 replies; 48+ messages in thread
From: Alexei Starovoitov @ 2018-11-11 0:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
quentin.monnet, guro, jiong.wang, bhole_prashant_q7,
john.fastabend, jbenc, treeze.taeung, yhs, osk, sandipan
On Fri, Nov 09, 2018 at 12:15:11PM -0800, Jakub Kicinski wrote:
> On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> > v5 changes:
> > * FILE -> PATH for load/loadall (can be either file or directory now)
> > * simpler implementation for __bpf_program__pin_name
> > * removed p_err for REQ_ARGS checks
> > * parse_atach_detach_args -> parse_attach_detach_args
> > * for -> while in bpf_object__pin_{programs,maps} recovery
>
> Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
Whole thing looks good to me.
Applied, Thanks everyone
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-11 0:00 ` alexei.starovoitov
0 siblings, 0 replies; 48+ messages in thread
From: alexei.starovoitov @ 2018-11-11 0:00 UTC (permalink / raw)
On Fri, Nov 09, 2018 at 12:15:11PM -0800, Jakub Kicinski wrote:
> On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> > v5 changes:
> > * FILE -> PATH for load/loadall (can be either file or directory now)
> > * simpler implementation for __bpf_program__pin_name
> > * removed p_err for REQ_ARGS checks
> > * parse_atach_detach_args -> parse_attach_detach_args
> > * for -> while in bpf_object__pin_{programs,maps} recovery
>
> Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
Whole thing looks good to me.
Applied, Thanks everyone
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
@ 2018-11-11 0:00 ` alexei.starovoitov
0 siblings, 0 replies; 48+ messages in thread
From: Alexei Starovoitov @ 2018-11-11 0:00 UTC (permalink / raw)
On Fri, Nov 09, 2018@12:15:11PM -0800, Jakub Kicinski wrote:
> On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> > v5 changes:
> > * FILE -> PATH for load/loadall (can be either file or directory now)
> > * simpler implementation for __bpf_program__pin_name
> > * removed p_err for REQ_ARGS checks
> > * parse_atach_detach_args -> parse_attach_detach_args
> > * for -> while in bpf_object__pin_{programs,maps} recovery
>
> Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
Whole thing looks good to me.
Applied, Thanks everyone
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-09 16:21 ` sdf
(?)
@ 2018-11-12 21:53 ` kafai
-1 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 21:53 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, jakub.kicinski,
quentin.monnet, Roman Gushchin, jiong.wang, bhole_prashant_q7,
john.fastabend, jbenc, treeze.taeung, Yonghong Song,
Okash Khawaja, sandipan
On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
[ ... ]
> @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> + if (prev == NULL)
> + return obj->programs;
> +
This patch breaks the behavior introduced in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
"Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main."
For example, the userspace could have been doing:
prog = bpf_program__next(NULL, obj);
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
bpf_object__load(obj);
For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
the prog returned by bpf_program__next() could be in ".text" instead of
the main bpf program. The next bpf_program__set_type() has
no effect to the main program. The following bpf_object__load()
will catch user in surprise with the main bpf prog in
the wrong BPF_PROG_TYPE.
> 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 21:53 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: kafai @ 2018-11-12 21:53 UTC (permalink / raw)
On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
[ ... ]
> @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> + if (prev == NULL)
> + return obj->programs;
> +
This patch breaks the behavior introduced in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
"Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main."
For example, the userspace could have been doing:
prog = bpf_program__next(NULL, obj);
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
bpf_object__load(obj);
For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
the prog returned by bpf_program__next() could be in ".text" instead of
the main bpf program. The next bpf_program__set_type() has
no effect to the main program. The following bpf_object__load()
will catch user in surprise with the main bpf prog in
the wrong BPF_PROG_TYPE.
> 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 21:53 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 21:53 UTC (permalink / raw)
On Fri, Nov 09, 2018@08:21:41AM -0800, Stanislav Fomichev wrote:
[ ... ]
> @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> + if (prev == NULL)
> + return obj->programs;
> +
This patch breaks the behavior introduced in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
"Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main."
For example, the userspace could have been doing:
prog = bpf_program__next(NULL, obj);
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
bpf_object__load(obj);
For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
the prog returned by bpf_program__next() could be in ".text" instead of
the main bpf program. The next bpf_program__set_type() has
no effect to the main program. The following bpf_object__load()
will catch user in surprise with the main bpf prog in
the wrong BPF_PROG_TYPE.
> 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-12 21:53 ` kafai
(?)
@ 2018-11-12 22:10 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 22:10 UTC (permalink / raw)
To: Martin Lau
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski, quentin.monnet, Roman Gushchin, jiong.wang,
bhole_prashant_q7, john.fastabend, jbenc, treeze.taeung,
Yonghong Song, Okash Khawaja,
On 11/12, Martin Lau wrote:
> On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> [ ... ]
> > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > + if (prev == NULL)
> > + return obj->programs;
> > +
> This patch breaks the behavior introduced in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> "Make bpf_program__next() skip over '.text' section if object file
> has pseudo calls. The '.text' section is hardly a program in that
> case, it's more of a storage for code of functions other than main."
>
> For example, the userspace could have been doing:
> prog = bpf_program__next(NULL, obj);
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> bpf_object__load(obj);
>
> For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> the prog returned by bpf_program__next() could be in ".text" instead of
> the main bpf program. The next bpf_program__set_type() has
> no effect to the main program. The following bpf_object__load()
> will catch user in surprise with the main bpf prog in
> the wrong BPF_PROG_TYPE.
Will something like the following fix your concern? (plus, assuming the
same for prev):
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
+ if (prev == NULL) {
+ prog = obj->programs;
+ if (!prog || !bpf_program__is_function_storage(prog, obj))
+ return prog;
+ }
do {
prog = __bpf_program__iter(prog, obj, 1);
Any suggestions for a better way to do it?
> > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 22:10 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-12 22:10 UTC (permalink / raw)
On 11/12, Martin Lau wrote:
> On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> [ ... ]
> > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > + if (prev == NULL)
> > + return obj->programs;
> > +
> This patch breaks the behavior introduced in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> "Make bpf_program__next() skip over '.text' section if object file
> has pseudo calls. The '.text' section is hardly a program in that
> case, it's more of a storage for code of functions other than main."
>
> For example, the userspace could have been doing:
> prog = bpf_program__next(NULL, obj);
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> bpf_object__load(obj);
>
> For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> the prog returned by bpf_program__next() could be in ".text" instead of
> the main bpf program. The next bpf_program__set_type() has
> no effect to the main program. The following bpf_object__load()
> will catch user in surprise with the main bpf prog in
> the wrong BPF_PROG_TYPE.
Will something like the following fix your concern? (plus, assuming the
same for prev):
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
+ if (prev == NULL) {
+ prog = obj->programs;
+ if (!prog || !bpf_program__is_function_storage(prog, obj))
+ return prog;
+ }
do {
prog = __bpf_program__iter(prog, obj, 1);
Any suggestions for a better way to do it?
> > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 22:10 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 22:10 UTC (permalink / raw)
On 11/12, Martin Lau wrote:
> On Fri, Nov 09, 2018@08:21:41AM -0800, Stanislav Fomichev wrote:
> [ ... ]
> > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > + if (prev == NULL)
> > + return obj->programs;
> > +
> This patch breaks the behavior introduced in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> "Make bpf_program__next() skip over '.text' section if object file
> has pseudo calls. The '.text' section is hardly a program in that
> case, it's more of a storage for code of functions other than main."
>
> For example, the userspace could have been doing:
> prog = bpf_program__next(NULL, obj);
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> bpf_object__load(obj);
>
> For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> the prog returned by bpf_program__next() could be in ".text" instead of
> the main bpf program. The next bpf_program__set_type() has
> no effect to the main program. The following bpf_object__load()
> will catch user in surprise with the main bpf prog in
> the wrong BPF_PROG_TYPE.
Will something like the following fix your concern? (plus, assuming the
same for prev):
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
+ if (prev == NULL) {
+ prog = obj->programs;
+ if (!prog || !bpf_program__is_function_storage(prog, obj))
+ return prog;
+ }
do {
prog = __bpf_program__iter(prog, obj, 1);
Any suggestions for a better way to do it?
> > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-12 22:10 ` sdf
(?)
@ 2018-11-12 23:10 ` kafai
-1 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 23:10 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski, quentin.monnet, Roman Gushchin, jiong.wang,
bhole_prashant_q7, john.fastabend, jbenc, treeze.taeung,
Yonghong Song, Okash Khawaja,
On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > [ ... ]
> > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > + if (prev == NULL)
> > > + return obj->programs;
> > > +
> > This patch breaks the behavior introduced in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > "Make bpf_program__next() skip over '.text' section if object file
> > has pseudo calls. The '.text' section is hardly a program in that
> > case, it's more of a storage for code of functions other than main."
> >
> > For example, the userspace could have been doing:
> > prog = bpf_program__next(NULL, obj);
> > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > bpf_object__load(obj);
> >
> > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > the prog returned by bpf_program__next() could be in ".text" instead of
> > the main bpf program. The next bpf_program__set_type() has
> > no effect to the main program. The following bpf_object__load()
> > will catch user in surprise with the main bpf prog in
> > the wrong BPF_PROG_TYPE.
>
> Will something like the following fix your concern? (plus, assuming the
> same for prev):
>
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> - if (prev == NULL)
> - return obj->programs;
> + if (prev == NULL) {
> + prog = obj->programs;
> + if (!prog || !bpf_program__is_function_storage(prog, obj))
> + return prog;
> + }
>
> do {
> prog = __bpf_program__iter(prog, obj, 1);
>
> Any suggestions for a better way to do it?
I think that would work. The bpf_program__prev() will need the same
treatment though...
Here is my mostly untested fix to unblock my other dev works. It moves
the very first NULL check back to __bpf_program__iter():
>From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 12 Nov 2018 14:52:39 -0800
Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
such that bpf_program__next() does not return pseudo programs in ".text".
Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
{
+ size_t nr_programs = obj->nr_programs;
ssize_t idx;
- if (!obj->programs)
+ if (!nr_programs)
return NULL;
+ if (!p)
+ /* Iter from the beginning */
+ return forward ? &obj->programs[0] :
+ &obj->programs[nr_programs - 1];
+
if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (p - obj->programs) + i;
+ idx = (p - obj->programs) + (forward ? 1 : -1);
if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
+ prog = __bpf_program__iter(prog, obj, true);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2231,14 +2234,8 @@ 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);
+ prog = __bpf_program__iter(prog, obj, false);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
--
2.17.1
>
> > > 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;
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:10 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: kafai @ 2018-11-12 23:10 UTC (permalink / raw)
On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > [ ... ]
> > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > + if (prev == NULL)
> > > + return obj->programs;
> > > +
> > This patch breaks the behavior introduced in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > "Make bpf_program__next() skip over '.text' section if object file
> > has pseudo calls. The '.text' section is hardly a program in that
> > case, it's more of a storage for code of functions other than main."
> >
> > For example, the userspace could have been doing:
> > prog = bpf_program__next(NULL, obj);
> > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > bpf_object__load(obj);
> >
> > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > the prog returned by bpf_program__next() could be in ".text" instead of
> > the main bpf program. The next bpf_program__set_type() has
> > no effect to the main program. The following bpf_object__load()
> > will catch user in surprise with the main bpf prog in
> > the wrong BPF_PROG_TYPE.
>
> Will something like the following fix your concern? (plus, assuming the
> same for prev):
>
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> - if (prev == NULL)
> - return obj->programs;
> + if (prev == NULL) {
> + prog = obj->programs;
> + if (!prog || !bpf_program__is_function_storage(prog, obj))
> + return prog;
> + }
>
> do {
> prog = __bpf_program__iter(prog, obj, 1);
>
> Any suggestions for a better way to do it?
I think that would work. The bpf_program__prev() will need the same
treatment though...
Here is my mostly untested fix to unblock my other dev works. It moves
the very first NULL check back to __bpf_program__iter():
>From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
From: Martin KaFai Lau <kafai at fb.com>
Date: Mon, 12 Nov 2018 14:52:39 -0800
Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
such that bpf_program__next() does not return pseudo programs in ".text".
Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai at fb.com>
---
tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
{
+ size_t nr_programs = obj->nr_programs;
ssize_t idx;
- if (!obj->programs)
+ if (!nr_programs)
return NULL;
+ if (!p)
+ /* Iter from the beginning */
+ return forward ? &obj->programs[0] :
+ &obj->programs[nr_programs - 1];
+
if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (p - obj->programs) + i;
+ idx = (p - obj->programs) + (forward ? 1 : -1);
if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
+ prog = __bpf_program__iter(prog, obj, true);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2231,14 +2234,8 @@ 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);
+ prog = __bpf_program__iter(prog, obj, false);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
--
2.17.1
>
> > > 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;
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:10 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 23:10 UTC (permalink / raw)
On Mon, Nov 12, 2018@02:10:11PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Fri, Nov 09, 2018@08:21:41AM -0800, Stanislav Fomichev wrote:
> > [ ... ]
> > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > + if (prev == NULL)
> > > + return obj->programs;
> > > +
> > This patch breaks the behavior introduced in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > "Make bpf_program__next() skip over '.text' section if object file
> > has pseudo calls. The '.text' section is hardly a program in that
> > case, it's more of a storage for code of functions other than main."
> >
> > For example, the userspace could have been doing:
> > prog = bpf_program__next(NULL, obj);
> > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > bpf_object__load(obj);
> >
> > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > the prog returned by bpf_program__next() could be in ".text" instead of
> > the main bpf program. The next bpf_program__set_type() has
> > no effect to the main program. The following bpf_object__load()
> > will catch user in surprise with the main bpf prog in
> > the wrong BPF_PROG_TYPE.
>
> Will something like the following fix your concern? (plus, assuming the
> same for prev):
>
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> - if (prev == NULL)
> - return obj->programs;
> + if (prev == NULL) {
> + prog = obj->programs;
> + if (!prog || !bpf_program__is_function_storage(prog, obj))
> + return prog;
> + }
>
> do {
> prog = __bpf_program__iter(prog, obj, 1);
>
> Any suggestions for a better way to do it?
I think that would work. The bpf_program__prev() will need the same
treatment though...
Here is my mostly untested fix to unblock my other dev works. It moves
the very first NULL check back to __bpf_program__iter():
>From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
From: Martin KaFai Lau <kafai@fb.com>
Date: Mon, 12 Nov 2018 14:52:39 -0800
Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
This patch restores the behavior in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
such that bpf_program__next() does not return pseudo programs in ".text".
Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
Signed-off-by: Martin KaFai Lau <kafai at fb.com>
---
tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e827542ffa3a..a01eb9584e52 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
}
static struct bpf_program *
-__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
+__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
{
+ size_t nr_programs = obj->nr_programs;
ssize_t idx;
- if (!obj->programs)
+ if (!nr_programs)
return NULL;
+ if (!p)
+ /* Iter from the beginning */
+ return forward ? &obj->programs[0] :
+ &obj->programs[nr_programs - 1];
+
if (p->obj != obj) {
pr_warning("error: program handler doesn't match object\n");
return NULL;
}
- idx = (p - obj->programs) + i;
+ idx = (p - obj->programs) + (forward ? 1 : -1);
if (idx >= obj->nr_programs || idx < 0)
return NULL;
return &obj->programs[idx];
@@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
+ prog = __bpf_program__iter(prog, obj, true);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
@@ -2231,14 +2234,8 @@ 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);
+ prog = __bpf_program__iter(prog, obj, false);
} while (prog && bpf_program__is_function_storage(prog, obj));
return prog;
--
2.17.1
>
> > > 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;
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-12 23:10 ` kafai
(?)
@ 2018-11-12 23:29 ` sdf
-1 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 23:29 UTC (permalink / raw)
To: Martin Lau
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski, quentin.monnet, Roman Gushchin, jiong.wang,
bhole_prashant_q7, john.fastabend, jbenc, treeze.taeung,
Yonghong Song, Okash Khawaja,
On 11/12, Martin Lau wrote:
> On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > On 11/12, Martin Lau wrote:
> > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > [ ... ]
> > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > {
> > > > struct bpf_program *prog = prev;
> > > >
> > > > + if (prev == NULL)
> > > > + return obj->programs;
> > > > +
> > > This patch breaks the behavior introduced in
> > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > "Make bpf_program__next() skip over '.text' section if object file
> > > has pseudo calls. The '.text' section is hardly a program in that
> > > case, it's more of a storage for code of functions other than main."
> > >
> > > For example, the userspace could have been doing:
> > > prog = bpf_program__next(NULL, obj);
> > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > bpf_object__load(obj);
> > >
> > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > the main bpf program. The next bpf_program__set_type() has
> > > no effect to the main program. The following bpf_object__load()
> > > will catch user in surprise with the main bpf prog in
> > > the wrong BPF_PROG_TYPE.
> >
> > Will something like the following fix your concern? (plus, assuming the
> > same for prev):
> >
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > - if (prev == NULL)
> > - return obj->programs;
> > + if (prev == NULL) {
> > + prog = obj->programs;
> > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > + return prog;
> > + }
> >
> > do {
> > prog = __bpf_program__iter(prog, obj, 1);
> >
> > Any suggestions for a better way to do it?
> I think that would work. The bpf_program__prev() will need the same
> treatment though...
>
> Here is my mostly untested fix to unblock my other dev works. It moves
> the very first NULL check back to __bpf_program__iter():
I like your version and it works with my simple flow dissector test :-)
Thanks for spotting and fixing it!
> From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> From: Martin KaFai Lau <kafai@fb.com>
> Date: Mon, 12 Nov 2018 14:52:39 -0800
> Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
>
> This patch restores the behavior in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> such that bpf_program__next() does not return pseudo programs in ".text".
>
> Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e827542ffa3a..a01eb9584e52 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> {
> + size_t nr_programs = obj->nr_programs;
> ssize_t idx;
>
> - if (!obj->programs)
> + if (!nr_programs)
> return NULL;
>
> + if (!p)
> + /* Iter from the beginning */
> + return forward ? &obj->programs[0] :
> + &obj->programs[nr_programs - 1];
> +
> if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (p - obj->programs) + i;
> + idx = (p - obj->programs) + (forward ? 1 : -1);
> if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> + prog = __bpf_program__iter(prog, obj, true);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> @@ -2231,14 +2234,8 @@ 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);
> + prog = __bpf_program__iter(prog, obj, false);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> --
> 2.17.1
>
> >
> > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:29 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: sdf @ 2018-11-12 23:29 UTC (permalink / raw)
On 11/12, Martin Lau wrote:
> On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > On 11/12, Martin Lau wrote:
> > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > [ ... ]
> > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > {
> > > > struct bpf_program *prog = prev;
> > > >
> > > > + if (prev == NULL)
> > > > + return obj->programs;
> > > > +
> > > This patch breaks the behavior introduced in
> > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > "Make bpf_program__next() skip over '.text' section if object file
> > > has pseudo calls. The '.text' section is hardly a program in that
> > > case, it's more of a storage for code of functions other than main."
> > >
> > > For example, the userspace could have been doing:
> > > prog = bpf_program__next(NULL, obj);
> > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > bpf_object__load(obj);
> > >
> > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > the main bpf program. The next bpf_program__set_type() has
> > > no effect to the main program. The following bpf_object__load()
> > > will catch user in surprise with the main bpf prog in
> > > the wrong BPF_PROG_TYPE.
> >
> > Will something like the following fix your concern? (plus, assuming the
> > same for prev):
> >
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > - if (prev == NULL)
> > - return obj->programs;
> > + if (prev == NULL) {
> > + prog = obj->programs;
> > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > + return prog;
> > + }
> >
> > do {
> > prog = __bpf_program__iter(prog, obj, 1);
> >
> > Any suggestions for a better way to do it?
> I think that would work. The bpf_program__prev() will need the same
> treatment though...
>
> Here is my mostly untested fix to unblock my other dev works. It moves
> the very first NULL check back to __bpf_program__iter():
I like your version and it works with my simple flow dissector test :-)
Thanks for spotting and fixing it!
> From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> From: Martin KaFai Lau <kafai at fb.com>
> Date: Mon, 12 Nov 2018 14:52:39 -0800
> Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
>
> This patch restores the behavior in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> such that bpf_program__next() does not return pseudo programs in ".text".
>
> Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> Signed-off-by: Martin KaFai Lau <kafai at fb.com>
> ---
> tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e827542ffa3a..a01eb9584e52 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> {
> + size_t nr_programs = obj->nr_programs;
> ssize_t idx;
>
> - if (!obj->programs)
> + if (!nr_programs)
> return NULL;
>
> + if (!p)
> + /* Iter from the beginning */
> + return forward ? &obj->programs[0] :
> + &obj->programs[nr_programs - 1];
> +
> if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (p - obj->programs) + i;
> + idx = (p - obj->programs) + (forward ? 1 : -1);
> if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> + prog = __bpf_program__iter(prog, obj, true);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> @@ -2231,14 +2234,8 @@ 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);
> + prog = __bpf_program__iter(prog, obj, false);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> --
> 2.17.1
>
> >
> > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:29 ` sdf
0 siblings, 0 replies; 48+ messages in thread
From: Stanislav Fomichev @ 2018-11-12 23:29 UTC (permalink / raw)
On 11/12, Martin Lau wrote:
> On Mon, Nov 12, 2018@02:10:11PM -0800, Stanislav Fomichev wrote:
> > On 11/12, Martin Lau wrote:
> > > On Fri, Nov 09, 2018@08:21:41AM -0800, Stanislav Fomichev wrote:
> > > [ ... ]
> > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > {
> > > > struct bpf_program *prog = prev;
> > > >
> > > > + if (prev == NULL)
> > > > + return obj->programs;
> > > > +
> > > This patch breaks the behavior introduced in
> > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > "Make bpf_program__next() skip over '.text' section if object file
> > > has pseudo calls. The '.text' section is hardly a program in that
> > > case, it's more of a storage for code of functions other than main."
> > >
> > > For example, the userspace could have been doing:
> > > prog = bpf_program__next(NULL, obj);
> > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > bpf_object__load(obj);
> > >
> > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > the main bpf program. The next bpf_program__set_type() has
> > > no effect to the main program. The following bpf_object__load()
> > > will catch user in surprise with the main bpf prog in
> > > the wrong BPF_PROG_TYPE.
> >
> > Will something like the following fix your concern? (plus, assuming the
> > same for prev):
> >
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > - if (prev == NULL)
> > - return obj->programs;
> > + if (prev == NULL) {
> > + prog = obj->programs;
> > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > + return prog;
> > + }
> >
> > do {
> > prog = __bpf_program__iter(prog, obj, 1);
> >
> > Any suggestions for a better way to do it?
> I think that would work. The bpf_program__prev() will need the same
> treatment though...
>
> Here is my mostly untested fix to unblock my other dev works. It moves
> the very first NULL check back to __bpf_program__iter():
I like your version and it works with my simple flow dissector test :-)
Thanks for spotting and fixing it!
> From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> From: Martin KaFai Lau <kafai at fb.com>
> Date: Mon, 12 Nov 2018 14:52:39 -0800
> Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
>
> This patch restores the behavior in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> such that bpf_program__next() does not return pseudo programs in ".text".
>
> Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> Signed-off-by: Martin KaFai Lau <kafai at fb.com>
> ---
> tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e827542ffa3a..a01eb9584e52 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> {
> + size_t nr_programs = obj->nr_programs;
> ssize_t idx;
>
> - if (!obj->programs)
> + if (!nr_programs)
> return NULL;
>
> + if (!p)
> + /* Iter from the beginning */
> + return forward ? &obj->programs[0] :
> + &obj->programs[nr_programs - 1];
> +
> if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (p - obj->programs) + i;
> + idx = (p - obj->programs) + (forward ? 1 : -1);
> if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> + prog = __bpf_program__iter(prog, obj, true);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> @@ -2231,14 +2234,8 @@ 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);
> + prog = __bpf_program__iter(prog, obj, false);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
> --
> 2.17.1
>
> >
> > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
2018-11-12 23:29 ` sdf
(?)
@ 2018-11-12 23:41 ` kafai
-1 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 23:41 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
jakub.kicinski, quentin.monnet, Roman Gushchin, jiong.wang,
bhole_prashant_q7, john.fastabend, jbenc, treeze.taeung,
Yonghong Song, Okash Khawaja,
On Mon, Nov 12, 2018 at 03:29:25PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > > On 11/12, Martin Lau wrote:
> > > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > > [ ... ]
> > > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > {
> > > > > struct bpf_program *prog = prev;
> > > > >
> > > > > + if (prev == NULL)
> > > > > + return obj->programs;
> > > > > +
> > > > This patch breaks the behavior introduced in
> > > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > > "Make bpf_program__next() skip over '.text' section if object file
> > > > has pseudo calls. The '.text' section is hardly a program in that
> > > > case, it's more of a storage for code of functions other than main."
> > > >
> > > > For example, the userspace could have been doing:
> > > > prog = bpf_program__next(NULL, obj);
> > > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > > bpf_object__load(obj);
> > > >
> > > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > > the main bpf program. The next bpf_program__set_type() has
> > > > no effect to the main program. The following bpf_object__load()
> > > > will catch user in surprise with the main bpf prog in
> > > > the wrong BPF_PROG_TYPE.
> > >
> > > Will something like the following fix your concern? (plus, assuming the
> > > same for prev):
> > >
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > - if (prev == NULL)
> > > - return obj->programs;
> > > + if (prev == NULL) {
> > > + prog = obj->programs;
> > > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > > + return prog;
> > > + }
> > >
> > > do {
> > > prog = __bpf_program__iter(prog, obj, 1);
> > >
> > > Any suggestions for a better way to do it?
> > I think that would work. The bpf_program__prev() will need the same
> > treatment though...
> >
> > Here is my mostly untested fix to unblock my other dev works. It moves
> > the very first NULL check back to __bpf_program__iter():
> I like your version and it works with my simple flow dissector test :-)
Great. I will send out an offical patch.
> Thanks for spotting and fixing it!
>
>
> > From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> > From: Martin KaFai Lau <kafai@fb.com>
> > Date: Mon, 12 Nov 2018 14:52:39 -0800
> > Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> >
> > This patch restores the behavior in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > such that bpf_program__next() does not return pseudo programs in ".text".
> >
> > Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> > 1 file changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e827542ffa3a..a01eb9584e52 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> > {
> > + size_t nr_programs = obj->nr_programs;
> > ssize_t idx;
> >
> > - if (!obj->programs)
> > + if (!nr_programs)
> > return NULL;
> >
> > + if (!p)
> > + /* Iter from the beginning */
> > + return forward ? &obj->programs[0] :
> > + &obj->programs[nr_programs - 1];
> > +
> > if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (p - obj->programs) + i;
> > + idx = (p - obj->programs) + (forward ? 1 : -1);
> > if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> > + prog = __bpf_program__iter(prog, obj, true);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > @@ -2231,14 +2234,8 @@ 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);
> > + prog = __bpf_program__iter(prog, obj, false);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > --
> > 2.17.1
> >
> > >
> > > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:41 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: kafai @ 2018-11-12 23:41 UTC (permalink / raw)
On Mon, Nov 12, 2018 at 03:29:25PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Mon, Nov 12, 2018 at 02:10:11PM -0800, Stanislav Fomichev wrote:
> > > On 11/12, Martin Lau wrote:
> > > > On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> > > > [ ... ]
> > > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > {
> > > > > struct bpf_program *prog = prev;
> > > > >
> > > > > + if (prev == NULL)
> > > > > + return obj->programs;
> > > > > +
> > > > This patch breaks the behavior introduced in
> > > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > > "Make bpf_program__next() skip over '.text' section if object file
> > > > has pseudo calls. The '.text' section is hardly a program in that
> > > > case, it's more of a storage for code of functions other than main."
> > > >
> > > > For example, the userspace could have been doing:
> > > > prog = bpf_program__next(NULL, obj);
> > > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > > bpf_object__load(obj);
> > > >
> > > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > > the main bpf program. The next bpf_program__set_type() has
> > > > no effect to the main program. The following bpf_object__load()
> > > > will catch user in surprise with the main bpf prog in
> > > > the wrong BPF_PROG_TYPE.
> > >
> > > Will something like the following fix your concern? (plus, assuming the
> > > same for prev):
> > >
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > - if (prev == NULL)
> > > - return obj->programs;
> > > + if (prev == NULL) {
> > > + prog = obj->programs;
> > > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > > + return prog;
> > > + }
> > >
> > > do {
> > > prog = __bpf_program__iter(prog, obj, 1);
> > >
> > > Any suggestions for a better way to do it?
> > I think that would work. The bpf_program__prev() will need the same
> > treatment though...
> >
> > Here is my mostly untested fix to unblock my other dev works. It moves
> > the very first NULL check back to __bpf_program__iter():
> I like your version and it works with my simple flow dissector test :-)
Great. I will send out an offical patch.
> Thanks for spotting and fixing it!
>
>
> > From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> > From: Martin KaFai Lau <kafai at fb.com>
> > Date: Mon, 12 Nov 2018 14:52:39 -0800
> > Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> >
> > This patch restores the behavior in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > such that bpf_program__next() does not return pseudo programs in ".text".
> >
> > Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> > Signed-off-by: Martin KaFai Lau <kafai at fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> > 1 file changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e827542ffa3a..a01eb9584e52 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> > {
> > + size_t nr_programs = obj->nr_programs;
> > ssize_t idx;
> >
> > - if (!obj->programs)
> > + if (!nr_programs)
> > return NULL;
> >
> > + if (!p)
> > + /* Iter from the beginning */
> > + return forward ? &obj->programs[0] :
> > + &obj->programs[nr_programs - 1];
> > +
> > if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (p - obj->programs) + i;
> > + idx = (p - obj->programs) + (forward ? 1 : -1);
> > if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> > + prog = __bpf_program__iter(prog, obj, true);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > @@ -2231,14 +2234,8 @@ 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);
> > + prog = __bpf_program__iter(prog, obj, false);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > --
> > 2.17.1
> >
> > >
> > > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
@ 2018-11-12 23:41 ` kafai
0 siblings, 0 replies; 48+ messages in thread
From: Martin Lau @ 2018-11-12 23:41 UTC (permalink / raw)
On Mon, Nov 12, 2018@03:29:25PM -0800, Stanislav Fomichev wrote:
> On 11/12, Martin Lau wrote:
> > On Mon, Nov 12, 2018@02:10:11PM -0800, Stanislav Fomichev wrote:
> > > On 11/12, Martin Lau wrote:
> > > > On Fri, Nov 09, 2018@08:21:41AM -0800, Stanislav Fomichev wrote:
> > > > [ ... ]
> > > > > @@ -1918,23 +2160,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 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > > > {
> > > > > struct bpf_program *prog = prev;
> > > > >
> > > > > + if (prev == NULL)
> > > > > + return obj->programs;
> > > > > +
> > > > This patch breaks the behavior introduced in
> > > > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > > > "Make bpf_program__next() skip over '.text' section if object file
> > > > has pseudo calls. The '.text' section is hardly a program in that
> > > > case, it's more of a storage for code of functions other than main."
> > > >
> > > > For example, the userspace could have been doing:
> > > > prog = bpf_program__next(NULL, obj);
> > > > bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> > > > bpf_object__load(obj);
> > > >
> > > > For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> > > > the prog returned by bpf_program__next() could be in ".text" instead of
> > > > the main bpf program. The next bpf_program__set_type() has
> > > > no effect to the main program. The following bpf_object__load()
> > > > will catch user in surprise with the main bpf prog in
> > > > the wrong BPF_PROG_TYPE.
> > >
> > > Will something like the following fix your concern? (plus, assuming the
> > > same for prev):
> > >
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > > {
> > > struct bpf_program *prog = prev;
> > >
> > > - if (prev == NULL)
> > > - return obj->programs;
> > > + if (prev == NULL) {
> > > + prog = obj->programs;
> > > + if (!prog || !bpf_program__is_function_storage(prog, obj))
> > > + return prog;
> > > + }
> > >
> > > do {
> > > prog = __bpf_program__iter(prog, obj, 1);
> > >
> > > Any suggestions for a better way to do it?
> > I think that would work. The bpf_program__prev() will need the same
> > treatment though...
> >
> > Here is my mostly untested fix to unblock my other dev works. It moves
> > the very first NULL check back to __bpf_program__iter():
> I like your version and it works with my simple flow dissector test :-)
Great. I will send out an offical patch.
> Thanks for spotting and fixing it!
>
>
> > From de1c89ae1768e756825a6874268b5b1686695c93 Mon Sep 17 00:00:00 2001
> > From: Martin KaFai Lau <kafai at fb.com>
> > Date: Mon, 12 Nov 2018 14:52:39 -0800
> > Subject: [PATCH] bpf: libbpf: Fix bpf_program__next() API
> >
> > This patch restores the behavior in
> > commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> > such that bpf_program__next() does not return pseudo programs in ".text".
> >
> > Fixes: 0c19a9fbc9cd ("libbpf: cleanup after partial failure in bpf_object__pin")
> > Signed-off-by: Martin KaFai Lau <kafai at fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 25 +++++++++++--------------
> > 1 file changed, 11 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index e827542ffa3a..a01eb9584e52 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -2193,19 +2193,25 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, bool forward)
> > {
> > + size_t nr_programs = obj->nr_programs;
> > ssize_t idx;
> >
> > - if (!obj->programs)
> > + if (!nr_programs)
> > return NULL;
> >
> > + if (!p)
> > + /* Iter from the beginning */
> > + return forward ? &obj->programs[0] :
> > + &obj->programs[nr_programs - 1];
> > +
> > if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (p - obj->programs) + i;
> > + idx = (p - obj->programs) + (forward ? 1 : -1);
> > if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > @@ -2216,11 +2222,8 @@ 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__iter(prog, obj, 1);
> > + prog = __bpf_program__iter(prog, obj, true);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > @@ -2231,14 +2234,8 @@ 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);
> > + prog = __bpf_program__iter(prog, obj, false);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
> > --
> > 2.17.1
> >
> > >
> > > > > 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;
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2018-11-13 9:37 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 16:21 [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 1/7] selftests/bpf: rename flow dissector section to flow_dissector Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-12 21:53 ` Martin Lau
2018-11-12 21:53 ` Martin Lau
2018-11-12 21:53 ` kafai
2018-11-12 22:10 ` Stanislav Fomichev
2018-11-12 22:10 ` Stanislav Fomichev
2018-11-12 22:10 ` sdf
2018-11-12 23:10 ` Martin Lau
2018-11-12 23:10 ` Martin Lau
2018-11-12 23:10 ` kafai
2018-11-12 23:29 ` Stanislav Fomichev
2018-11-12 23:29 ` Stanislav Fomichev
2018-11-12 23:29 ` sdf
2018-11-12 23:41 ` Martin Lau
2018-11-12 23:41 ` Martin Lau
2018-11-12 23:41 ` kafai
2018-11-09 16:21 ` [PATCH v5 bpf-next 3/7] libbpf: bpf_program__pin: add special case for instances.nr == 1 Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 4/7] libbpf: add internal pin_name Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 5/7] bpftool: add loadall command Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 6/7] bpftool: add pinmaps argument to the load/loadall Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 16:21 ` [PATCH v5 bpf-next 7/7] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-09 16:21 ` Stanislav Fomichev
2018-11-09 16:21 ` sdf
2018-11-09 17:38 ` [PATCH v5 bpf-next 0/7] " Quentin Monnet
2018-11-09 17:38 ` Quentin Monnet
2018-11-09 17:38 ` quentin.monnet
2018-11-09 20:15 ` Jakub Kicinski
2018-11-09 20:15 ` Jakub Kicinski
2018-11-09 20:15 ` jakub.kicinski
2018-11-11 0:00 ` Alexei Starovoitov
2018-11-11 0:00 ` Alexei Starovoitov
2018-11-11 0:00 ` alexei.starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.