Andrii Nakryiko wrote:
> 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@xxxxxxxxxx>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
[...]
> @@ -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) {
^^^^^^^^
with non-Null log buf? Seems comment and above are out of sync?
Should it be, 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);
> --
> 2.30.2
>
