Re: [PATCH bpf-next v1 2/4] libbpf: Render decl_tags in btf_dump

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 9/17/26 4:59 PM, Andrii Nakryiko wrote:
> On Wed, Sep 16, 2026 at 6:21 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> [...]
>> ---
>>  tools/lib/bpf/btf_dump.c | 94 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 94 insertions(+)
>>
> 
> series looks good, just small nits below

Hi Andrii, thanks for the review.
Sorry I didn't reply earlier, wanted to get that REF_TYPE_FRAME stuff out.

> 
>> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
>> index 635fa969e145..1e8236c8b9a5 100644
>> --- a/tools/lib/bpf/btf_dump.c
>> +++ b/tools/lib/bpf/btf_dump.c
>> @@ -77,6 +77,11 @@ struct btf_dump_data {
>>         bool is_array_char;
>>  };
>>
>> +struct decl_tag_desc {
>> +       __u32 target_id;
>> +       __u32 tag_id;
>> +};
>> +
>>  struct btf_dump {
>>         const struct btf *btf;
>>         btf_dump_printf_fn_t printf_fn;
>> @@ -93,6 +98,11 @@ struct btf_dump {
>>         const char **cached_names;
>>         size_t cached_names_cap;
>>
>> +       /* decl tags, sorted by (target ID, tag ID) */
>> +       struct decl_tag_desc *decl_tags;
>> +       size_t decl_tags_cnt;
>> +       size_t decl_tags_cap;
> 
> nit: decl_tag_cnt, decl_tag_cap, "count" expects singular noun before
> it: "table count" not "tables count"

ack

> 
>> +
>>         /* topo-sorted list of dependent type definitions */
>>         __u32 *emit_queue;
>>         int emit_queue_cap;
>> @@ -192,9 +202,37 @@ struct btf_dump *btf_dump__new(const struct btf *btf,
>>         return libbpf_err_ptr(err);
>>  }
>>
>> +static int btf_dump_cmp_decl_tags(const void *a, const void *b)
>> +{
>> +       const struct decl_tag_desc *x = a, *y = b;
>> +
>> +       if (x->target_id != y->target_id)
>> +               return x->target_id < y->target_id ? -1 : 1;
>> +       return x->tag_id < y->tag_id ? -1 : 1;
>> +}
>> +
>> +static int btf_dump_push_decl_tag(struct btf_dump *d, __u32 id, const struct btf_type *t)
>> +{
>> +       const struct btf_type *target = btf__type_by_id(d->btf, t->type);
>> +
>> +       if (!target || (!btf_is_composite(target) && !btf_is_typedef(target)))
> 
> target shouldn't be null with correct t->type, don't add defensive checks

According to comments, decl_tags technically can refer to a type that
is not the BTF yet:

tools/lib/bpf/btf.c:3314-3323:

  /*
   * Append new BTF_KIND_DECL_TAG type with:
   *   - *value* - non-empty/non-NULL string;
   *   - *ref_type_id* - referenced type ID, it might not exist yet;
   *   ...
   */
  int btf__add_decl_tag(...)

Granted, I wouldn't expect this to be a real use-case. But if we
decide to drop checks like this, then I think we should be explicit
about "no incremental dump", and maybe simplify relevant code a bit.

> 
> but also why this artificial limitation on the kind of thing that is
> tagged? memory savings?

Yeah, I guess the effect of this is memory savings, but that wasn't
the motivation.  I just thought "why collect tags that we are not
going to dump?".  But the index can collect everything, it shouldn't
affect the dump. Do you think we should do that?


> 
> 
>> +               return 0;
>> +
>> +       if (libbpf_ensure_mem((void **)&d->decl_tags, &d->decl_tags_cap,
>> +                             sizeof(*d->decl_tags), d->decl_tags_cnt + 1))
>> +               return -ENOMEM;
>> +
>> +       d->decl_tags[d->decl_tags_cnt++] = (struct decl_tag_desc) {
>> +               .target_id = t->type,
>> +               .tag_id = id,
>> +       };
>> +       return 0;
>> +}
>> +
> 
> [...]