From: Andrii Nakryiko <andrii@kernel.org>
Date: Thu, 9 Dec 2021 11:38:29 -0800
Subject: libbpf: Fix bpf_prog_load() log_buf logic for log_level 0
Patch-mainline: v5.17-rc1
Git-commit: 4cf23a3c6359556a1cca489cf2b901e2b904c4b0
References: jsc#PED-1377
To unify libbpf APIs behavior w.r.t. log_buf and log_level, fix
bpf_prog_load() to follow the same logic as bpf_btf_load() and
high-level bpf_object__load() API will follow in the subsequent patches:
- if log_level is 0 and non-NULL log_buf is provided by a user, attempt
load operation initially with no log_buf and log_level set;
- if successful, we are done, return new FD;
- on error, retry the load operation with log_level bumped to 1 and
log_buf set; this way verbose logging will be requested only when we
are sure that there is a failure, but will be fast in the
common/expected success case.
Of course, user can still specify log_level > 0 from the very beginning
to force log collection.
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20211209193840.1248570-2-andrii@kernel.org
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
---
tools/lib/bpf/bpf.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -303,10 +303,6 @@ int bpf_prog_load_v0_6_0(enum bpf_prog_t
if (log_level && !log_buf)
return libbpf_err(-EINVAL);
- attr.log_level = log_level;
- attr.log_buf = ptr_to_u64(log_buf);
- attr.log_size = log_size;
-
func_info_rec_size = OPTS_GET(opts, func_info_rec_size, 0);
func_info = OPTS_GET(opts, func_info, NULL);
attr.func_info_rec_size = func_info_rec_size;
@@ -321,6 +317,12 @@ int bpf_prog_load_v0_6_0(enum bpf_prog_t
attr.fd_array = ptr_to_u64(OPTS_GET(opts, fd_array, NULL));
+ if (log_level) {
+ attr.log_buf = ptr_to_u64(log_buf);
+ attr.log_size = log_size;
+ attr.log_level = log_level;
+ }
+
fd = sys_bpf_prog_load(&attr, sizeof(attr), attempts);
if (fd >= 0)
return fd;
@@ -366,16 +368,17 @@ int bpf_prog_load_v0_6_0(enum bpf_prog_t
goto done;
}
- if (log_level || !log_buf)
- goto done;
-
- /* Try again with log */
- log_buf[0] = 0;
- attr.log_buf = ptr_to_u64(log_buf);
- attr.log_size = log_size;
- attr.log_level = 1;
+ if (log_level == 0 && log_buf) {
+ /* log_level == 0 with non-NULL log_buf requires retrying on error
+ * with log_level == 1 and log_buf/log_buf_size set, to get details of
+ * failure
+ */
+ attr.log_buf = ptr_to_u64(log_buf);
+ attr.log_size = log_size;
+ attr.log_level = 1;
- fd = sys_bpf_prog_load(&attr, sizeof(attr), attempts);
+ fd = sys_bpf_prog_load(&attr, sizeof(attr), attempts);
+ }
done:
/* free() doesn't affect errno, so we don't need to restore it */
free(finfo);