* [PATCH bpf-next 0/2] tools: bpftool: fix object pinning and bash
@ 2020-03-12 18:25 Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 2/2] tools: bpftool: fix minor bash completion mistakes Quentin Monnet
0 siblings, 2 replies; 4+ messages in thread
From: Quentin Monnet @ 2020-03-12 18:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
The first patch of this series improves user experience by allowing to pass
all kinds of handles for programs and maps (id, tag, name, pinned path)
instead of simply ids when pinning them with "bpftool (prog|map) pin".
The second patch improves or fix bash completion, including for object
pinning.
Quentin Monnet (2):
tools: bpftool: allow all prog/map handles for pinning objects
tools: bpftool: fix minor bash completion mistakes
tools/bpf/bpftool/bash-completion/bpftool | 29 ++++++++++++-----
tools/bpf/bpftool/common.c | 39 +++--------------------
tools/bpf/bpftool/main.h | 2 +-
tools/bpf/bpftool/map.c | 2 +-
tools/bpf/bpftool/prog.c | 2 +-
5 files changed, 29 insertions(+), 45 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects
2020-03-12 18:25 [PATCH bpf-next 0/2] tools: bpftool: fix object pinning and bash Quentin Monnet
@ 2020-03-12 18:25 ` Quentin Monnet
2020-03-12 18:39 ` Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 2/2] tools: bpftool: fix minor bash completion mistakes Quentin Monnet
1 sibling, 1 reply; 4+ messages in thread
From: Quentin Monnet @ 2020-03-12 18:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
Documentation and interactive help for bpftool have always explained
that the regular handles for programs (id|name|tag|pinned) and maps
(id|name|pinned) can be passed to the utility when attempting to pin
objects (bpftool prog pin PROG / bpftool map pin MAP).
THIS IS A LIE!! The tool actually accepts only ids, as the parsing is
done in do_pin_any() in common.c instead of reusing the parsing
functions that have long been generic for program and map handles.
Instead of fixing the doc, fix the code. It is trivial to reuse the
generic parsing, and to simplify do_pin_any() in the process.
Do not accept to pin multiple objects at the same time with
prog_parse_fds() or map_parse_fds() (this would require a more complex
syntax for passing multiple sysfs paths and validating that they
correspond to the number of e.g. programs we find for a given name or
tag).
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/common.c | 39 +++++---------------------------------
tools/bpf/bpftool/main.h | 2 +-
tools/bpf/bpftool/map.c | 2 +-
tools/bpf/bpftool/prog.c | 2 +-
4 files changed, 8 insertions(+), 37 deletions(-)
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index b75b8ec5469c..92e51a62bd72 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -211,44 +211,15 @@ int do_pin_fd(int fd, const char *name)
return err;
}
-int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
+int do_pin_any(int argc, char **argv, int (*get_fd)(int *, char ***))
{
- unsigned int id;
- char *endptr;
- int err;
int fd;
- if (argc < 3) {
- p_err("too few arguments, id ID and FILE path is required");
- return -1;
- } else if (argc > 3) {
- p_err("too many arguments");
- return -1;
- }
-
- if (!is_prefix(*argv, "id")) {
- p_err("expected 'id' got %s", *argv);
- return -1;
- }
- NEXT_ARG();
-
- id = strtoul(*argv, &endptr, 0);
- if (*endptr) {
- p_err("can't parse %s as ID", *argv);
- return -1;
- }
- NEXT_ARG();
-
- fd = get_fd_by_id(id);
- if (fd < 0) {
- p_err("can't open object by id (%u): %s", id, strerror(errno));
- return -1;
- }
-
- err = do_pin_fd(fd, *argv);
+ fd = get_fd(&argc, &argv);
+ if (fd < 0)
+ return fd;
- close(fd);
- return err;
+ return do_pin_fd(fd, *argv);
}
const char *get_fd_type_name(enum bpf_obj_type type)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 724ef9d941d3..d57972dd0f2b 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -146,7 +146,7 @@ char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path, bool quiet);
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_any(int argc, char **argv, int (*get_fd_by_id)(int *, char ***));
int do_pin_fd(int fd, const char *name);
int do_prog(int argc, char **arg);
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e6c85680b34d..693a632f6813 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -1384,7 +1384,7 @@ static int do_pin(int argc, char **argv)
{
int err;
- err = do_pin_any(argc, argv, bpf_map_get_fd_by_id);
+ err = do_pin_any(argc, argv, map_parse_fd);
if (!err && json_output)
jsonw_null(json_wtr);
return err;
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 576ddd82bc96..6e3d69f06b60 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -813,7 +813,7 @@ static int do_pin(int argc, char **argv)
{
int err;
- err = do_pin_any(argc, argv, bpf_prog_get_fd_by_id);
+ err = do_pin_any(argc, argv, prog_parse_fd);
if (!err && json_output)
jsonw_null(json_wtr);
return err;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf-next 2/2] tools: bpftool: fix minor bash completion mistakes
2020-03-12 18:25 [PATCH bpf-next 0/2] tools: bpftool: fix object pinning and bash Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects Quentin Monnet
@ 2020-03-12 18:25 ` Quentin Monnet
1 sibling, 0 replies; 4+ messages in thread
From: Quentin Monnet @ 2020-03-12 18:25 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev, Quentin Monnet
Minor fixes for bash completion: addition of program name completion for
two subcommands, and correction for program test-runs and map pinning.
The completion for the following commands is fixed or improved:
# bpftool prog run [TAB]
# bpftool prog pin [TAB]
# bpftool map pin [TAB]
# bpftool net attach xdp name [TAB]
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/bash-completion/bpftool | 29 ++++++++++++++++-------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a9cce9d3745a..9b0534f558f1 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -542,8 +542,8 @@ _bpftool()
esac
;;
run)
- if [[ ${#words[@]} -lt 5 ]]; then
- _filedir
+ if [[ ${#words[@]} -eq 4 ]]; then
+ COMPREPLY=( $( compgen -W "$PROG_TYPE" -- "$cur" ) )
return 0
fi
case $prev in
@@ -551,6 +551,10 @@ _bpftool()
_bpftool_get_prog_ids
return 0
;;
+ name)
+ _bpftool_get_prog_names
+ return 0
+ ;;
data_in|data_out|ctx_in|ctx_out)
_filedir
return 0
@@ -756,11 +760,17 @@ _bpftool()
esac
;;
pin)
- if [[ $prev == "$command" ]]; then
- COMPREPLY=( $( compgen -W "$PROG_TYPE" -- "$cur" ) )
- else
- _filedir
- fi
+ case $prev in
+ $command)
+ COMPREPLY=( $( compgen -W "$MAP_TYPE" -- "$cur" ) )
+ ;;
+ id)
+ _bpftool_get_map_ids
+ ;;
+ name)
+ _bpftool_get_map_names
+ ;;
+ esac
return 0
;;
event_pipe)
@@ -887,7 +897,7 @@ _bpftool()
case $command in
skeleton)
_filedir
- ;;
+ ;;
*)
[[ $prev == $object ]] && \
COMPREPLY=( $( compgen -W 'skeleton help' -- "$cur" ) )
@@ -987,6 +997,9 @@ _bpftool()
id)
_bpftool_get_prog_ids
;;
+ name)
+ _bpftool_get_prog_names
+ ;;
pinned)
_filedir
;;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects
2020-03-12 18:25 ` [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects Quentin Monnet
@ 2020-03-12 18:39 ` Quentin Monnet
0 siblings, 0 replies; 4+ messages in thread
From: Quentin Monnet @ 2020-03-12 18:39 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf, netdev
2020-03-12 18:25 UTC+0000 ~ Quentin Monnet <quentin@isovalent.com>
> Documentation and interactive help for bpftool have always explained
> that the regular handles for programs (id|name|tag|pinned) and maps
> (id|name|pinned) can be passed to the utility when attempting to pin
> objects (bpftool prog pin PROG / bpftool map pin MAP).
>
> THIS IS A LIE!! The tool actually accepts only ids, as the parsing is
> done in do_pin_any() in common.c instead of reusing the parsing
> functions that have long been generic for program and map handles.
>
> Instead of fixing the doc, fix the code. It is trivial to reuse the
> generic parsing, and to simplify do_pin_any() in the process.
>
> Do not accept to pin multiple objects at the same time with
> prog_parse_fds() or map_parse_fds() (this would require a more complex
> syntax for passing multiple sysfs paths and validating that they
> correspond to the number of e.g. programs we find for a given name or
> tag).
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
> tools/bpf/bpftool/common.c | 39 +++++---------------------------------
> tools/bpf/bpftool/main.h | 2 +-
> tools/bpf/bpftool/map.c | 2 +-
> tools/bpf/bpftool/prog.c | 2 +-
> 4 files changed, 8 insertions(+), 37 deletions(-)
>
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index b75b8ec5469c..92e51a62bd72 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -211,44 +211,15 @@ int do_pin_fd(int fd, const char *name)
> return err;
> }
>
> -int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
> +int do_pin_any(int argc, char **argv, int (*get_fd)(int *, char ***))
> {
> - unsigned int id;
> - char *endptr;
> - int err;
> int fd;
>
> - if (argc < 3) {
> - p_err("too few arguments, id ID and FILE path is required");
> - return -1;
> - } else if (argc > 3) {
> - p_err("too many arguments");
> - return -1;
> - }
> -
> - if (!is_prefix(*argv, "id")) {
> - p_err("expected 'id' got %s", *argv);
> - return -1;
> - }
> - NEXT_ARG();
> -
> - id = strtoul(*argv, &endptr, 0);
> - if (*endptr) {
> - p_err("can't parse %s as ID", *argv);
> - return -1;
> - }
> - NEXT_ARG();
> -
> - fd = get_fd_by_id(id);
> - if (fd < 0) {
> - p_err("can't open object by id (%u): %s", id, strerror(errno));
> - return -1;
> - }
> -
> - err = do_pin_fd(fd, *argv);
> + fd = get_fd(&argc, &argv);
> + if (fd < 0)
> + return fd;
>
> - close(fd);
> - return err;
> + return do_pin_fd(fd, *argv);
Looks like someone trimmed too much and forgot to close his fd. Will
send v2.
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-12 18:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 18:25 [PATCH bpf-next 0/2] tools: bpftool: fix object pinning and bash Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 1/2] tools: bpftool: allow all prog/map handles for pinning objects Quentin Monnet
2020-03-12 18:39 ` Quentin Monnet
2020-03-12 18:25 ` [PATCH bpf-next 2/2] tools: bpftool: fix minor bash completion mistakes Quentin Monnet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).