On Thu, Sep 29, 2022 at 6:58 PM John Fastabend <john.fastabend@xxxxxxxxx> wrote:
>
> 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.
Agree. I don't think we should modify libbpf for this, best to add
extra check in bpftool.
>
> >
> > Thanks.
> >
>
>
