* [bpf-next v3] bpf: adjust btf load error logging @ 2025-03-07 15:00 Jordan Rome 2025-03-11 0:09 ` Andrii Nakryiko 0 siblings, 1 reply; 3+ messages in thread From: Jordan Rome @ 2025-03-07 15:00 UTC (permalink / raw) To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kernel Team 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@jordanrome.com> --- 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); + 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [bpf-next v3] bpf: adjust btf load error logging 2025-03-07 15:00 [bpf-next v3] bpf: adjust btf load error logging Jordan Rome @ 2025-03-11 0:09 ` Andrii Nakryiko 2025-03-12 0:11 ` Jordan Rome 0 siblings, 1 reply; 3+ messages in thread From: Andrii Nakryiko @ 2025-03-11 0:09 UTC (permalink / raw) To: Jordan Rome Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kernel Team On Fri, Mar 7, 2025 at 7:00 AM Jordan Rome <linux@jordanrome.com> 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@jordanrome.com> > --- > 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 > ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [bpf-next v3] bpf: adjust btf load error logging 2025-03-11 0:09 ` Andrii Nakryiko @ 2025-03-12 0:11 ` Jordan Rome 0 siblings, 0 replies; 3+ messages in thread From: Jordan Rome @ 2025-03-12 0:11 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Kernel Team On Mon, Mar 10, 2025 at 8:10 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Mar 7, 2025 at 7:00 AM Jordan Rome <linux@jordanrome.com> 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@jordanrome.com> > > --- > > 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 > Let me try another version where we don't complicate the conditionals too much and don't repeat the log messages. > > + 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 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-12 0:11 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-07 15:00 [bpf-next v3] bpf: adjust btf load error logging Jordan Rome 2025-03-11 0:09 ` Andrii Nakryiko 2025-03-12 0:11 ` Jordan Rome
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox