diff mbox series

[bpf-next,4/5] bpftool: Switch to libbpf's hashmap for programs/maps in BTF listing

Message ID 20211022171647.27885-5-quentin@isovalent.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpftool: Switch to libbpf's hashmap for referencing BPF objects | expand

Checks

Context Check Description
netdev/cover_letter success Series has a cover letter
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 5 maintainers not CCed: john.fastabend@gmail.com yhs@fb.com songliubraving@fb.com kafai@fb.com kpsingh@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 259 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success No static functions without inline keyword in header files
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next success VM_Test

Commit Message

Quentin Monnet Oct. 22, 2021, 5:16 p.m. UTC 
In order to show BPF programs and maps using BTF objects when the latter
are being listed, bpftool creates hash maps to store all relevant items.
This commit is part of a set that transitions from the kernel's hash map
implementation to the one coming with libbpf.

The motivation is to make bpftool less dependent of kernel headers, to
ease the path to a potential out-of-tree mirror, like libbpf has.

This commit focuses on the two hash maps used by bpftool when listing
BTF objects to store references to programs and maps, and convert them
to the libbpf's implementation.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/btf.c  | 126 ++++++++++++++++-----------------------
 tools/bpf/bpftool/main.h |   5 ++
 2 files changed, 57 insertions(+), 74 deletions(-)

Comments

Andrii Nakryiko Oct. 23, 2021, 1:47 a.m. UTC | #1 
On Fri, Oct 22, 2021 at 10:16 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> In order to show BPF programs and maps using BTF objects when the latter
> are being listed, bpftool creates hash maps to store all relevant items.
> This commit is part of a set that transitions from the kernel's hash map
> implementation to the one coming with libbpf.
>
> The motivation is to make bpftool less dependent of kernel headers, to
> ease the path to a potential out-of-tree mirror, like libbpf has.
>
> This commit focuses on the two hash maps used by bpftool when listing
> BTF objects to store references to programs and maps, and convert them
> to the libbpf's implementation.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>  tools/bpf/bpftool/btf.c  | 126 ++++++++++++++++-----------------------
>  tools/bpf/bpftool/main.h |   5 ++
>  2 files changed, 57 insertions(+), 74 deletions(-)
>

[...]

