Re: [PATCH bpf-next 2/5] bpftool: Do not expose and init hash maps for pinned path in main.c
From: Quentin Monnet <hidden>
Date: 2021-10-23 20:50:59
Also in:
netdev
2021-10-22 17:13 UTC-0700 ~ Andrii Nakryiko [off-list ref]
On Fri, Oct 22, 2021 at 10:16 AM Quentin Monnet [off-list ref] wrote:quoted
BPF programs, maps, and links, can all be listed with their pinned paths by bpftool, when the "-f" option is provided. To do so, bpftool builds hash maps containing all pinned paths for each kind of objects. These three hash maps are always initialised in main.c, and exposed through main.h. There appear to be no particular reason to do so: we can just as well make them static to the files that need them (prog.c, map.c, and link.c respectively), and initialise them only when we want to show objects and the "-f" switch is provided. This may prevent unnecessary memory allocations if the implementation of the hash maps was to change in the future. Signed-off-by: Quentin Monnet <redacted> --- tools/bpf/bpftool/link.c | 9 ++++++++- tools/bpf/bpftool/main.c | 12 ------------ tools/bpf/bpftool/main.h | 3 --- tools/bpf/bpftool/map.c | 9 ++++++++- tools/bpf/bpftool/prog.c | 9 ++++++++- 5 files changed, 24 insertions(+), 18 deletions(-)diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c index 8cc3e36f8cc6..ebf29be747b3 100644 --- a/tools/bpf/bpftool/link.c +++ b/tools/bpf/bpftool/link.c@@ -20,6 +20,8 @@ static const char * const link_type_name[] = { [BPF_LINK_TYPE_NETNS] = "netns", }; +static struct pinned_obj_table link_table; + static int link_parse_fd(int *argc, char ***argv) { int fd;@@ -302,8 +304,10 @@ static int do_show(int argc, char **argv) __u32 id = 0; int err, fd; - if (show_pinned) + if (show_pinned) { + hash_init(link_table.table); build_pinned_obj_table(&link_table, BPF_OBJ_LINK); + } build_obj_refs_table(&refs_table, BPF_OBJ_LINK); if (argc == 2) {@@ -384,6 +388,9 @@ static int do_detach(int argc, char **argv) if (json_output) jsonw_null(json_wtr); + if (show_pinned) + delete_pinned_obj_table(&link_table);Shouldn't this be in do_show rather than do_detach?
Yikes! How did it end up here? Thanks for the catch, sure, I'll fix it.