Received-SPF: pass (google.com: domain of linux-kernel+bounces-112379-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <d3677b19-63fa-4787-8d5b-468ddbad173a@gmail.com>
Date: Sat, 23 Mar 2024 22:40:04 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] libbpf: add specific btf name info when do core
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
 Daniel Borkmann <daniel@iogearbox.net>,
 Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>,
 Yonghong Song <yonghong.song@linux.dev>,
 John Fastabend <john.fastabend@gmail.com>, bpf@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240321170444.388225-1-chen.dylane@gmail.com>
 <CAEf4BzYJeEViQaRJb6u06GJSLa6uBmykRZa4JnWJ9VXw+WoX2g@mail.gmail.com>
 <f5fe3709-bfc8-4906-a0cc-5fe9b388be6b@gmail.com>
 <CAEf4Bzb-Wf5sTnDLu29KQ-zWfCnffdUZYLSe_tQTNW_bTSfnPg@mail.gmail.com>
From: Tao Chen <chen.dylane@gmail.com>
In-Reply-To: <CAEf4Bzb-Wf5sTnDLu29KQ-zWfCnffdUZYLSe_tQTNW_bTSfnPg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Got it, thanks for your detailed reply.

在 2024/3/23 上午1:57, Andrii Nakryiko 写道:
> On Fri, Mar 22, 2024 at 6:37 AM Tao Chen <chen.dylane@gmail.com> wrote:
>>
>> Hi, Nakryiko, thank you for your reply. I try to familiarize with core
>> in libbpf， the debug info is really helpful for me. But i use the old
>> kernel btf, so origion debuginfo like:
>>    struct task_struct___x: found target candidate [130] struct
>> task_struct in [vmlinux]
> 
> the idea here is to not emit path to BTF (otherwise we normally should
> emit [/sys/kernel/btf/vmlinux], but rather distinguish whether it's a
> kernel BTF ([vmlinux]) or some kernel module BTF ([<module-name>]).
> 
> In your case you are overriding vmlinux BTF by using btf_custom_path
> option (so you should know where to find it, if you need to debug
> something). It's still, conceptually, a [vmlinux], and I'd like to
> keep it this way.
> 
>> I think it may be more clear, if we print btf name when we use old
>> kernel btf like:
>>    struct task_struct___x: found target candidate [130] struct
>> task_struct in [/boot/***.btf]
>> The patch just solve debug info show for newbies above.
>>
>> 在 2024/3/22 上午2:52, Andrii Nakryiko 写道:
>>> On Thu, Mar 21, 2024 at 10:04 AM Tao Chen <chen.dylane@gmail.com> wrote:
>>>>
>>>> No logic changed, just add specific btf name when core info
>>>> print, maybe it seems more understandable.
>>>>
>>>> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
>>>> ---
>>>>    tools/lib/bpf/libbpf.c | 14 +++++++++-----
>>>>    1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>
>>> Can you elaborate on what problem you are trying to solve?
>>> Conceptually libbpf does look for types in vmlinux (meaning main
>>> kernel BTF), even if user overrides BTF location (presumably because
>>> of old kernel). So even when we emit "vmlinux" in logs, it seems
>>> correct.
>>>
>>> pw-bot: cr
>>>
>>>
>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>> index afd09571c482..b32154288b4a 100644
>>>> --- a/tools/lib/bpf/libbpf.c
>>>> +++ b/tools/lib/bpf/libbpf.c
>>>> @@ -5653,7 +5653,8 @@ static int load_module_btfs(struct bpf_object *obj)
>>>>    }
>>>>
>>>>    static struct bpf_core_cand_list *
>>>> -bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 local_type_id)
>>>> +bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 local_type_id,
>>>> +                   const char *targ_btf_path)
>>>>    {
>>>>           struct bpf_core_cand local_cand = {};
>>>>           struct bpf_core_cand_list *cands;
>>>> @@ -5680,7 +5681,8 @@ bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 l
>>>>
>>>>           /* Attempt to find target candidates in vmlinux BTF first */
>>>>           main_btf = obj->btf_vmlinux_override ?: obj->btf_vmlinux;
>>>> -       err = bpf_core_add_cands(&local_cand, local_essent_len, main_btf, "vmlinux", 1, cands);
>>>> +       err = bpf_core_add_cands(&local_cand, local_essent_len, main_btf,
>>>> +                                targ_btf_path ?: "vmlinux", 1, cands);
>>>>           if (err)
>>>>                   goto err_out;
>>>>
>>>> @@ -5793,7 +5795,8 @@ static int bpf_core_resolve_relo(struct bpf_program *prog,
>>>>                                    int relo_idx,
>>>>                                    const struct btf *local_btf,
>>>>                                    struct hashmap *cand_cache,
>>>> -                                struct bpf_core_relo_res *targ_res)
>>>> +                                struct bpf_core_relo_res *targ_res,
>>>> +                                const char *targ_btf_path)
>>>>    {
>>>>           struct bpf_core_spec specs_scratch[3] = {};
>>>>           struct bpf_core_cand_list *cands = NULL;
>>>> @@ -5813,7 +5816,7 @@ static int bpf_core_resolve_relo(struct bpf_program *prog,
>>>>
>>>>           if (relo->kind != BPF_CORE_TYPE_ID_LOCAL &&
>>>>               !hashmap__find(cand_cache, local_id, &cands)) {
>>>> -               cands = bpf_core_find_cands(prog->obj, local_btf, local_id);
>>>> +               cands = bpf_core_find_cands(prog->obj, local_btf, local_id, targ_btf_path);
>>>>                   if (IS_ERR(cands)) {
>>>>                           pr_warn("prog '%s': relo #%d: target candidate search failed for [%d] %s %s: %ld\n",
>>>>                                   prog_name, relo_idx, local_id, btf_kind_str(local_type),
>>>> @@ -5920,7 +5923,8 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
>>>>                           if (prog->obj->gen_loader)
>>>>                                   continue;
>>>>
>>>> -                       err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res);
>>>> +                       err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res,
>>>> +                                                   targ_btf_path);
>>>>                           if (err) {
>>>>                                   pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
>>>>                                           prog->name, i, err);
>>>> --
>>>> 2.34.1
>>>>