[v1] libbpf: Improve debug message when the base BTF cannot be found

[PATCH bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Ben Olson 1 year, 2 months ago

When running `bpftool` on a kernel module installed in `/lib/modules...`,
this error is encountered if the user does not specify `--base-btf` to
point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
However, looking at the debug output to determine the cause of the error
simply says `Invalid BTF string section`, which does not point to the
actual source of the error. This just improves that debug message to tell
users what happened.

Signed-off-by: Ben Olson <matthew.olson@intel.com>
---
 tools/lib/bpf/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 12468ae0d573..1a17de9d99e6 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
 		return -EINVAL;
 	}
 	if (!btf->base_btf && start[0]) {
-		pr_debug("Invalid BTF string section\n");
+		pr_debug("Cannot find base BTF\n");
 		return -EINVAL;
 	}
 	return 0;
-- 
2.47.0

Re: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Olson, Matthew 1 year, 2 months ago

From 22ed11ee2153fc921987eac7de24f564da9f9230 Mon Sep 17 00:00:00 2001
From: Ben Olson <matthew.olson@intel.com>
Date: Thu, 21 Nov 2024 11:26:35 -0600
Subject: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF
 cannot be found

When running `bpftool` on a kernel module installed in `/lib/modules...`,
this error is encountered if the user does not specify `--base-btf` to
point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
However, looking at the debug output to determine the cause of the error
simply says `Invalid BTF string section`, which does not point to the
actual source of the error. This just improves that debug message to tell
users what happened.

Signed-off-by: Ben Olson <matthew.olson@intel.com>
---

Changed in v2:
  * Made error message better reflect the condition

 tools/lib/bpf/btf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 12468ae0d573..a4ae2df68b91 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
     return -EINVAL;
   }
   if (!btf->base_btf && start[0]) {
-    pr_debug("Invalid BTF string section\n");
+    pr_debug("Malformed BTF string section, did you forget to provide base BTF?\n");
     return -EINVAL;
   }
   return 0;
--
2.47.0

Re: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Andrii Nakryiko 1 year, 2 months ago

On Thu, Nov 21, 2024 at 5:07 PM Olson, Matthew <matthew.olson@intel.com> wrote:
>
> From 22ed11ee2153fc921987eac7de24f564da9f9230 Mon Sep 17 00:00:00 2001
> From: Ben Olson <matthew.olson@intel.com>
> Date: Thu, 21 Nov 2024 11:26:35 -0600
> Subject: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF
>  cannot be found
>
> When running `bpftool` on a kernel module installed in `/lib/modules...`,
> this error is encountered if the user does not specify `--base-btf` to
> point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> However, looking at the debug output to determine the cause of the error
> simply says `Invalid BTF string section`, which does not point to the
> actual source of the error. This just improves that debug message to tell
> users what happened.
>
> Signed-off-by: Ben Olson <matthew.olson@intel.com>
> ---
>
> Changed in v2:
>   * Made error message better reflect the condition
>
>  tools/lib/bpf/btf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 12468ae0d573..a4ae2df68b91 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
>      return -EINVAL;
>    }
>    if (!btf->base_btf && start[0]) {
> -    pr_debug("Invalid BTF string section\n");
> +    pr_debug("Malformed BTF string section, did you forget to provide base BTF?\n");

I'm not sure why, but this v2 didn't make it into patchworks, so I
can't apply it. Can you please resend?

Also please make sure you don't change indentation (tabs -> spaces),
because it looks like that's what happened here.

>      return -EINVAL;
>    }
>    return 0;
> --
> 2.47.0

Re: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Olson, Matthew 1 year, 2 months ago