> @@ -741,28 +724,20 @@ build_btf_type_table(struct btf_attach_table *tab, enum bpf_obj_type type,
>                 if (!btf_id)
>                         continue;
>
> -               obj_node = calloc(1, sizeof(*obj_node));
> -               if (!obj_node) {
> -                       p_err("failed to allocate memory: %s", strerror(errno));
> -                       err = -ENOMEM;
> -                       goto err_free;
> -               }
> -
> -               obj_node->obj_id = id;
> -               obj_node->btf_id = btf_id;
> -               hash_add(tab->table, &obj_node->hash, obj_node->btf_id);
> +               hashmap__append(tab, u32_as_hash_field(btf_id),
> +                               u32_as_hash_field(id));

error handling is missing

>         }
>
>         return 0;
>
>  err_free:
> -       delete_btf_table(tab);
> +       hashmap__free(tab);
>         return err;
>  }
>
>  static int
> -build_btf_tables(struct btf_attach_table *btf_prog_table,
> -                struct btf_attach_table *btf_map_table)
> +build_btf_tables(struct hashmap *btf_prog_table,
> +                struct hashmap *btf_map_table)
>  {
>         struct bpf_prog_info prog_info;
>         __u32 prog_len = sizeof(prog_info);
> @@ -778,7 +753,7 @@ build_btf_tables(struct btf_attach_table *btf_prog_table,
>         err = build_btf_type_table(btf_map_table, BPF_OBJ_MAP, &map_info,
>                                    &map_len);
>         if (err) {
> -               delete_btf_table(btf_prog_table);
> +               hashmap__free(btf_prog_table);
>                 return err;
>         }
>
> @@ -787,10 +762,10 @@ build_btf_tables(struct btf_attach_table *btf_prog_table,
>
>  static void
>  show_btf_plain(struct bpf_btf_info *info, int fd,
> -              struct btf_attach_table *btf_prog_table,
> -              struct btf_attach_table *btf_map_table)
> +              struct hashmap *btf_prog_table,
> +              struct hashmap *btf_map_table)
>  {
> -       struct btf_attach_point *obj;
> +       struct hashmap_entry *entry;
>         const char *name = u64_to_ptr(info->name);
>         int n;
>
> @@ -804,18 +779,17 @@ show_btf_plain(struct bpf_btf_info *info, int fd,
>         printf("size %uB", info->btf_size);
>
>         n = 0;
> -       hash_for_each_possible(btf_prog_table->table, obj, hash, info->id) {
> -               if (obj->btf_id == info->id)
> -                       printf("%s%u", n++ == 0 ? "  prog_ids " : ",",
> -                              obj->obj_id);
> -       }
> +       hashmap__for_each_key_entry(btf_prog_table, entry,
> +                                   u32_as_hash_field(info->id))
> +               printf("%s%u", n++ == 0 ? "  prog_ids " : ",",
> +                      hash_field_as_u32(entry->value));

nit: I'd add {}, it's getting a bit hard to follow

>
>         n = 0;
> -       hash_for_each_possible(btf_map_table->table, obj, hash, info->id) {
> -               if (obj->btf_id == info->id)
> -                       printf("%s%u", n++ == 0 ? "  map_ids " : ",",
> -                              obj->obj_id);
> -       }
> +       hashmap__for_each_key_entry(btf_map_table, entry,
> +                                   u32_as_hash_field(info->id))
> +               printf("%s%u", n++ == 0 ? "  map_ids " : ",",
> +                      hash_field_as_u32(entry->value));
> +
>         emit_obj_refs_plain(&refs_table, info->id, "\n\tpids ");
>
>         printf("\n");

[...]
diff mbox series

Patch

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 7b68d4f65fe6..84959aa05265 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -8,14 +8,16 @@ 
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
-#include <bpf/bpf.h>
-#include <bpf/btf.h>
-#include <bpf/libbpf.h>
 #include <linux/btf.h>
 #include <linux/hashtable.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include <bpf/bpf.h>
+#include <bpf/btf.h>
+#include <bpf/hashmap.h>
+#include <bpf/libbpf.h>
+
 #include "json_writer.h"
 #include "main.h"
 
@@ -40,14 +42,9 @@  static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_DECL_TAG]	= "DECL_TAG",
 };
 
-struct btf_attach_table {
-	DECLARE_HASHTABLE(table, 16);
-};
-
 struct btf_attach_point {
 	__u32 obj_id;
 	__u32 btf_id;
-	struct hlist_node hash;
 };
 
 static const char *btf_int_enc_str(__u8 encoding)
@@ -645,21 +642,8 @@  static int btf_parse_fd(int *argc, char ***argv)
 	return fd;
 }
 
-static void delete_btf_table(struct btf_attach_table *tab)
-{
-	struct btf_attach_point *obj;
-	struct hlist_node *tmp;
-
-	unsigned int bkt;
-
-	hash_for_each_safe(tab->table, bkt, tmp, obj, hash) {
-		hash_del(&obj->hash);
-		free(obj);
-	}
-}
-
 static int
-build_btf_type_table(struct btf_attach_table *tab, enum bpf_obj_type type,
+build_btf_type_table(struct hashmap *tab, enum bpf_obj_type type,
 		     void *info, __u32 *len)
 {
 	static const char * const names[] = {
@@ -667,7 +651,6 @@  build_btf_type_table(struct btf_attach_table *tab, enum bpf_obj_type type,
 		[BPF_OBJ_PROG]		= "prog",
 		[BPF_OBJ_MAP]		= "map",
 	};
-	struct btf_attach_point *obj_node;
 	__u32 btf_id, id = 0;
 	int err;
 	int fd;
@@ -741,28 +724,20 @@  build_btf_type_table(struct btf_attach_table *tab, enum bpf_obj_type type,
 		if (!btf_id)
 			continue;
 
-		obj_node = calloc(1, sizeof(*obj_node));
-		if (!obj_node) {
-			p_err("failed to allocate memory: %s", strerror(errno));
-			err = -ENOMEM;
-			goto err_free;
-		}
-
-		obj_node->obj_id = id;
-		obj_node->btf_id = btf_id;
-		hash_add(tab->table, &obj_node->hash, obj_node->btf_id);
+		hashmap__append(tab, u32_as_hash_field(btf_id),
+				u32_as_hash_field(id));
 	}
 
 	return 0;
 
 err_free:
-	delete_btf_table(tab);
+	hashmap__free(tab);
 	return err;
 }
 
 static int
-build_btf_tables(struct btf_attach_table *btf_prog_table,
-		 struct btf_attach_table *btf_map_table)
+build_btf_tables(struct hashmap *btf_prog_table,
+		 struct hashmap *btf_map_table)
 {
 	struct bpf_prog_info prog_info;
 	__u32 prog_len = sizeof(prog_info);
@@ -778,7 +753,7 @@  build_btf_tables(struct btf_attach_table *btf_prog_table,
 	err = build_btf_type_table(btf_map_table, BPF_OBJ_MAP, &map_info,
 				   &map_len);
 	if (err) {
-		delete_btf_table(btf_prog_table);
+		hashmap__free(btf_prog_table);
 		return err;
 	}
 
@@ -787,10 +762,10 @@  build_btf_tables(struct btf_attach_table *btf_prog_table,
 
 static void
 show_btf_plain(struct bpf_btf_info *info, int fd,
-	       struct btf_attach_table *btf_prog_table,
-	       struct btf_attach_table *btf_map_table)
+	       struct hashmap *btf_prog_table,
+	       struct hashmap *btf_map_table)
 {
-	struct btf_attach_point *obj;
+	struct hashmap_entry *entry;
 	const char *name = u64_to_ptr(info->name);
 	int n;
 
@@ -804,18 +779,17 @@  show_btf_plain(struct bpf_btf_info *info, int fd,
 	printf("size %uB", info->btf_size);
 
 	n = 0;
-	hash_for_each_possible(btf_prog_table->table, obj, hash, info->id) {
-		if (obj->btf_id == info->id)
-			printf("%s%u", n++ == 0 ? "  prog_ids " : ",",
-			       obj->obj_id);
-	}
+	hashmap__for_each_key_entry(btf_prog_table, entry,
+				    u32_as_hash_field(info->id))
+		printf("%s%u", n++ == 0 ? "  prog_ids " : ",",
+		       hash_field_as_u32(entry->value));
 
 	n = 0;
-	hash_for_each_possible(btf_map_table->table, obj, hash, info->id) {
-		if (obj->btf_id == info->id)
-			printf("%s%u", n++ == 0 ? "  map_ids " : ",",
-			       obj->obj_id);
-	}
+	hashmap__for_each_key_entry(btf_map_table, entry,
+				    u32_as_hash_field(info->id))
+		printf("%s%u", n++ == 0 ? "  map_ids " : ",",
+		       hash_field_as_u32(entry->value));
+
 	emit_obj_refs_plain(&refs_table, info->id, "\n\tpids ");
 
 	printf("\n");
@@ -823,10 +797,10 @@  show_btf_plain(struct bpf_btf_info *info, int fd,
 
 static void
 show_btf_json(struct bpf_btf_info *info, int fd,
-	      struct btf_attach_table *btf_prog_table,
-	      struct btf_attach_table *btf_map_table)
+	      struct hashmap *btf_prog_table,
+	      struct hashmap *btf_map_table)
 {
-	struct btf_attach_point *obj;
+	struct hashmap_entry *entry;
 	const char *name = u64_to_ptr(info->name);
 
 	jsonw_start_object(json_wtr);	/* btf object */
@@ -835,20 +809,16 @@  show_btf_json(struct bpf_btf_info *info, int fd,
 
 	jsonw_name(json_wtr, "prog_ids");
 	jsonw_start_array(json_wtr);	/* prog_ids */
-	hash_for_each_possible(btf_prog_table->table, obj, hash,
-			       info->id) {
-		if (obj->btf_id == info->id)
-			jsonw_uint(json_wtr, obj->obj_id);
-	}
+	hashmap__for_each_key_entry(btf_prog_table, entry,
+				    u32_as_hash_field(info->id))
+		jsonw_uint(json_wtr, hash_field_as_u32(entry->value));
 	jsonw_end_array(json_wtr);	/* prog_ids */
 
 	jsonw_name(json_wtr, "map_ids");
 	jsonw_start_array(json_wtr);	/* map_ids */
-	hash_for_each_possible(btf_map_table->table, obj, hash,
-			       info->id) {
-		if (obj->btf_id == info->id)
-			jsonw_uint(json_wtr, obj->obj_id);
-	}
+	hashmap__for_each_key_entry(btf_map_table, entry,
+				    u32_as_hash_field(info->id))
+		jsonw_uint(json_wtr, hash_field_as_u32(entry->value));
 	jsonw_end_array(json_wtr);	/* map_ids */
 
 	emit_obj_refs_json(&refs_table, info->id, json_wtr); /* pids */
@@ -862,8 +832,8 @@  show_btf_json(struct bpf_btf_info *info, int fd,
 }
 
 static int
-show_btf(int fd, struct btf_attach_table *btf_prog_table,
-	 struct btf_attach_table *btf_map_table)
+show_btf(int fd, struct hashmap *btf_prog_table,
+	 struct hashmap *btf_map_table)
 {
 	struct bpf_btf_info info;
 	__u32 len = sizeof(info);
@@ -900,8 +870,8 @@  show_btf(int fd, struct btf_attach_table *btf_prog_table,
 
 static int do_show(int argc, char **argv)
 {
-	struct btf_attach_table btf_prog_table;
-	struct btf_attach_table btf_map_table;
+	struct hashmap *btf_prog_table;
+	struct hashmap *btf_map_table;
 	int err, fd = -1;
 	__u32 id = 0;
 
@@ -917,9 +887,17 @@  static int do_show(int argc, char **argv)
 		return BAD_ARG();
 	}
 
-	hash_init(btf_prog_table.table);
-	hash_init(btf_map_table.table);
-	err = build_btf_tables(&btf_prog_table, &btf_map_table);
+	btf_prog_table = hashmap__new(bpftool_hash_fn, bpftool_equal_fn, NULL);
+	btf_map_table = hashmap__new(bpftool_hash_fn, bpftool_equal_fn, NULL);
+	if (!btf_prog_table || !btf_map_table) {
+		hashmap__free(btf_prog_table);
+		hashmap__free(btf_map_table);
+		if (fd >= 0)
+			close(fd);
+		p_err("failed to create hashmap for object references");
+		return -1;
+	}
+	err = build_btf_tables(btf_prog_table, btf_map_table);
 	if (err) {
 		if (fd >= 0)
 			close(fd);
@@ -928,7 +906,7 @@  static int do_show(int argc, char **argv)
 	build_obj_refs_table(&refs_table, BPF_OBJ_BTF);
 
 	if (fd >= 0) {
-		err = show_btf(fd, &btf_prog_table, &btf_map_table);
+		err = show_btf(fd, btf_prog_table, btf_map_table);
 		close(fd);
 		goto exit_free;
 	}
@@ -960,7 +938,7 @@  static int do_show(int argc, char **argv)
 			break;
 		}
 
-		err = show_btf(fd, &btf_prog_table, &btf_map_table);
+		err = show_btf(fd, btf_prog_table, btf_map_table);
 		close(fd);
 		if (err)
 			break;
@@ -970,8 +948,8 @@  static int do_show(int argc, char **argv)
 		jsonw_end_array(json_wtr);	/* root array */
 
 exit_free:
-	delete_btf_table(&btf_prog_table);
-	delete_btf_table(&btf_map_table);
+	hashmap__free(btf_prog_table);
+	hashmap__free(btf_map_table);
 	delete_obj_refs_table(&refs_table);
 
 	return err;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index f61be172d864..a8e71ead848c 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -256,6 +256,11 @@  static inline void *u32_as_hash_field(__u32 x)
 	return (void *)(uintptr_t)x;
 }
 
+static inline __u32 hash_field_as_u32(const void *x)
+{
+	return (__u32)(uintptr_t)x;
+}
+
 static inline bool hashmap__empty(struct hashmap *map)
 {
 	return map ? hashmap__size(map) == 0 : true;