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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4Bzb4Q+w1ziZqo++VLGme1sV=o261d6XVdS9tdKdS7qNVtQ@mail.gmail.com>
Date:   Fri, 22 Oct 2021 18:47:31 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 4/5] bpftool: Switch to libbpf's hashmap for
 programs/maps in BTF listing

On Fri, Oct 22, 2021 at 10:16 AM Quentin Monnet <quentin@...valent.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@...valent.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");

[...]