* [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