Re: [PATCH] libbpf: Add friendly error prompt for missing .BTF section

[John Fastabend <john.fastabend@gmail.com>]

Tianyi Liu wrote:
> > -----Original Messages-----
> > From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > Date: Tuesday, September 27, 2022 at 12:37
> > To: Tianyi Liu <i.pear@outlook.com>
> > Cc: andrii@kernel.org <andrii@kernel.org>, bpf@vger.kernel.org <bpf@vger.kernel.org>, trivial@kernel.org <trivial@kernel.org>
> > 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@outlook.com> 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@outlook.com>
> > > ---
> > >  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.
>