On Tue, Nov 26, 2024 at 11:21:21AM -0800, Andrii Nakryiko wrote:
> On Thu, Nov 21, 2024 at 5:07 PM Olson, Matthew <matthew.olson@intel.com> wrote:
> >
> > From 22ed11ee2153fc921987eac7de24f564da9f9230 Mon Sep 17 00:00:00 2001
> > From: Ben Olson <matthew.olson@intel.com>
> > Date: Thu, 21 Nov 2024 11:26:35 -0600
> > Subject: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF
> >  cannot be found
> >
> > When running `bpftool` on a kernel module installed in `/lib/modules...`,
> > this error is encountered if the user does not specify `--base-btf` to
> > point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> > However, looking at the debug output to determine the cause of the error
> > simply says `Invalid BTF string section`, which does not point to the
> > actual source of the error. This just improves that debug message to tell
> > users what happened.
> >
> > Signed-off-by: Ben Olson <matthew.olson@intel.com>
> > ---
> >
> > Changed in v2:
> >   * Made error message better reflect the condition
> >
> >  tools/lib/bpf/btf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 12468ae0d573..a4ae2df68b91 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
> >      return -EINVAL;
> >    }
> >    if (!btf->base_btf && start[0]) {
> > -    pr_debug("Invalid BTF string section\n");
> > +    pr_debug("Malformed BTF string section, did you forget to provide base BTF?\n");
> 
> I'm not sure why, but this v2 didn't make it into patchworks, so I
> can't apply it. Can you please resend?

Sure thing. Thanks.

> 
> Also please make sure you don't change indentation (tabs -> spaces),
> because it looks like that's what happened here.

Ach, rookie mistake. I'll add clang-format to my git hooks.

> 
> >      return -EINVAL;
> >    }
> >    return 0;
> > --
> > 2.47.0

Re: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Olson, Matthew 1 year, 2 months ago

On Tue, Nov 26, 2024 at 01:39:36PM -0600, Olson, Matthew wrote:
> On Tue, Nov 26, 2024 at 11:21:21AM -0800, Andrii Nakryiko wrote:
> > On Thu, Nov 21, 2024 at 5:07 PM Olson, Matthew <matthew.olson@intel.com> wrote:
> > >
> > > From 22ed11ee2153fc921987eac7de24f564da9f9230 Mon Sep 17 00:00:00 2001
> > > From: Ben Olson <matthew.olson@intel.com>
> > > Date: Thu, 21 Nov 2024 11:26:35 -0600
> > > Subject: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF
> > >  cannot be found
> > >
> > > When running `bpftool` on a kernel module installed in `/lib/modules...`,
> > > this error is encountered if the user does not specify `--base-btf` to
> > > point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> > > However, looking at the debug output to determine the cause of the error
> > > simply says `Invalid BTF string section`, which does not point to the
> > > actual source of the error. This just improves that debug message to tell
> > > users what happened.
> > >
> > > Signed-off-by: Ben Olson <matthew.olson@intel.com>
> > > ---
> > >
> > > Changed in v2:
> > >   * Made error message better reflect the condition
> > >
> > >  tools/lib/bpf/btf.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > > index 12468ae0d573..a4ae2df68b91 100644
> > > --- a/tools/lib/bpf/btf.c
> > > +++ b/tools/lib/bpf/btf.c
> > > @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
> > >      return -EINVAL;
> > >    }
> > >    if (!btf->base_btf && start[0]) {
> > > -    pr_debug("Invalid BTF string section\n");
> > > +    pr_debug("Malformed BTF string section, did you forget to provide base BTF?\n");
> > 
> > I'm not sure why, but this v2 didn't make it into patchworks, so I
> > can't apply it. Can you please resend?
> 
> Sure thing. Thanks.

Ah, I think I figured out why it didn't make it into patchworks; I'll resend
yet again.

> 
> > 
> > Also please make sure you don't change indentation (tabs -> spaces),
> > because it looks like that's what happened here.
> 
> Ach, rookie mistake. I'll add clang-format to my git hooks.
> 
> > 
> > >      return -EINVAL;
> > >    }
> > >    return 0;
> > > --
> > > 2.47.0
>

Re: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by John Fastabend 1 year, 2 months ago

Olson, Matthew wrote:
> From 22ed11ee2153fc921987eac7de24f564da9f9230 Mon Sep 17 00:00:00 2001
> From: Ben Olson <matthew.olson@intel.com>
> Date: Thu, 21 Nov 2024 11:26:35 -0600
> Subject: [PATCH v2 bpf-next] libbpf: Improve debug message when the base BTF
>  cannot be found
> 
> When running `bpftool` on a kernel module installed in `/lib/modules...`,
> this error is encountered if the user does not specify `--base-btf` to
> point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> However, looking at the debug output to determine the cause of the error
> simply says `Invalid BTF string section`, which does not point to the
> actual source of the error. This just improves that debug message to tell
> users what happened.
> 
> Signed-off-by: Ben Olson <matthew.olson@intel.com>
> ---


