diff mbox series

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

Message ID Zz-uG3hligqOqAMe@bolson-desk (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next] libbpf: Improve debug message when the base BTF cannot be found | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-17 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success Errors and warnings before: 2 (+0) this patch: 2 (+0)
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3 this patch: 3
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Olson, Matthew Nov. 21, 2024, 10:03 p.m. UTC 
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(-)

Comments

Andrii Nakryiko Nov. 21, 2024, 11:55 p.m. UTC | #1 
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
Olson, Matthew Nov. 22, 2024, 12:59 a.m. UTC | #2 
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
>
Olson, Matthew Nov. 22, 2024, 1:01 a.m. UTC | #3 
>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
John Fastabend Nov. 22, 2024, 5:35 a.m. UTC | #4 
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
diff mbox series

Patch

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;