Received-SPF: pass (google.com: domain of linux-kernel+bounces-36004-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240123195449.1303643-1-irogers@google.com>
In-Reply-To: <20240123195449.1303643-1-irogers@google.com>
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date: Tue, 23 Jan 2024 12:06:42 -0800
Message-ID: <CAEf4BzY04bASavt2LTVBR6kFZb4ebci4Bs+V6rv91xo8OAYHNw@mail.gmail.com>
Subject: Re: [PATCH v1] libbpf: Add some details for BTF parsing failures
To: Ian Rogers <irogers@google.com>
Cc: Alan Maguire <alan.maguire@oracle.com>, Alexei Starovoitov <ast@kernel.org>, 
	Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, 
	Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, 
	Yonghong Song <yonghong.song@linux.dev>, John Fastabend <john.fastabend@gmail.com>, 
	KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, 
	Jiri Olsa <jolsa@kernel.org>, bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 23, 2024 at 11:54=E2=80=AFAM Ian Rogers <irogers@google.com> wr=
ote:
>
> As CONFIG_DEBUG_INFO_BTF is default off the existing "failed to find
> valid kernel BTF" message makes diagnosing the kernel build issue some
> what cryptic. Add a little more detail with the hope of helping users.
>
> Before:
> ```
> libbpf: failed to find valid kernel BTF
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> After no access:
> ```
> libbpf: failed to find a kernel (typically /sys/kernel/btf/vmlinux) for B=
TF data
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> After no BTF:
> ```
> libbpf: failed to find BTF in kernel (was CONFIG_DEBUG_INFO_BTF enabled?)
> libbpf: Error loading vmlinux BTF: -3
> libbpf: failed to load object 'lock_contention_bpf'
> libbpf: failed to load BPF skeleton 'lock_contention_bpf': -3
> ```
>
> Closes: https://lore.kernel.org/bpf/CAP-5=3DfU+DN_+Y=3DY4gtELUsJxKNDDCOvJ=
zPHvjUVaUoeFAzNnig@mail.gmail.com/
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/bpf/btf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index ee95fd379d4d..505c0fb2d1ed 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -4942,6 +4942,7 @@ struct btf *btf__load_vmlinux_btf(void)
>         struct utsname buf;
>         struct btf *btf;
>         int i, err;
> +       bool found_path =3D false;
>
>         uname(&buf);
>
> @@ -4951,6 +4952,7 @@ struct btf *btf__load_vmlinux_btf(void)
>                 if (faccessat(AT_FDCWD, path, R_OK, AT_EACCESS))
>                         continue;
>
> +               found_path =3D true;
>                 btf =3D btf__parse(path, NULL);
>                 err =3D libbpf_get_error(btf);
>                 pr_debug("loading kernel BTF '%s': %d\n", path, err);
> @@ -4960,7 +4962,11 @@ struct btf *btf__load_vmlinux_btf(void)
>                 return btf;
>         }
>
> -       pr_warn("failed to find valid kernel BTF\n");
> +       if (found_path)

wrong condition, should be !found_path (or rather swap below pr_warns)?

> +               pr_warn("failed to find BTF in kernel (was CONFIG_DEBUG_I=
NFO_BTF enabled?)\n");
> +       else
> +               pr_warn("failed to find a kernel (typically /sys/kernel/b=
tf/vmlinux) for BTF data\n");

"find a kernel for BTF data"? a) "kernel for BTF" reads weird and b)
we found it (found_path=3D=3Dtrue), we just failed to load it. So how
about:

  "failed to load kernel BTF data (typically at /sys/kernel/btf/vmlinux)"?

But I'd say we should do a bit better here. If /sys/kernel/btf/vmlinux
is there, we should probably stop and not even try fallback paths. And
so if we fail loading /sys/kernel/btf/vmlinux, then report failure
(probably with error code as well). If we have to fallback, then we
can remember the path we did find, but failed to load, and report that
(instead of wrongly reporting /sys/kernel/btf/vmlinux).


> +
>         return libbpf_err_ptr(-ESRCH);
>  }
>
> --
> 2.43.0.429.g432eaa2c6b-goog
>