linux-kernel - Re: [RFC PATCH v4 1/7] libbpf: Extract BTF type remapping logic into helper function

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEf4BzaPDKJvQtCss4Gm1073wyBGXmixv4s9V5twnF7uEHRhPg@mail.gmail.com>
Date: Tue, 4 Nov 2025 16:11:14 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Donglin Peng <dolinux.peng@...il.com>
Cc: ast@...nel.org, linux-kernel@...r.kernel.org, bpf@...r.kernel.org, 
	Eduard Zingerman <eddyz87@...il.com>, Alan Maguire <alan.maguire@...cle.com>, Song Liu <song@...nel.org>, 
	pengdonglin <pengdonglin@...omi.com>
Subject: Re: [RFC PATCH v4 1/7] libbpf: Extract BTF type remapping logic into
 helper function

On Tue, Nov 4, 2025 at 5:40 AM Donglin Peng <dolinux.peng@...il.com> wrote:
>
> From: pengdonglin <pengdonglin@...omi.com>
>
> Refactor btf_dedup_remap_types() by extracting its core logic into a new
> btf_remap_types() helper function. This eliminates code duplication
> and improves modularity while maintaining the same functionality.
>
> The new function encapsulates iteration over BTF types and BTF ext
> sections, accepting a callback for flexible type ID remapping. This
> makes the type remapping logic more maintainable and reusable.
>
> Cc: Eduard Zingerman <eddyz87@...il.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Andrii Nakryiko <andrii.nakryiko@...il.com>
> Cc: Alan Maguire <alan.maguire@...cle.com>
> Cc: Song Liu <song@...nel.org>
> Signed-off-by: pengdonglin <pengdonglin@...omi.com>

Signed-off-by is supposed to have properly spelled and capitalized
real name of the contributor

> Signed-off-by: Donglin Peng <dolinux.peng@...il.com>
> ---
>  tools/lib/bpf/btf.c             | 63 +++++++++++++++++----------------
>  tools/lib/bpf/libbpf_internal.h |  1 +
>  2 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 18907f0fcf9f..5e1c09b5dce8 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -3400,6 +3400,37 @@ int btf_ext__set_endianness(struct btf_ext *btf_ext, enum btf_endianness endian)
>         return 0;
>  }
>
> +static int btf_remap_types(struct btf *btf, struct btf_ext *btf_ext,
> +                          btf_remap_type_fn visit, void *ctx)

tbh, my goal is to reduce the amount of callback usage within libbpf,
not add more of it...

I don't like this refactoring. We should convert
btf_ext_visit_type_ids() into iterators, have btf_field_iter_init +
btf_field_iter_next usable in for_each() form, and not try to reuse 5
lines of code. See my comments in the next patch.



> +{
> +       int i, r;
> +
> +       for (i = 0; i < btf->nr_types; i++) {
> +               struct btf_type *t = btf_type_by_id(btf, btf->start_id + i);
> +               struct btf_field_iter it;
> +               __u32 *type_id;
> +
> +               r = btf_field_iter_init(&it, t, BTF_FIELD_ITER_IDS);
> +               if (r)
> +                       return r;
> +
> +               while ((type_id = btf_field_iter_next(&it))) {
> +                       r = visit(type_id, ctx);
> +                       if (r)
> +                               return r;
> +               }
> +       }
> +
> +       if (!btf_ext)
> +               return 0;
> +
> +       r = btf_ext_visit_type_ids(btf_ext, visit, ctx);
> +       if (r)
> +               return r;
> +
> +       return 0;
> +}
> +
>  struct btf_dedup;
>
>  static struct btf_dedup *btf_dedup_new(struct btf *btf, const struct btf_dedup_opts *opts);
> @@ -5320,37 +5351,7 @@ static int btf_dedup_remap_type_id(__u32 *type_id, void *ctx)
>   */
>  static int btf_dedup_remap_types(struct btf_dedup *d)
>  {
> -       int i, r;
> -
> -       for (i = 0; i < d->btf->nr_types; i++) {
> -               struct btf_type *t = btf_type_by_id(d->btf, d->btf->start_id + i);
> -               struct btf_field_iter it;
> -               __u32 *type_id;
> -
> -               r = btf_field_iter_init(&it, t, BTF_FIELD_ITER_IDS);
> -               if (r)
> -                       return r;
> -
> -               while ((type_id = btf_field_iter_next(&it))) {
> -                       __u32 resolved_id, new_id;
> -
> -                       resolved_id = resolve_type_id(d, *type_id);
> -                       new_id = d->hypot_map[resolved_id];
> -                       if (new_id > BTF_MAX_NR_TYPES)
> -                               return -EINVAL;
> -
> -                       *type_id = new_id;
> -               }
> -       }
> -
> -       if (!d->btf_ext)
> -               return 0;
> -
> -       r = btf_ext_visit_type_ids(d->btf_ext, btf_dedup_remap_type_id, d);
> -       if (r)
> -               return r;
> -
> -       return 0;
> +       return btf_remap_types(d->btf, d->btf_ext, btf_dedup_remap_type_id, d);
>  }
>
>  /*
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 35b2527bedec..b09d6163f5c3 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -582,6 +582,7 @@ int btf_ext_visit_type_ids(struct btf_ext *btf_ext, type_id_visit_fn visit, void
>  int btf_ext_visit_str_offs(struct btf_ext *btf_ext, str_off_visit_fn visit, void *ctx);
>  __s32 btf__find_by_name_kind_own(const struct btf *btf, const char *type_name,
>                                  __u32 kind);
> +typedef int (*btf_remap_type_fn)(__u32 *type_id, void *ctx);
>
>  /* handle direct returned errors */
>  static inline int libbpf_err(int ret)
> --
> 2.34.1
>