LGTM

Acked-by: John Fastabend <john.fastabend@gmail.com>

> 
> Changed in v2:
>   * Made error message better reflect the condition
> 
>  tools/lib/bpf/btf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 12468ae0d573..a4ae2df68b91 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
>      return -EINVAL;
>    }
>    if (!btf->base_btf && start[0]) {
> -    pr_debug("Invalid BTF string section\n");
> +    pr_debug("Malformed BTF string section, did you forget to provide base BTF?\n");
>      return -EINVAL;
>    }
>    return 0;
> --
> 2.47.0

Re: [PATCH bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Andrii Nakryiko 1 year, 2 months ago

On Thu, Nov 21, 2024 at 2:08 PM Ben Olson <matthew.olson@intel.com> wrote:
>
> When running `bpftool` on a kernel module installed in `/lib/modules...`,
> this error is encountered if the user does not specify `--base-btf` to
> point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> However, looking at the debug output to determine the cause of the error
> simply says `Invalid BTF string section`, which does not point to the
> actual source of the error. This just improves that debug message to tell
> users what happened.
>
> Signed-off-by: Ben Olson <matthew.olson@intel.com>
> ---
>  tools/lib/bpf/btf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 12468ae0d573..1a17de9d99e6 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
>                 return -EINVAL;
>         }
>         if (!btf->base_btf && start[0]) {
> -               pr_debug("Invalid BTF string section\n");
> +               pr_debug("Cannot find base BTF\n");

Well, the check indeed checks the well-formedness of the BTF string
section. It is specified that the first byte has to be zero ("empty
string"), unless it's a split BTF.

Base BTF being missing is just one possible reason for this condition,
so I'm not sure if it's completely accurate to specialize this error
message so much. Perhaps maybe emitting "Malformed BTF string section,
did you forget to provide base BTF?" would be a bit better.

pw-bot: cr

>                 return -EINVAL;
>         }
>         return 0;
> --
> 2.47.0

Re: [PATCH bpf-next] libbpf: Improve debug message when the base BTF cannot be found

Posted by Olson, Matthew 1 year, 2 months ago

On Thu, Nov 21, 2024 at 03:55:15PM -0800, Andrii Nakryiko wrote:
> On Thu, Nov 21, 2024 at 2:08 PM Ben Olson <matthew.olson@intel.com> wrote:
> >
> > When running `bpftool` on a kernel module installed in `/lib/modules...`,
> > this error is encountered if the user does not specify `--base-btf` to
> > point to a valid base BTF (e.g. usually in `/sys/kernel/btf/vmlinux`).
> > However, looking at the debug output to determine the cause of the error
> > simply says `Invalid BTF string section`, which does not point to the
> > actual source of the error. This just improves that debug message to tell
> > users what happened.
> >
> > Signed-off-by: Ben Olson <matthew.olson@intel.com>
> > ---
> >  tools/lib/bpf/btf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 12468ae0d573..1a17de9d99e6 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -283,7 +283,7 @@ static int btf_parse_str_sec(struct btf *btf)
> >                 return -EINVAL;
> >         }
> >         if (!btf->base_btf && start[0]) {
> > -               pr_debug("Invalid BTF string section\n");
> > +               pr_debug("Cannot find base BTF\n");
> 
> Well, the check indeed checks the well-formedness of the BTF string
> section. It is specified that the first byte has to be zero ("empty
> string"), unless it's a split BTF.
> 
> Base BTF being missing is just one possible reason for this condition,
> so I'm not sure if it's completely accurate to specialize this error
> message so much. Perhaps maybe emitting "Malformed BTF string section,
> did you forget to provide base BTF?" would be a bit better.

That sounds much better; as long as it hints that the user should
check if they specified a base BTF or not, it's good with me! Thanks.

> 
> pw-bot: cr
> 
> >                 return -EINVAL;
> >         }
> >         return 0;
> > --
> > 2.47.0
>