libbpf: Fix bpf_prog_load() log_buf logic for log_level 0

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

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 4e7836e1a7b525ab11e4c837fa9c71ac689e15f4..3dc86342f0a0fb396e5fc2fd75a7d17e2539b8a1 100644 (file)
--- 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_type prog_type,
        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_type prog_type,
 
        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_type prog_type,
                        goto done;
        }
 
-       if (log_level || !log_buf)
-               goto done;
+       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;
 
-       /* 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;
-
-       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);