Re: [PATCH v5 bpf-next 3/9] libbpf: use kind layout to compute an unknown kind size

[Alan Maguire <alan.maguire@oracle.com>]

On 29/05/2025 06:35, Alexei Starovoitov wrote:
> On Wed, May 28, 2025 at 2:58 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> This allows BTF parsing to proceed even if we do not know the
>> kind.
>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>>  tools/lib/bpf/btf.c | 35 ++++++++++++++++++++++++++++-------
>>  1 file changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> index 43d1fce8977c..7a197dbfc689 100644
>> --- a/tools/lib/bpf/btf.c
>> +++ b/tools/lib/bpf/btf.c
>> @@ -355,7 +355,29 @@ static int btf_parse_kind_layout_sec(struct btf *btf)
>>         return 0;
>>  }
>>
>> -static int btf_type_size(const struct btf_type *t)
>> +/* for unknown kinds, consult kind layout. */
>> +static int btf_type_size_unknown(const struct btf *btf, const struct btf_type *t)
>> +{
>> +       int size = sizeof(struct btf_type);
>> +       struct btf_kind_layout *k = NULL;
>> +       __u16 vlen = btf_vlen(t);
>> +       __u8 kind = btf_kind(t);
>> +
>> +       if (btf->kind_layout)
>> +               k = &((struct btf_kind_layout *)btf->kind_layout)[kind];
>> +
>> +       if (!k || (void *)k > ((void *)btf->kind_layout + btf->hdr->kind_layout_len)) {
>> +               pr_debug("Unsupported BTF_KIND: %u\n", btf_kind(t));
>> +               return -EINVAL;
> 
> I'm missing the point around kind_layout->flags.
> I was expecting that this helper and others at least
> would check that flags == 0, but none of it is happening.
> The patches say that flags is unused and do nothing.
> Why add flags field at all?
>

The intent of the flags field is to provide space to add additional
information about BTF kind encoding that may prove useful. E.g. at time
of encoding for this kind, was the kind flag supported? Perhaps if the
size/type field specifies a type or a size might be another useful flag
setting. But basically the idea is to provide space for additional
information around kind encoding for future use.

So in that context, should we check that flags are 0 now? I'm not sure,
because in some cases we'd like to have older libbpf be able to handle
newer kind layouts which might make use of flags.

>> +       }
>> +
>> +       size += k->info_sz;
>> +       size += vlen * k->elem_sz;
>> +
>> +       return size;
>> +}