Re: [PATCH bpf-next 1/2] libbpf: Add btf__type_cnt() and btf__raw_data() APIs

[Hengqi Chen <hengqi.chen@gmail.com>]

On 2021/10/21 7:14 AM, Andrii Nakryiko wrote:
> On Wed, Oct 20, 2021 at 6:51 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>
>>
>>
>> On 2021/10/20 1:48 AM, Andrii Nakryiko wrote:
>>> On Sat, Oct 9, 2021 at 8:01 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>>>
>>>> Add btf__type_cnt() and btf__raw_data() APIs and deprecate
>>>> btf__get_nr_type() and btf__get_raw_data() since the old APIs
>>>> don't follow the libbpf naming convention for getters which
>>>> omit 'get' in the name.[0] btf__raw_data() is just an alias to
>>>
>>> nit: this ".[0]" looks out of place, please use it as a reference in a
>>> sentence, e.g.,:
>>>
>>> omit 'get' in the name (see [0]).
>>>
>>> So that it reads naturally and fits the overall commit message.
>>>
>>>
>>
>> Got it. Will do.
>>
>>>> the existing btf__get_raw_data(). btf__type_cnt() now returns
>>>> the number of all types of the BTF object including 'void'.
>>>>
>>>>   [0] Closes: https://github.com/libbpf/libbpf/issues/279
>>>>
>>>> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
>>>> ---
>>>>  tools/lib/bpf/btf.c      | 36 ++++++++++++++++++++++--------------
>>>>  tools/lib/bpf/btf.h      |  4 ++++
>>>>  tools/lib/bpf/btf_dump.c |  8 ++++----
>>>>  tools/lib/bpf/libbpf.c   | 32 ++++++++++++++++----------------
>>>>  tools/lib/bpf/libbpf.map |  2 ++
>>>>  tools/lib/bpf/linker.c   | 28 ++++++++++++++--------------
>>>>  6 files changed, 62 insertions(+), 48 deletions(-)
>>>>
>>>
>>> [...]
>>>
>>>> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
>>>> index 864eb51753a1..49397a22d72b 100644
>>>> --- a/tools/lib/bpf/btf.h
>>>> +++ b/tools/lib/bpf/btf.h
>>>> @@ -131,7 +131,9 @@ LIBBPF_API __s32 btf__find_by_name(const struct btf *btf,
>>>>                                    const char *type_name);
>>>>  LIBBPF_API __s32 btf__find_by_name_kind(const struct btf *btf,
>>>>                                         const char *type_name, __u32 kind);
>>>> +LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__type_cnt() instead")
>>>
>>> it has to be scheduled to 0.7 to have a release with new API
>>> (btf__type_cnt) before we deprecate btf__get_nr_types(). It's probably
>>> worth mentioning in the deprecation message that btf__type_cnt()
>>> return is +1 from btf__get_nr_types(). Maybe something like:
>>>
>>
>> I am a little confused about this scheduling. You mentioned that
>> we can deprecate old API on the development version (0.6). See [0].
> 
> If we add some new API and deprecate old API (but recommend to use new
> API instead), we need to make sure that new API is there in at least
> one released libbpf version. Only then we can mark old API as
> deprecated in the next released libbpf version. In this case
> btf__type_cnt() has to go into v0.6 and btf__get_nr_types() can be
> deprecated in v0.7, not in v0.6.
> 
> Previous case in [0] was different, there was no new API we had to
> wait for, so we could deprecate the old API immediately.
> 

Thanks. That's clear.
Will do in next revision.

>>
>>
>>> LIBBPF_DEPRECATED_SINCE(0, 7, "use btf__type_cnt() instead; note that
>>> btf__get_nr_types() == btf__type_cnt() - 1")
>>>
>>
>> Will take this in v2.
>>
>>>>  LIBBPF_API __u32 btf__get_nr_types(const struct btf *btf);
>>>> +LIBBPF_API __u32 btf__type_cnt(const struct btf *btf);
>>>>  LIBBPF_API const struct btf *btf__base_btf(const struct btf *btf);
>>>>  LIBBPF_API const struct btf_type *btf__type_by_id(const struct btf *btf,
>>>>                                                   __u32 id);
>>>> @@ -144,7 +146,9 @@ LIBBPF_API int btf__resolve_type(const struct btf *btf, __u32 type_id);
>>>>  LIBBPF_API int btf__align_of(const struct btf *btf, __u32 id);
>>>>  LIBBPF_API int btf__fd(const struct btf *btf);
>>>>  LIBBPF_API void btf__set_fd(struct btf *btf, int fd);
>>>> +LIBBPF_DEPRECATED_SINCE(0, 6, "use btf__raw_data() instead")
>>>
>>> same, 0.7+
>>>
>>>>  LIBBPF_API const void *btf__get_raw_data(const struct btf *btf, __u32 *size);
>>>> +LIBBPF_API const void *btf__raw_data(const struct btf *btf, __u32 *size);
>>>>  LIBBPF_API const char *btf__name_by_offset(const struct btf *btf, __u32 offset);
>>>>  LIBBPF_API const char *btf__str_by_offset(const struct btf *btf, __u32 offset);
>>>>  LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name,
>>>
>>> [...]
>>>
>>
>>   [0] https://lore.kernel.org/all/CAEf4BzZ_JB1VLAF0=7gu=2M0M735aXava=nPL8m8ewQWdS3m8g@mail.gmail.com/