Tianyi Liu wrote:
> > -----Original Messages-----
> > From: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>
> > Date: Tuesday, September 27, 2022 at 12:37
> > To: Tianyi Liu <i.pear@xxxxxxxxxxx>
> > Cc: andrii@xxxxxxxxxx <andrii@xxxxxxxxxx>, bpf@xxxxxxxxxxxxxxx <bpf@xxxxxxxxxxxxxxx>, trivial@xxxxxxxxxx <trivial@xxxxxxxxxx>
> > Subject: Re: [PATCH] libbpf: Add friendly error prompt for missing .BTF section
> > On Sun, Sep 25, 2022 at 10:54 PM Tianyi Liu <i.pear@xxxxxxxxxxx> wrote:
> > >
> > > Fresh users usually forget to turn on BTF generating flags compiling
> > > kernels, and will receive a confusing error "No such file or directory"
> > > (from return value ENOENT) with command "bpftool btf dump file vmlinux".
> > >
> > > Hope this can help them find the mistake.
> > >
> > > Signed-off-by: Tianyi Liu <i.pear@xxxxxxxxxxx>
> > > ---
> > > tools/lib/bpf/btf.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > index 2d14f1a52..9fbae1f3d 100644
> > > --- a/tools/lib/bpf/btf.c
> > > +++ b/tools/lib/bpf/btf.c
> > > @@ -990,6 +990,8 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> > > err = 0;
> > >
> > > if (!btf_data) {
> > > + pr_warn("Failed to get section %s from ELF %s, check CONFIG_DEBUG_INFO_BTF if compiling kernel\n",
> > > + BTF_ELF_SEC, path);
> >
> > This is going to be very confusing for any user trying to load BTF
> > from some other ELF file. If we want to add such helpful suggestion
> > (and even then it's a bit of a hit and miss, as not every passed in
> > file is supposed to be vmlinux kernel image), it should be done in
> > bpftool proper.
> >
> > > err = -ENOENT;
> > > goto done;
> > > }
> > > --
> > > 2.37.3
> > >
>
> Hi Andrii :
> I agree with you, I will try to implement it in bpftool.
>
> But there’s another problem here, in btf_parse_elf() from tools/lib/bpf/btf.c:
> If the path does not exist, open() assigns ENOENT to errno, and btf_parse_elf()
> returns -ENOENT. Besides, if .BTF section can not be found in the ELF file,
> btf_parse_elf() also returns -ENOENT, the same as above. So we can't determine
> which kind of error it is from the outside.
Just do a stat() on the file to see if it exists on error and then
give more verbose warning? We did something similar in our loader
code for tooling fwiw.
>
> Could we change the err code in the second case to make it clearer, Such as
> changing ENOENT to EPROTO / adding a new error code? Or we can just warn
> that .BTF section does not exist.
I don't think its needed.
>
> Thanks.
>
