Re: [PATCH bpf-next 04/12] libbpf: Add btf enum64 support

[Yonghong Song <yhs@fb.com>]

On 5/10/22 3:40 PM, Yonghong Song wrote:
> 
> 
> On 5/9/22 4:25 PM, Andrii Nakryiko wrote:
>> On Sun, May 1, 2022 at 12:00 PM Yonghong Song <yhs@fb.com> wrote:
>>>
>>> Add BTF_KIND_ENUM64 support. Deprecated btf__add_enum() and
>>> btf__add_enum_value() and introduced the following new APIs
>>>    btf__add_enum32()
>>>    btf__add_enum32_value()
>>>    btf__add_enum64()
>>>    btf__add_enum64_value()
>>> due to new kind and introduction of kflag.
>>>
>>> To support old kernel with enum64, the sanitization is
>>> added to replace BTF_KIND_ENUM64 with a bunch of
>>> pointer-to-void types.
>>>
>>> The enum64 value relocation is also supported. The enum64
>>> forward resolution, with enum type as forward declaration
>>> and enum64 as the actual definition, is also supported.
>>>
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>> ---
>>>   tools/lib/bpf/btf.c                           | 226 +++++++++++++++++-
>>>   tools/lib/bpf/btf.h                           |  21 ++
>>>   tools/lib/bpf/btf_dump.c                      |  94 ++++++--
>>>   tools/lib/bpf/libbpf.c                        |  64 ++++-
>>>   tools/lib/bpf/libbpf.map                      |   4 +
>>>   tools/lib/bpf/libbpf_internal.h               |   2 +
>>>   tools/lib/bpf/linker.c                        |   2 +
>>>   tools/lib/bpf/relo_core.c                     |  93 ++++---
>>>   .../selftests/bpf/prog_tests/btf_dump.c       |  10 +-
>>>   .../selftests/bpf/prog_tests/btf_write.c      |   6 +-
>>>   10 files changed, 450 insertions(+), 72 deletions(-)
>>>
>>
[...]
>>
>>
>>> +       t->size = tsize;
>>> +
>>> +       return btf_commit_type(btf, sz);
>>> +}
>>> +
>>> +/*
>>> + * Append new BTF_KIND_ENUM type with:
>>> + *   - *name* - name of the enum, can be NULL or empty for anonymous 
>>> enums;
>>> + *   - *is_unsigned* - whether the enum values are unsigned or not;
>>> + *
>>> + * Enum initially has no enum values in it (and corresponds to enum 
>>> forward
>>> + * declaration). Enumerator values can be added by 
>>> btf__add_enum64_value()
>>> + * immediately after btf__add_enum() succeeds.
>>> + *
>>> + * Returns:
>>> + *   - >0, type ID of newly added BTF type;
>>> + *   - <0, on error.
>>> + */
>>> +int btf__add_enum32(struct btf *btf, const char *name, bool 
>>> is_unsigned)
>>
>> given it's still BTF_KIND_ENUM in UAPI, let's keep 32-bit ones as just
>> btf__add_enum()/btf__add_enum_value() and not deprecate anything.
>> ENUM64 can be thought about as more of a special case, so I think it's
>> ok.
> 
> The current btf__add_enum api:
> LIBBPF_API int btf__add_enum(struct btf *btf, const char *name, __u32 
> bytes_sz);
> 
> The issue is it doesn't have signedness parameter. if the user input
> is
>     enum { A = -1, B = 0, C = 1 };
> the actual printout btf format will be
>     enum { A 4294967295, B = 0, C = 1}
> does not match the original source.

I think I found a way to keep the current btf__add_enum() API.
Initially, the signedness will be unsigned. But during
btf__add_enum_value() api calls, if any negative value
is found, the signedness will change to signed. I think
this should work.

> 
>>
>>> +{
>>> +       return btf_add_enum_common(btf, name, is_unsigned, 
>>> BTF_KIND_ENUM, 4);
>>> +}
>>> +
>>
>> [...]
>>
[...]