2019-12-10 17:06 UTC+0100 ~ Paul Chaignon <paul.chaignon@xxxxxxxxxx>
> This patch implements lookup by name for maps and changes the behavior of
> lookups by tag to be consistent with prog subcommands. Similarly to
> program subcommands, the show and dump commands will return all maps with
> the given name (or tag), whereas other commands will error out if several
> maps have the same name (resp. tag).
>
> When a map has BTF info, it is dumped in JSON with available BTF info.
> This patch requires that all matched maps have BTF info before switching
> the output format to JSON.
>
> Signed-off-by: Paul Chaignon <paul.chaignon@xxxxxxxxxx>
> ---
> .../bpf/bpftool/Documentation/bpftool-map.rst | 10 +-
> tools/bpf/bpftool/bash-completion/bpftool | 131 ++++++-
> tools/bpf/bpftool/main.h | 2 +-
> tools/bpf/bpftool/map.c | 366 +++++++++++++++---
> 4 files changed, 432 insertions(+), 77 deletions(-)
>
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index 05b5be4a6ef9..21c676a1eeb1 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
Nice work on the completion, thanks!
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index de61d73b9030..f0e0be08ba21 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
[...]
> @@ -654,14 +771,42 @@ static int do_show(int argc, char **argv)
> build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
>
> if (argc == 2) {
> - fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> - if (fd < 0)
> + fds = malloc(sizeof(int));
> + if (!fds) {
> + p_err("mem alloc failed");
> return -1;
> + }
> + nb_fds = map_parse_fds(&argc, &argv, fds);
> + if (nb_fds < 1)
> + goto err_free;
> +
> + if (json_output && nb_fds > 1)
> + jsonw_start_array(json_wtr); /* root array */
> + for (i = 0; i < nb_fds; i++) {
> + err = bpf_obj_get_info_by_fd(fds[i], &info, &len);
> + if (err) {
> + p_err("can't get map info: %s",
> + strerror(errno));
> + for (; i < nb_fds; i++)
> + close(fds[i]);
> + goto err_free;
Same remarks as on patch 1, we may want to keep listing the maps even if
we get a failure for one of them?
> + }
>
> - if (json_output)
> - return show_map_close_json(fd, &info);
> - else
> - return show_map_close_plain(fd, &info);
> + if (json_output)
> + show_map_close_json(fds[i], &info);
> + else
> + show_map_close_plain(fds[i], &info);
> +
> + close(fds[i]);
> + }
> + if (json_output && nb_fds > 1)
> + jsonw_end_array(json_wtr); /* root array */
> +
> + return 0;
> +
> +err_free:
> + free(fds);
> + return -1;
> }
>
> if (argc)
The rest of the code looks good to me, thanks a lot for working on this!
Quentin
