Re: [PATCH bpf-next 3/5] libbpf: add btf__new_split() API that was declared but not implemented

[Yonghong Song <yonghong.song@linux.dev>]

On 1/31/24 9:20 AM, Andrii Nakryiko wrote:
> On Tue, Jan 30, 2024 at 9:30 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 1/30/24 11:36 AM, Andrii Nakryiko wrote:
>>> Seems like original commit adding split BTF support intended to add
>>> btf__new_split() API, and even declared it in libbpf.map, but never
>>> added (trivial) implementation. Fix this.
>>>
>>> Fixes: ba451366bf44 ("libbpf: Implement basic split BTF support")
>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>> The patch LGTM. We did some cross checking between libbpf.map
>> and the implementation. What things are missing here to
>> capture missed implementation or LIBBPF_API marking?
> Yes, we still have it, and it does detect issues when API wasn't added
> into libbpf.map.
>
> I haven't investigated exactly why it didn't catch the issue when API
> is in libbpf.map, but is not marked with LIBBPF_API, but I suspect
> it's because existing check doesn't take into account visibility of
> the symbol, so there is some room for improvement.
>
> Similarly, not sure why it didn't detect that btf_ext__new_split()
> wasn't even implemented, probably because we don't distinguish UNDEF
> and FUNC symbols? So something to follow up on for sure.

Agreed. A followup to improve API matching between libbpf.map and code
can be done later.

>
>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>>
>>> ---
>>>    tools/lib/bpf/btf.c      | 5 +++++
>>>    tools/lib/bpf/libbpf.map | 3 ++-
>>>    2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>>> index 95db88b36cf3..845034d15420 100644
>>> --- a/tools/lib/bpf/btf.c
>>> +++ b/tools/lib/bpf/btf.c
>>> @@ -1079,6 +1079,11 @@ struct btf *btf__new(const void *data, __u32 size)
>>>        return libbpf_ptr(btf_new(data, size, NULL));
>>>    }
>>>
>>> +struct btf *btf__new_split(const void *data, __u32 size, struct btf *base_btf)
>>> +{
>>> +     return libbpf_ptr(btf_new(data, size, base_btf));
>>> +}
>>> +
>>>    static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
>>>                                 struct btf_ext **btf_ext)
>>>    {
>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>> index d9e1f57534fa..386964f572a8 100644
>>> --- a/tools/lib/bpf/libbpf.map
>>> +++ b/tools/lib/bpf/libbpf.map
>>> @@ -245,7 +245,6 @@ LIBBPF_0.3.0 {
>>>                btf__parse_raw_split;
>>>                btf__parse_split;
>>>                btf__new_empty_split;
>>> -             btf__new_split;
>>>                ring_buffer__epoll_fd;
>>>    } LIBBPF_0.2.0;
>>>
>>> @@ -411,5 +410,7 @@ LIBBPF_1.3.0 {
>>>    } LIBBPF_1.2.0;
>>>
>>>    LIBBPF_1.4.0 {
>>> +     global:
>>>                bpf_token_create;
>>> +             btf__new_split;
>>>    } LIBBPF_1.3.0;