Re: [bpf-next v3] bpf: adjust btf load error logging

[Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx>]

On Fri, Mar 7, 2025 at 7:00 AM Jordan Rome <linux@xxxxxxxxxxxxxx> wrote:
>
> For kernels where btf is not mandatory
> we should log loading errors with `pr_info`
> and not retry where we increase the log level
> as this is just added noise.
>
> Signed-off-by: Jordan Rome <linux@xxxxxxxxxxxxxx>
> ---
>  tools/lib/bpf/btf.c             | 16 ++++++++++++----
>  tools/lib/bpf/libbpf.c          |  3 ++-
>  tools/lib/bpf/libbpf_internal.h |  2 +-
>  3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index eea99c766a20..c8139c3bc9e0 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -1379,7 +1379,7 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
>
>  int btf_load_into_kernel(struct btf *btf,
>                          char *log_buf, size_t log_sz, __u32 log_level,
> -                        int token_fd)
> +                        int token_fd, bool btf_mandatory)
>  {
>         LIBBPF_OPTS(bpf_btf_load_opts, opts);
>         __u32 buf_sz = 0, raw_size;
> @@ -1435,6 +1435,15 @@ int btf_load_into_kernel(struct btf *btf,
>
>         btf->fd = bpf_btf_load(raw_data, raw_size, &opts);
>         if (btf->fd < 0) {
> +               if (!btf_mandatory) {
> +                       err = -errno;
> +                       pr_info("BTF loading error: %s\n", errstr(err));
> +
> +                       if (!log_buf && log_level)
> +                               pr_info("-- BEGIN BTF LOAD LOG ---\n%s\n-- END BTF LOAD LOG --\n", buf);

I'm not a fan of duplicating this. Wouldn't something along the
following lines work as well?

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index eea99c766a20..fc06f3a8e8d7 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1436,18 +1436,19 @@ int btf_load_into_kernel(struct btf *btf,
        btf->fd = bpf_btf_load(raw_data, raw_size, &opts);
        if (btf->fd < 0) {
                /* time to turn on verbose mode and try again */
-               if (log_level == 0) {
+               if (log_level == 0 && (log_buf || btf_mandatory)) {
                        log_level = 1;
                        goto retry_load;
                }
                /* only retry if caller didn't provide custom log_buf, but
                 * make sure we can never overflow buf_sz
                 */
-               if (!log_buf && errno == ENOSPC && buf_sz <= UINT_MAX / 2)
+               if (!log_buf && btf_mandatory && errno == ENOSPC &&
buf_sz <= UINT_MAX / 2)
                        goto retry_load;

                err = -errno;
-               pr_warn("BTF loading error: %s\n", errstr(err));
+               __pr(btf_mandatory ? LIBBPF_WARN : LIBBPF_INFO,
+                    "BTF loading error: %s\n", errstr(err));
                /* don't print out contents of custom log_buf */
                if (!log_buf && buf[0])
                        pr_warn("-- BEGIN BTF LOAD LOG ---\n%s\n-- END
BTF LOAD LOG --\n", buf);


pw-bot: cr

> +                       goto done;
> +               }
> +
>                 /* time to turn on verbose mode and try again */
>                 if (log_level == 0) {
>                         log_level = 1;
> @@ -1448,8 +1457,7 @@ int btf_load_into_kernel(struct btf *btf,
>
>                 err = -errno;
>                 pr_warn("BTF loading error: %s\n", errstr(err));
> -               /* don't print out contents of custom log_buf */
> -               if (!log_buf && buf[0])
> +               if (!log_buf && log_level)
>                         pr_warn("-- BEGIN BTF LOAD LOG ---\n%s\n-- END BTF LOAD LOG --\n", buf);
>         }
>
> @@ -1460,7 +1468,7 @@ int btf_load_into_kernel(struct btf *btf,
>
>  int btf__load_into_kernel(struct btf *btf)
>  {
> -       return btf_load_into_kernel(btf, NULL, 0, 0, 0);
> +       return btf_load_into_kernel(btf, NULL, 0, 0, 0, true);
>  }
>
>  int btf__fd(const struct btf *btf)
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8e32286854ef..2cb3f067a12e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3604,9 +3604,10 @@ static int bpf_object__sanitize_and_load_btf(struct bpf_object *obj)
>                  */
>                 btf__set_fd(kern_btf, 0);
>         } else {
> +               btf_mandatory = kernel_needs_btf(obj);
>                 /* currently BPF_BTF_LOAD only supports log_level 1 */
>                 err = btf_load_into_kernel(kern_btf, obj->log_buf, obj->log_size,
> -                                          obj->log_level ? 1 : 0, obj->token_fd);
> +                                          obj->log_level ? 1 : 0, obj->token_fd, btf_mandatory);
>         }
>         if (sanitize) {
>                 if (!err) {
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index de498e2dd6b0..f1de2ba462c3 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -408,7 +408,7 @@ int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
>                          int token_fd);
>  int btf_load_into_kernel(struct btf *btf,
>                          char *log_buf, size_t log_sz, __u32 log_level,
> -                        int token_fd);
> +                        int token_fd, bool btf_mandatory);
>
>  struct btf *btf_get_from_fd(int btf_fd, struct btf *base_btf);
>  void btf_get_kernel_prefix_kind(enum bpf_attach_type attach_type,
> --
> 2.43.5
>