| Message ID | 20200713161749.3077526-1-yhs@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: implement bpf iterator for map elements | expand |
2020-07-13 09:17 UTC-0700 ~ Yonghong Song <yhs@fb.com> > The optional parameter "map MAP" can be added to "bpftool iter" > command to create a bpf iterator for map elements. For example, > bpftool iter pin ./prog.o /sys/fs/bpf/p1 map id 333 > > For map element bpf iterator "map MAP" parameter is required. > Otherwise, bpf link creation will return an error. > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > .../bpftool/Documentation/bpftool-iter.rst | 16 ++++++++-- > tools/bpf/bpftool/iter.c | 32 ++++++++++++++++--- > 2 files changed, 42 insertions(+), 6 deletions(-) > > diff --git a/tools/bpf/bpftool/Documentation/bpftool-iter.rst b/tools/bpf/bpftool/Documentation/bpftool-iter.rst > index 8dce698eab79..53ee4fb188b4 100644 > --- a/tools/bpf/bpftool/Documentation/bpftool-iter.rst > +++ b/tools/bpf/bpftool/Documentation/bpftool-iter.rst > @@ -17,14 +17,15 @@ SYNOPSIS > ITER COMMANDS > =================== > > -| **bpftool** **iter pin** *OBJ* *PATH* > +| **bpftool** **iter pin** *OBJ* *PATH* [**map** *MAP*] > | **bpftool** **iter help** > | > | *OBJ* := /a/file/of/bpf_iter_target.o > +| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* } Please don't change the indentation style (other lines have a tab). > > DESCRIPTION > =========== > - **bpftool iter pin** *OBJ* *PATH* > + **bpftool iter pin** *OBJ* *PATH* [**map** *MAP*] > A bpf iterator combines a kernel iterating of > particular kernel data (e.g., tasks, bpf_maps, etc.) > and a bpf program called for each kernel data object > @@ -37,6 +38,10 @@ DESCRIPTION > character ('.'), which is reserved for future extensions > of *bpffs*. > > + Map element bpf iterator requires an additional parameter > + *MAP* so bpf program can iterate over map elements for > + that map. > + Same note on indentation. Could you please also explain in a few words what the "Map element bpf iterator" is? Reusing part of your cover letter (see below) could do, it's just so that users not familiar with the concept can get an idea of what it does. --- User can have a bpf program in kernel to run with each map element, do checking, filtering, aggregation, etc. without copying data to user space. --- > User can then *cat PATH* to see the bpf iterator output. > > **bpftool iter help** [...] > @@ -62,13 +83,16 @@ static int do_pin(int argc, char **argv) > bpf_link__destroy(link); > close_obj: > bpf_object__close(obj); > +close_map_fd: > + if (map_fd >= 0) > + close(map_fd); > return err; > } > > static int do_help(int argc, char **argv) > { > fprintf(stderr, > - "Usage: %1$s %2$s pin OBJ PATH\n" > + "Usage: %1$s %2$s pin OBJ PATH [map MAP]\n" You probably want to add HELP_SPEC_MAP (as in map.c) to tell the user what MAP should be. Could you please also update the bash completion? Thanks, Quentin
On 7/16/20 9:39 AM, Quentin Monnet wrote: > 2020-07-13 09:17 UTC-0700 ~ Yonghong Song <yhs@fb.com> >> The optional parameter "map MAP" can be added to "bpftool iter" >> command to create a bpf iterator for map elements. For example, >> bpftool iter pin ./prog.o /sys/fs/bpf/p1 map id 333 >> >> For map element bpf iterator "map MAP" parameter is required. >> Otherwise, bpf link creation will return an error. >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> .../bpftool/Documentation/bpftool-iter.rst | 16 ++++++++-- >> tools/bpf/bpftool/iter.c | 32 ++++++++++++++++--- >> 2 files changed, 42 insertions(+), 6 deletions(-) >> >> diff --git a/tools/bpf/bpftool/Documentation/bpftool-iter.rst b/tools/bpf/bpftool/Documentation/bpftool-iter.rst >> index 8dce698eab79..53ee4fb188b4 100644 >> --- a/tools/bpf/bpftool/Documentation/bpftool-iter.rst >> +++ b/tools/bpf/bpftool/Documentation/bpftool-iter.rst >> @@ -17,14 +17,15 @@ SYNOPSIS >> ITER COMMANDS >> =================== >> >> -| **bpftool** **iter pin** *OBJ* *PATH* >> +| **bpftool** **iter pin** *OBJ* *PATH* [**map** *MAP*] >> | **bpftool** **iter help** >> | >> | *OBJ* := /a/file/of/bpf_iter_target.o >> +| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* } > > Please don't change the indentation style (other lines have a tab). Will fix. > >> >> DESCRIPTION >> =========== >> - **bpftool iter pin** *OBJ* *PATH* >> + **bpftool iter pin** *OBJ* *PATH* [**map** *MAP*] >> A bpf iterator combines a kernel iterating of >> particular kernel data (e.g., tasks, bpf_maps, etc.) >> and a bpf program called for each kernel data object >> @@ -37,6 +38,10 @@ DESCRIPTION >> character ('.'), which is reserved for future extensions >> of *bpffs*. >> >> + Map element bpf iterator requires an additional parameter >> + *MAP* so bpf program can iterate over map elements for >> + that map. >> + > > Same note on indentation. > > Could you please also explain in a few words what the "Map element bpf > iterator" is? Reusing part of your cover letter (see below) could do, > it's just so that users not familiar with the concept can get an idea of > what it does. Will do. > > --- > User can have a bpf program in kernel to run with each map element, > do checking, filtering, aggregation, etc. without copying data > to user space. > --- > >> User can then *cat PATH* to see the bpf iterator output. >> >> **bpftool iter help** > > [...] > >> @@ -62,13 +83,16 @@ static int do_pin(int argc, char **argv) >> bpf_link__destroy(link); >> close_obj: >> bpf_object__close(obj); >> +close_map_fd: >> + if (map_fd >= 0) >> + close(map_fd); >> return err; >> } >> >> static int do_help(int argc, char **argv) >> { >> fprintf(stderr, >> - "Usage: %1$s %2$s pin OBJ PATH\n" >> + "Usage: %1$s %2$s pin OBJ PATH [map MAP]\n" > > You probably want to add HELP_SPEC_MAP (as in map.c) to tell the user > what MAP should be. Good suggestion. > > Could you please also update the bash completion? This is always my hardest part! In this case it is bpftool iter pin <filedir> <filedir> [map MAP] Any particular existing bpftool implementation I can imitate? > > Thanks, > Quentin >
2020-07-16 10:42 UTC-0700 ~ Yonghong Song <yhs@fb.com> > > > On 7/16/20 9:39 AM, Quentin Monnet wrote: >> 2020-07-13 09:17 UTC-0700 ~ Yonghong Song <yhs@fb.com> [...] >> Could you please also update the bash completion? > > This is always my hardest part! In this case it is > bpftool iter pin <filedir> <filedir> [map MAP] > > Any particular existing bpftool implementation I can imitate? I would say the closest/easiest to reuse we have would be completion for the MAP part in either bpftool prog attach PROG ATTACH_TYPE [MAP] or bpftool map pin MAP FILE But I'll save you some time, I gave it a go and this is what I came up with: ------ diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index 25b25aca1112..6640e18096a8 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -613,9 +613,26 @@ _bpftool() esac ;; iter) + local MAP_TYPE='id pinned name' case $command in pin) - _filedir + case $prev in + $command) + _filedir + ;; + id) + _bpftool_get_map_ids + ;; + name) + _bpftool_get_map_names + ;; + pinned) + _filedir + ;; + *) + _bpftool_one_of_list $MAP_TYPE + ;; + esac return 0 ;; *) ------ So if we complete "bpftool iter pin", if we're right after "pin" we still complete with file names (for the object file to pin). If we're after one of the map keywords (id|name|pinned), complete with map ids or map names or file names, depending on the case. For other cases (i.e. after object file to pin), offer the map keywords (id|name|pinned). Feel free to reuse. Best, Quentin
On 7/17/20 5:57 AM, Quentin Monnet wrote: > 2020-07-16 10:42 UTC-0700 ~ Yonghong Song <yhs@fb.com> >> >> >> On 7/16/20 9:39 AM, Quentin Monnet wrote: >>> 2020-07-13 09:17 UTC-0700 ~ Yonghong Song <yhs@fb.com> > > [...] > >>> Could you please also update the bash completion? >> >> This is always my hardest part! In this case it is >> bpftool iter pin <filedir> <filedir> [map MAP] >> >> Any particular existing bpftool implementation I can imitate? > > I would say the closest/easiest to reuse we have would be > completion for the MAP part in either > > bpftool prog attach PROG ATTACH_TYPE [MAP] > > or > > bpftool map pin MAP FILE > > But I'll save you some time, I gave it a go and this is what > I came up with: > > ------ > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool > index 25b25aca1112..6640e18096a8 100644 > --- a/tools/bpf/bpftool/bash-completion/bpftool > +++ b/tools/bpf/bpftool/bash-completion/bpftool > @@ -613,9 +613,26 @@ _bpftool() > esac > ;; > iter) > + local MAP_TYPE='id pinned name' > case $command in > pin) > - _filedir > + case $prev in > + $command) > + _filedir > + ;; > + id) > + _bpftool_get_map_ids > + ;; > + name) > + _bpftool_get_map_names > + ;; > + pinned) > + _filedir > + ;; > + *) > + _bpftool_one_of_list $MAP_TYPE > + ;; > + esac > return 0 > ;; > *) > ------ > > So if we complete "bpftool iter pin", if we're right after "pin" > we still complete with file names (for the object file to pin). > If we're after one of the map keywords (id|name|pinned), complete > with map ids or map names or file names, depending on the case. > For other cases (i.e. after object file to pin), offer the map > keywords (id|name|pinned). > > Feel free to reuse. Thanks a lot! Will reuse and mention your suggestion of this code in commit message. > > Best, > Quentin >
diff --git a/tools/bpf/bpftool/Documentation/bpftool-iter.rst b/tools/bpf/bpftool/Documentation/bpftool-iter.rst index 8dce698eab79..53ee4fb188b4 100644 --- a/tools/bpf/bpftool/Documentation/bpftool-iter.rst +++ b/tools/bpf/bpftool/Documentation/bpftool-iter.rst @@ -17,14 +17,15 @@ SYNOPSIS ITER COMMANDS =================== -| **bpftool** **iter pin** *OBJ* *PATH* +| **bpftool** **iter pin** *OBJ* *PATH* [**map** *MAP*] | **bpftool** **iter help** | | *OBJ* := /a/file/of/bpf_iter_target.o +| *MAP* := { **id** *MAP_ID* | **pinned** *FILE* } DESCRIPTION =========== - **bpftool iter pin** *OBJ* *PATH* + **bpftool iter pin** *OBJ* *PATH* [**map** *MAP*] A bpf iterator combines a kernel iterating of particular kernel data (e.g., tasks, bpf_maps, etc.) and a bpf program called for each kernel data object @@ -37,6 +38,10 @@ DESCRIPTION character ('.'), which is reserved for future extensions of *bpffs*. + Map element bpf iterator requires an additional parameter + *MAP* so bpf program can iterate over map elements for + that map. + User can then *cat PATH* to see the bpf iterator output. **bpftool iter help** @@ -64,6 +69,13 @@ EXAMPLES Create a file-based bpf iterator from bpf_iter_netlink.o and pin it to /sys/fs/bpf/my_netlink +**# bpftool iter pin bpf_iter_hashmap.o /sys/fs/bpf/my_hashmap map id 20** + +:: + + Create a file-based bpf iterator from bpf_iter_hashmap.o and map with + id 20, and pin it to /sys/fs/bpf/my_hashmap + SEE ALSO ======== **bpf**\ (2), diff --git a/tools/bpf/bpftool/iter.c b/tools/bpf/bpftool/iter.c index 33240fcc6319..cc1d9bdf6e9d 100644 --- a/tools/bpf/bpftool/iter.c +++ b/tools/bpf/bpftool/iter.c @@ -2,6 +2,7 @@ // Copyright (C) 2020 Facebook #define _GNU_SOURCE +#include <unistd.h> #include <linux/err.h> #include <bpf/libbpf.h> @@ -9,11 +10,12 @@ static int do_pin(int argc, char **argv) { + DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, iter_opts); const char *objfile, *path; struct bpf_program *prog; struct bpf_object *obj; struct bpf_link *link; - int err; + int err = -1, map_fd = -1; if (!REQ_ARGS(2)) usage(); @@ -21,10 +23,26 @@ static int do_pin(int argc, char **argv) objfile = GET_ARG(); path = GET_ARG(); + /* optional arguments */ + if (argc) { + if (is_prefix(*argv, "map")) { + NEXT_ARG(); + + if (!REQ_ARGS(2)) { + p_err("incorrect map spec"); + return -1; + } + + map_fd = map_parse_fd(&argc, &argv); + if (map_fd < 0) + return -1; + } + } + obj = bpf_object__open(objfile); if (IS_ERR(obj)) { p_err("can't open objfile %s", objfile); - return -1; + goto close_map_fd; } err = bpf_object__load(obj); @@ -39,7 +57,10 @@ static int do_pin(int argc, char **argv) goto close_obj; } - link = bpf_program__attach_iter(prog, NULL); + if (map_fd >= 0) + iter_opts.map_fd = map_fd; + + link = bpf_program__attach_iter(prog, &iter_opts); if (IS_ERR(link)) { err = PTR_ERR(link); p_err("attach_iter failed for program %s", @@ -62,13 +83,16 @@ static int do_pin(int argc, char **argv) bpf_link__destroy(link); close_obj: bpf_object__close(obj); +close_map_fd: + if (map_fd >= 0) + close(map_fd); return err; } static int do_help(int argc, char **argv) { fprintf(stderr, - "Usage: %1$s %2$s pin OBJ PATH\n" + "Usage: %1$s %2$s pin OBJ PATH [map MAP]\n" " %1$s %2$s help\n" "", bin_name, "iter");
The optional parameter "map MAP" can be added to "bpftool iter" command to create a bpf iterator for map elements. For example, bpftool iter pin ./prog.o /sys/fs/bpf/p1 map id 333 For map element bpf iterator "map MAP" parameter is required. Otherwise, bpf link creation will return an error. Signed-off-by: Yonghong Song <yhs@fb.com> --- .../bpftool/Documentation/bpftool-iter.rst | 16 ++++++++-- tools/bpf/bpftool/iter.c | 32 ++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-)