Received-SPF: pass (google.com: domain of linux-kernel+bounces-111901-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240321170444.388225-1-chen.dylane@gmail.com>
 <CAEf4BzYJeEViQaRJb6u06GJSLa6uBmykRZa4JnWJ9VXw+WoX2g@mail.gmail.com> <f5fe3709-bfc8-4906-a0cc-5fe9b388be6b@gmail.com>
In-Reply-To: <f5fe3709-bfc8-4906-a0cc-5fe9b388be6b@gmail.com>
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date: Fri, 22 Mar 2024 10:57:39 -0700
Message-ID: <CAEf4Bzb-Wf5sTnDLu29KQ-zWfCnffdUZYLSe_tQTNW_bTSfnPg@mail.gmail.com>
Subject: Re: [PATCH] libbpf: add specific btf name info when do core
To: Tao Chen <chen.dylane@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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 22, 2024 at 6:37=E2=80=AFAM Tao Chen <chen.dylane@gmail.com> wr=
ote:
>
> Hi, Nakryiko, thank you for your reply. I try to familiarize with core
> in libbpf=EF=BC=8C 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.
>
> =E5=9C=A8 2024/3/22 =E4=B8=8A=E5=8D=882:52, Andrii Nakryiko =E5=86=99=E9=
=81=93:
> > On Thu, Mar 21, 2024 at 10:04=E2=80=AFAM Tao Chen <chen.dylane@gmail.co=
m> 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 *o=
bj)
> >>   }
> >>
> >>   static struct bpf_core_cand_list *
> >> -bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_b=
tf, __u32 local_type_id)
> >> +bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_b=
tf, __u32 local_type_id,
> >> +                   const char *targ_btf_path)
> >>   {
> >>          struct bpf_core_cand local_cand =3D {};
> >>          struct bpf_core_cand_list *cands;
> >> @@ -5680,7 +5681,8 @@ bpf_core_find_cands(struct bpf_object *obj, cons=
t struct btf *local_btf, __u32 l
> >>
> >>          /* Attempt to find target candidates in vmlinux BTF first */
> >>          main_btf =3D obj->btf_vmlinux_override ?: obj->btf_vmlinux;
> >> -       err =3D bpf_core_add_cands(&local_cand, local_essent_len, main=
_btf, "vmlinux", 1, cands);
> >> +       err =3D 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_prog=
ram *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] =3D {};
> >>          struct bpf_core_cand_list *cands =3D NULL;
> >> @@ -5813,7 +5816,7 @@ static int bpf_core_resolve_relo(struct bpf_prog=
ram *prog,
> >>
> >>          if (relo->kind !=3D BPF_CORE_TYPE_ID_LOCAL &&
> >>              !hashmap__find(cand_cache, local_id, &cands)) {
> >> -               cands =3D bpf_core_find_cands(prog->obj, local_btf, lo=
cal_id);
> >> +               cands =3D bpf_core_find_cands(prog->obj, local_btf, lo=
cal_id, targ_btf_path);
> >>                  if (IS_ERR(cands)) {
> >>                          pr_warn("prog '%s': relo #%d: target candidat=
e search failed for [%d] %s %s: %ld\n",
> >>                                  prog_name, relo_idx, local_id, btf_ki=
nd_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 =3D bpf_core_resolve_relo(prog, rec, i, ob=
j->btf, cand_cache, &targ_res);
> >> +                       err =3D bpf_core_resolve_relo(prog, rec, i, ob=
j->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
> >>