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

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

[Tianyi Liu]

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);
 		err = -ENOENT;
 		goto done;
 	}
-- 
2.37.3

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

[Andrii Nakryiko]

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
>

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

[Tianyi Liu]

> -----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.

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.

Thanks.

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

[John Fastabend]

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.
> 

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

[Andrii Nakryiko]

On Thu, Sep 29, 2022 at 6:58 PM John Fastabend <john.fastabend@gmail.com> wrote:
>
> 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.

Agree. I don't think we should modify libbpf for this, best to add
extra check in bpftool.

>
> >
> > Thanks.
> >
>
>