2022-11-26 11:47:40

by Changbin Du

[permalink] [raw]
Subject: [PATCH 0/2] bpftool: improve error handing for missing .BTF section


Changbin Du (2):
libbpf: show more info about missing ".BTF" section
makefiles: do not generate empty vmlinux.h

samples/bpf/Makefile | 2 +-
tools/bpf/bpftool/Makefile | 2 +-
tools/bpf/runqslower/Makefile | 2 +-
tools/lib/bpf/btf.c | 12 ++++++++++++
tools/perf/Makefile.perf | 2 +-
tools/testing/selftests/bpf/Makefile | 2 +-
6 files changed, 17 insertions(+), 5 deletions(-)

--
2.37.2


2022-11-26 11:47:41

by Changbin Du

[permalink] [raw]
Subject: [PATCH 1/2] libbpf: show more info about missing ".BTF" section

Show more information about why failed instead of just saying "No such file or
directory".

Now will print below info:
libbpf: can not find '.BTF' section
libbpf: is CONFIG_DEBUG_INFO_BTF enabled for kernel?
Error: failed to load BTF from /home/changbin/work/linux/vmlinux: No such file or directory

Signed-off-by: Changbin Du <[email protected]>
---
tools/lib/bpf/btf.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index d88647da2c7f..3f661d991808 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -906,6 +906,15 @@ struct btf *btf__new(const void *data, __u32 size)
return libbpf_ptr(btf_new(data, size, NULL));
}

+static bool is_vmlinux(const char *path)
+{
+ size_t path_len = strlen(path);
+ size_t suffix_len = strlen("vmlinux");
+
+ return (path_len >= suffix_len) &&
+ (!memcmp(path + path_len - suffix_len, "vmlinux", suffix_len));
+}
+
static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
struct btf_ext **btf_ext)
{
@@ -990,6 +999,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
err = 0;

if (!btf_data) {
+ pr_warn("can not find '%s' section\n", BTF_ELF_SEC);
+ if (is_vmlinux(path))
+ pr_warn("is CONFIG_DEBUG_INFO_BTF enabled for kernel?\n");
err = -ENOENT;
goto done;
}
--
2.37.2

2022-11-26 11:48:02

by Changbin Du

[permalink] [raw]
Subject: [PATCH 2/2] makefiles: do not generate empty vmlinux.h

Remove the empty vmlinux.h if bpftool failed to dump btf info.
The emptry vmlinux.h can hide real error when reading output
of make.

Signed-off-by: Changbin Du <[email protected]>
---
samples/bpf/Makefile | 2 +-
tools/bpf/bpftool/Makefile | 2 +-
tools/bpf/runqslower/Makefile | 2 +-
tools/perf/Makefile.perf | 2 +-
tools/testing/selftests/bpf/Makefile | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 727da3c5879b..ab4788b4883e 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -362,7 +362,7 @@ ifeq ($(VMLINUX_BTF),)
$(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)",\
build the kernel or set VMLINUX_BTF or VMLINUX_H variable)
endif
- $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
+ $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@ || { rm $@; exit 1; }
else
$(Q)cp "$(VMLINUX_H)" $@
endif
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 4a95c017ad4c..d9d6f890884c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -177,7 +177,7 @@ BUILD_BPF_SKELS := 1

$(OUTPUT)vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL_BOOTSTRAP)
ifeq ($(VMLINUX_H),)
- $(QUIET_GEN)$(BPFTOOL_BOOTSTRAP) btf dump file $< format c > $@
+ $(QUIET_GEN)$(BPFTOOL_BOOTSTRAP) btf dump file $< format c > $@ || { rm $@; exit 1; }
else
$(Q)cp "$(VMLINUX_H)" $@
endif
diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
index 8b3d87b82b7a..2d7911f4666b 100644
--- a/tools/bpf/runqslower/Makefile
+++ b/tools/bpf/runqslower/Makefile
@@ -77,7 +77,7 @@ ifeq ($(VMLINUX_H),)
"specify its location." >&2; \
exit 1;\
fi
- $(QUIET_GEN)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
+ $(QUIET_GEN)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@ || { rm $@; exit 1; }
else
$(Q)cp "$(VMLINUX_H)" $@
endif
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index a432e59afc42..0546d408aa4e 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -1064,7 +1064,7 @@ VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))

$(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
ifeq ($(VMLINUX_H),)
- $(QUIET_GEN)$(BPFTOOL) btf dump file $< format c > $@
+ $(QUIET_GEN)$(BPFTOOL) btf dump file $< format c > $@ || { rm $@; exit 1; }
else
$(Q)cp "$(VMLINUX_H)" $@
endif
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e6cf21fad69f..9aa2475b4ac6 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -284,7 +284,7 @@ endif
$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
ifeq ($(VMLINUX_H),)
$(call msg,GEN,,$@)
- $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
+ $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@ || { rm $@; exit 1; }
else
$(call msg,CP,,$@)
$(Q)cp "$(VMLINUX_H)" $@
--
2.37.2

2022-11-29 06:03:00

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/2] libbpf: show more info about missing ".BTF" section

On Sat, Nov 26, 2022 at 3:13 AM Changbin Du <[email protected]> wrote:
>
> Show more information about why failed instead of just saying "No such file or
> directory".
>
> Now will print below info:
> libbpf: can not find '.BTF' section
> libbpf: is CONFIG_DEBUG_INFO_BTF enabled for kernel?
> Error: failed to load BTF from /home/changbin/work/linux/vmlinux: No such file or directory
>
> Signed-off-by: Changbin Du <[email protected]>
> ---
> tools/lib/bpf/btf.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index d88647da2c7f..3f661d991808 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -906,6 +906,15 @@ struct btf *btf__new(const void *data, __u32 size)
> return libbpf_ptr(btf_new(data, size, NULL));
> }
>
> +static bool is_vmlinux(const char *path)
> +{
> + size_t path_len = strlen(path);
> + size_t suffix_len = strlen("vmlinux");
> +
> + return (path_len >= suffix_len) &&
> + (!memcmp(path + path_len - suffix_len, "vmlinux", suffix_len));
> +}
> +
> static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> struct btf_ext **btf_ext)
> {
> @@ -990,6 +999,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> err = 0;
>
> if (!btf_data) {
> + pr_warn("can not find '%s' section\n", BTF_ELF_SEC);
> + if (is_vmlinux(path))
> + pr_warn("is CONFIG_DEBUG_INFO_BTF enabled for kernel?\n");

this is generic piece of BTF loading code in libbpf, it knows nothing
and should know nothing about vmlinux and CONFIG_DEBUG_INFO_BTF, this
is not the right place to add such suggestions.

Check bpf_object__load_vmlinux_btf(), libbpf emits vmlinux-specific
error there: "Error loading vmlinux BTF". If we need to mention
CONFIG_DEBUG_INFO_BTF anywhere, that would be the place to do that.

> err = -ENOENT;
> goto done;
> }
> --
> 2.37.2
>

2022-11-29 06:46:44

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 2/2] makefiles: do not generate empty vmlinux.h

On Sat, Nov 26, 2022 at 3:13 AM Changbin Du <[email protected]> wrote:
>
> Remove the empty vmlinux.h if bpftool failed to dump btf info.
> The emptry vmlinux.h can hide real error when reading output
> of make.

wouldn't this be better handled by adding .DELETE_ON_ERROR: to
bpftool's Makefile?

>
> Signed-off-by: Changbin Du <[email protected]>
> ---
> samples/bpf/Makefile | 2 +-
> tools/bpf/bpftool/Makefile | 2 +-
> tools/bpf/runqslower/Makefile | 2 +-
> tools/perf/Makefile.perf | 2 +-
> tools/testing/selftests/bpf/Makefile | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 727da3c5879b..ab4788b4883e 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -362,7 +362,7 @@ ifeq ($(VMLINUX_BTF),)
> $(error Cannot find a vmlinux for VMLINUX_BTF at any of "$(VMLINUX_BTF_PATHS)",\
> build the kernel or set VMLINUX_BTF or VMLINUX_H variable)
> endif
> - $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
> + $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@ || { rm $@; exit 1; }
> else
> $(Q)cp "$(VMLINUX_H)" $@
> endif
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 4a95c017ad4c..d9d6f890884c 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -177,7 +177,7 @@ BUILD_BPF_SKELS := 1
>
> $(OUTPUT)vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL_BOOTSTRAP)
> ifeq ($(VMLINUX_H),)
> - $(QUIET_GEN)$(BPFTOOL_BOOTSTRAP) btf dump file $< format c > $@
> + $(QUIET_GEN)$(BPFTOOL_BOOTSTRAP) btf dump file $< format c > $@ || { rm $@; exit 1; }
> else
> $(Q)cp "$(VMLINUX_H)" $@
> endif
> diff --git a/tools/bpf/runqslower/Makefile b/tools/bpf/runqslower/Makefile
> index 8b3d87b82b7a..2d7911f4666b 100644
> --- a/tools/bpf/runqslower/Makefile
> +++ b/tools/bpf/runqslower/Makefile
> @@ -77,7 +77,7 @@ ifeq ($(VMLINUX_H),)
> "specify its location." >&2; \
> exit 1;\
> fi
> - $(QUIET_GEN)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@
> + $(QUIET_GEN)$(BPFTOOL) btf dump file $(VMLINUX_BTF_PATH) format c > $@ || { rm $@; exit 1; }
> else
> $(Q)cp "$(VMLINUX_H)" $@
> endif
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index a432e59afc42..0546d408aa4e 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -1064,7 +1064,7 @@ VMLINUX_BTF ?= $(abspath $(firstword $(wildcard $(VMLINUX_BTF_PATHS))))
>
> $(SKEL_OUT)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL)
> ifeq ($(VMLINUX_H),)
> - $(QUIET_GEN)$(BPFTOOL) btf dump file $< format c > $@
> + $(QUIET_GEN)$(BPFTOOL) btf dump file $< format c > $@ || { rm $@; exit 1; }
> else
> $(Q)cp "$(VMLINUX_H)" $@
> endif
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index e6cf21fad69f..9aa2475b4ac6 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -284,7 +284,7 @@ endif
> $(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
> ifeq ($(VMLINUX_H),)
> $(call msg,GEN,,$@)
> - $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
> + $(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@ || { rm $@; exit 1; }
> else
> $(call msg,CP,,$@)
> $(Q)cp "$(VMLINUX_H)" $@
> --
> 2.37.2
>

2022-11-29 09:14:09

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH 2/2] makefiles: do not generate empty vmlinux.h

On Mon, Nov 28, 2022 at 10:00:14PM -0800, Andrii Nakryiko wrote:
> On Sat, Nov 26, 2022 at 3:13 AM Changbin Du <[email protected]> wrote:
> >
> > Remove the empty vmlinux.h if bpftool failed to dump btf info.
> > The emptry vmlinux.h can hide real error when reading output
> > of make.
>
> wouldn't this be better handled by adding .DELETE_ON_ERROR: to
> bpftool's Makefile?
>
yes, let me replace it.

--
Cheers,
Changbin Du

2022-11-29 10:09:09

by Changbin Du

[permalink] [raw]
Subject: Re: [PATCH 1/2] libbpf: show more info about missing ".BTF" section

On Mon, Nov 28, 2022 at 09:59:00PM -0800, Andrii Nakryiko wrote:
> On Sat, Nov 26, 2022 at 3:13 AM Changbin Du <[email protected]> wrote:
> >
> > Show more information about why failed instead of just saying "No such file or
> > directory".
> >
> > Now will print below info:
> > libbpf: can not find '.BTF' section
> > libbpf: is CONFIG_DEBUG_INFO_BTF enabled for kernel?
> > Error: failed to load BTF from /home/changbin/work/linux/vmlinux: No such file or directory
> >
> > Signed-off-by: Changbin Du <[email protected]>
> > ---
> > tools/lib/bpf/btf.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index d88647da2c7f..3f661d991808 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -906,6 +906,15 @@ struct btf *btf__new(const void *data, __u32 size)
> > return libbpf_ptr(btf_new(data, size, NULL));
> > }
> >
> > +static bool is_vmlinux(const char *path)
> > +{
> > + size_t path_len = strlen(path);
> > + size_t suffix_len = strlen("vmlinux");
> > +
> > + return (path_len >= suffix_len) &&
> > + (!memcmp(path + path_len - suffix_len, "vmlinux", suffix_len));
> > +}
> > +
> > static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> > struct btf_ext **btf_ext)
> > {
> > @@ -990,6 +999,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> > err = 0;
> >
> > if (!btf_data) {
> > + pr_warn("can not find '%s' section\n", BTF_ELF_SEC);
> > + if (is_vmlinux(path))
> > + pr_warn("is CONFIG_DEBUG_INFO_BTF enabled for kernel?\n");
>
> this is generic piece of BTF loading code in libbpf, it knows nothing
> and should know nothing about vmlinux and CONFIG_DEBUG_INFO_BTF, this
> is not the right place to add such suggestions.
>
> Check bpf_object__load_vmlinux_btf(), libbpf emits vmlinux-specific
> error there: "Error loading vmlinux BTF". If we need to mention
> CONFIG_DEBUG_INFO_BTF anywhere, that would be the place to do that.
>
Agreed. I think adding "can not find '.BTF' section" is enough to diagnose the problem.
The standalone error msg "No such file or directory" is really weird.

> > err = -ENOENT;
> > goto done;
> > }
> > --
> > 2.37.2
> >

--
Cheers,
Changbin Du