From: John Fastabend <john.fastabend@gmail.com>
To: Andrii Nakryiko <andrii@kernel.org>,
bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net
Cc: andrii@kernel.org, kernel-team@fb.com
Subject: RE: [PATCH v2 bpf-next 01/12] libbpf: fix bpf_prog_load() log_buf logic for log_level 0
Date: Wed, 08 Dec 2021 22:26:12 -0800 [thread overview]
Message-ID: <61b1a1844d712_ae146208b@john.notmuch> (raw)
In-Reply-To: <20211209004920.4085377-2-andrii@kernel.org>
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@kernel.org>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
[...]
> @@ -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
>
next prev parent reply other threads:[~2021-12-09 6:26 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 0:49 [PATCH v2 bpf-next 00/12] Enhance and rework logging controls in libbpf Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 01/12] libbpf: fix bpf_prog_load() log_buf logic for log_level 0 Andrii Nakryiko
2021-12-09 6:26 ` John Fastabend [this message]
2021-12-09 7:01 ` Andrii Nakryiko
2021-12-09 19:44 ` Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 02/12] libbpf: add OPTS-based bpf_btf_load() API Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 03/12] libbpf: allow passing preallocated log_buf when loading BTF into kernel Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 04/12] libbpf: allow passing user log setting through bpf_object_open_opts Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 05/12] libbpf: improve logging around BPF program loading Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 06/12] libbpf: preserve kernel error code and remove kprobe prog type guessing Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 07/12] libbpf: add per-program log buffer setter and getter Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 08/12] libbpf: deprecate bpf_object__load_xattr() Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 09/12] selftests/bpf: replace all uses of bpf_load_btf() with bpf_btf_load() Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 10/12] selftests/bpf: add test for libbpf's custom log_buf behavior Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 11/12] selftests/bpf: remove the only use of deprecated bpf_object__load_xattr() Andrii Nakryiko
2021-12-09 0:49 ` [PATCH v2 bpf-next 12/12] bpftool: switch bpf_object__load_xattr() to bpf_object__load() Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=61b1a1844d712_ae146208b@john.notmuch \
--to=john.fastabend@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox