On Tue, Feb 28, 2023 at 6:26 AM Dmitrii Dolgov <9erthalion6@xxxxxxxxx> wrote:
>
> Use libbpf_strerror_r to expand the error when failed to parse the btf
> file at btf_custom_path. It does not change a lot locally, but since the
> error will bubble up through a few layers, it may become quite
> confusing otherwise. As an example here is what happens when the file
> indicated via btf_custom_path does not exist and the caller uses
> strerror as well:
>
> libbpf: failed to parse target BTF: -2
> libbpf: failed to perform CO-RE relocations: -2
> libbpf: failed to load object 'bpf_probe'
> libbpf: failed to load BPF skeleton 'bpf_probe': -2
> [caller]: failed to load BPF object (errno: 2 | message: No such file or directory)
>
> In this context "No such file or directory" could be easily
> misinterpreted as belonging to some other part of loading process, e.g.
> the BPF object itself. With this change it would look a bit better:
>
> libbpf: failed to parse target BTF: No such file or directory
> libbpf: failed to perform CO-RE relocations: -2
> libbpf: failed to load object 'bpf_probe'
> libbpf: failed to load BPF skeleton 'bpf_probe': -2
> [caller]: failed to load BPF object (errno: 2 | message: No such file or directory)
>
> Signed-off-by: Dmitrii Dolgov <9erthalion6@xxxxxxxxx>
> ---
I find these text-only error messages more harmful, actually. Very
often their literal meaning is confusing, and instead the process is
to guess what's -Exxx error they represent, and go from there.
Recently me and Quentin discussed moving towards an approach where
we'd log both symbolic error value (-EPERM instead of -1) and also
human-readable text message. So I'd prefer us figuring out how to do
this ergonomically in libbpf and bpftool code base, and start moving
in that direction.
> tools/lib/bpf/libbpf.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 05c4db355f28..02a47552ad14 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -5683,7 +5683,10 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
> obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
> err = libbpf_get_error(obj->btf_vmlinux_override);
> if (err) {
> - pr_warn("failed to parse target BTF: %d\n", err);
> + char *cp, errmsg[STRERR_BUFSIZE];
> +
> + cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
> + pr_warn("failed to parse target BTF: %s\n", cp);
> return err;
> }
> }
> --
> 2.31.1
>
