On 4/28/20 2:27 AM, Quentin Monnet wrote:
2020-04-27 13:12 UTC-0700 ~ Yonghong Song <[email protected]>
Currently, only one command is supported
bpftool iter pin <bpf_prog.o> <path>
It will pin the trace/iter bpf program in
the object file <bpf_prog.o> to the <path>
where <path> should be on a bpffs mount.
For example,
$ bpftool iter pin ./bpf_iter_ipv6_route.o \
/sys/fs/bpf/my_route
User can then do a `cat` to print out the results:
$ cat /sys/fs/bpf/my_route
fe800000000000000000000000000000 40 00000000000000000000000000000000 ...
00000000000000000000000000000000 00 00000000000000000000000000000000 ...
00000000000000000000000000000001 80 00000000000000000000000000000000 ...
fe800000000000008c0162fffebdfd57 80 00000000000000000000000000000000 ...
ff000000000000000000000000000000 08 00000000000000000000000000000000 ...
00000000000000000000000000000000 00 00000000000000000000000000000000 ...
The implementation for ipv6_route iterator is in one of subsequent
patches.
Signed-off-by: Yonghong Song <[email protected]>
---
.../bpftool/Documentation/bpftool-iter.rst | 71 ++++++++++++++++
tools/bpf/bpftool/bash-completion/bpftool | 13 +++
tools/bpf/bpftool/iter.c | 84 +++++++++++++++++++
tools/bpf/bpftool/main.c | 3 +-
tools/bpf/bpftool/main.h | 1 +
5 files changed, 171 insertions(+), 1 deletion(-)
create mode 100644 tools/bpf/bpftool/Documentation/bpftool-iter.rst
create mode 100644 tools/bpf/bpftool/iter.c
diff --git a/tools/bpf/bpftool/Documentation/bpftool-iter.rst
b/tools/bpf/bpftool/Documentation/bpftool-iter.rst
new file mode 100644
index 000000000000..1997a6bac4a0
--- /dev/null
+++ b/tools/bpf/bpftool/Documentation/bpftool-iter.rst
@@ -0,0 +1,71 @@
+============
+bpftool-iter
+============
+-------------------------------------------------------------------------------
+tool to create BPF iterators
+-------------------------------------------------------------------------------
+
+:Manual section: 8
+
+SYNOPSIS
+========
+
+ **bpftool** [*OPTIONS*] **iter** *COMMAND*
+
+ *COMMANDS* := { **pin** | **help** }
+
+STRUCT_OPS COMMANDS
s/STRUCT_OPS/ITER/
Oops. copy-paste error. Will fix.
+===================
+
+| **bpftool** **iter pin** *OBJ* *PATH*
+| **bpftool** **struct_ops help**
s/struct_ops/iter/
Will fix.
+|
+| *OBJ* := /a/file/of/bpf_iter_target.o
+
+
+DESCRIPTION
+===========
+ **bpftool iter pin** *OBJ* *PATH*
Would be great to have a small blurb on what BPF iterators are and what
they can do. I'm afraid users reading this man page will have no idea
whatsoever.
Will add.
+ Create a bpf iterator from *OBJ*, and pin it to
+ *PATH*. The *PATH* should be located in *bpffs* mount.
Can you keep the note that other pages have about the dot character
being forbidden in *PATH* basename, please?
Will add.
+
+ **bpftool struct_ops help**
s/struct_ops/iter/
Will fix.
+ Print short help message.
+
+OPTIONS
+=======
+ -h, --help
+ Print short generic help message (similar to **bpftool
help**).
+
+ -V, --version
+ Print version number (similar to **bpftool version**).
+
+ -d, --debug
+ Print all logs available, even debug-level information. This
+ includes logs from libbpf as well as from the verifier, when
+ attempting to load programs.> +
+EXAMPLES
+========
+**# bpftool iter pin bpf_iter_netlink.o /sys/fs/bpf/my_netlink**
+
+::
+
+ Create a file-based bpf iterator from bpf_iter_netlink.o and pin it
+ to /sys/fs/bpf/my_netlink
+
+
+SEE ALSO
+========
+ **bpf**\ (2),
+ **bpf-helpers**\ (7),
+ **bpftool**\ (8),
+ **bpftool-prog**\ (8),
+ **bpftool-map**\ (8),
+ **bpftool-cgroup**\ (8),
+ **bpftool-feature**\ (8),
+ **bpftool-net**\ (8),
+ **bpftool-perf**\ (8),
+ **bpftool-btf**\ (8)
+ **bpftool-gen**\ (8)
+ **bpftool-struct_ops**\ (8)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool
b/tools/bpf/bpftool/bash-completion/bpftool
index 45ee99b159e2..17a81695da0f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -604,6 +604,19 @@ _bpftool()
;;
esac
;;
+ iter)
+ case $command in
+ pin)
+ _filedir
+ return 0
+ ;;
+ *)
+ [[ $prev == $object ]] && \
+ COMPREPLY=( $( compgen -W 'help' \
+ -- "$cur" ) )
You should probably offer "pin" here in addition to "help".
Will add.
+ ;;
+ esac
+ ;;
map)
local MAP_TYPE='id pinned name'
case $command in
diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c
new file mode 100644
index 000000000000..db9fae6be716
--- /dev/null
+++ b/tools/bpf/bpftool/iter.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+// Copyright (C) 2020 Facebook
+
+#define _GNU_SOURCE
+#include <linux/err.h>
+#include <bpf/libbpf.h>
+
+#include "main.h"
+
+static int do_pin(int argc, char **argv)
+{
+ const char *objfile, *path;
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ struct bpf_link *link;
+ int err;
Nit: initialise err t0 -1 do you don't have to set it three times below?
Double checked cmd_select() handling the return value:
0 : success
non-0 : failure
Looking like I can remove two `err = -1` below.
+
+ if (!REQ_ARGS(2))
+ usage();
+
+ objfile = GET_ARG();
+ path = GET_ARG();
+
+ obj = bpf_object__open(objfile);
+ if (IS_ERR_OR_NULL(obj)) {
+ p_err("can't open objfile %s", objfile);
+ return -1;
+ }
+
+ err = bpf_object__load(obj);
+ if (err < 0) {
+ err = -1;
+ p_err("can't load objfile %s", objfile);
+ goto close_obj;
+ }
+
+ prog = bpf_program__next(NULL, obj);
+ link = bpf_program__attach_iter(prog, NULL);
+ if (IS_ERR(link)) {
+ err = -1;
+ p_err("attach_iter failed for program %s",
+ bpf_program__name(prog));
+ goto close_obj;
+ }
+
+ err = bpf_link__pin(link, path);
Try to mount bpffs before that if "-n" is not passed? You could even
call do_pin_any() from common.c by passing bpf_link__fd().
You probably means do_pin_fd()? That is a good suggestion, will use it
in the next revision.
+ if (err) {
+ err = -1;
+ p_err("pin_iter failed for program %s to path %s",
+ bpf_program__name(prog), path);
+ goto close_link;
+ }
+
+ err = 0;
+
+close_link:
+ bpf_link__disconnect(link);
+ bpf_link__destroy(link);
+close_obj:
+ bpf_object__close(obj);
+ return err;
+}
+
+static int do_help(int argc, char **argv)
+{
+ fprintf(stderr,
+ "Usage: %s %s pin OBJ PATH\n"
+ " %s %s help\n"
+ "\n",
+ bin_name, argv[-2], bin_name, argv[-2]);
+
+ return 0;
+}
+
+static const struct cmd cmds[] = {
+ { "help", do_help },
+ { "pin", do_pin },
+ { 0 }
+};
+
+int do_iter(int argc, char **argv)
+{
+ return cmd_select(cmds, argc, argv, do_help);
+}
dif "",
bin_name, bin_name, bin_name);
@@ -222,6 +222,7 @@ static const struct cmd cmds[] = {
{ "btf", do_btf },
{ "gen", do_gen },
{ "struct_ops", do_struct_ops },
+ { "iter", do_iter },
{ "version", do_version },
{ 0 }
};
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 86f14ce26fd7..2b5d4a616b48 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -162,6 +162,7 @@ int do_feature(int argc, char **argv);
int do_btf(int argc, char **argv);
int do_gen(int argc, char **argv);
int do_struct_ops(int argc, char **argv);
+int do_iter(int argc, char **argv);
int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
int prog_parse_fd(int *argc, char ***argv);
f --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 466c269eabdd..6805b77789cb 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -58,7 +58,7 @@ static int do_help(int argc, char **argv)
" %s batch file FILE\n"
" %s version\n"
"\n"
- " OBJECT := { prog | map | cgroup | perf | net | feature | btf
| gen | struct_ops }\n"
+ " OBJECT := { prog | map | cgroup | perf | net | feature | btf
| gen | struct_ops | iter }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, bin_name, bin_name);
@@ -222,6 +222,7 @@ static const struct cmd cmds[] = {
{ "btf", do_btf },
{ "gen", do_gen },
{ "struct_ops", do_struct_ops },
+ { "iter", do_iter },
{ "version", do_version },
{ 0 }
};
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 86f14ce26fd7..2b5d4a616b48 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -162,6 +162,7 @@ int do_feature(int argc, char **argv);
int do_btf(int argc, char **argv);
int do_gen(int argc, char **argv);
int do_struct_ops(int argc, char **argv);
+int do_iter(int argc, char **argv);
int parse_u32_arg(int *argc, char ***argv, __u32 *val, const char *what);
int prog_parse_fd(int *argc, char ***argv);
Have you considered simply adapting the more traditional workflow
"bpftool prog load && bpftool prog attach" so that it supports iterators
instead of adding a new command? It would:
This is a good question, I should have clarified better in the commit
message.
- prog load && prog attach won't work.
the create_iter is a three stage process:
1. prog load
2. create and attach to a link
3. pin link
In the current implementation, the link merely just has the program.
But in the future, the link will have other parameters like map_id,
tgid/gid, or cgroup_id, or others.
We could say to do:
1. bpftool prog load <pin_path>
2. bpftool iter pin prog file
<maybe more parameters in the future>
But this requires to pin the program itself in the bpffs, which
mostly unneeded for file iterator creator.
So this command `bpftool iter pin ...` is created for ease of use.
- Avoid adding yet another bpftool command with a single subcommand
So far, yes, in the future we may have more. In my RFC patcch, I have
`bpftool iter show ...` for introspection, this is to show all
registered targets and all file iterators prog_id's.
This patch does not have it and I left it for the future work.
I am considering to use bpf iterator to do introspection here...
- Enable to reuse the code from prog load, in particular for map reuse
(I'm not sure how relevant maps are for iterators, but I wouldn't be
surprised if someone finds a use case at some point?)
Yes, we do plan to have map element iterators. We can also have
bpf_prog or other iterators. Yes, map element iterator use
implementation should be `bpftool map` code base since it is
a use of bpf_iter infrastructure.
- Avoid users naively trying to run "bpftool prog load && bpftool prog
attach <prog> iter" and not understanding why it fails
`bpftool prog attach <prog> [map_id]` mostly used to attach a program to
a map, right? In this case, it won't apply, right?
BTW, Thanks for reviewing and catching my mistakes!
Quentin
