[bpf-next] bpftool: Enable libbpf logs when loading pid_iter in debug mode

Commit Message

Quentin Monnet March 20, 2024, 1:22 a.m. UTC

When trying to load the pid_iter BPF program used to iterate over the
PIDs of the processes holding file descriptors to BPF links, we would
unconditionally silence libbpf in order to keep the output clean if the
kernel does not support iterators and loading fails.

Although this is the desirable behaviour in most cases, this may hide
bugs in the pid_iter program that prevent it from loading, and it makes
it hard to debug such load failures, even in "debug" mode. Instead, it
makes more sense to print libbpf's logs when we pass the -d|--debug flag
to bpftool, so that users get the logs to investigate failures without
having to edit bpftool's source code.

Signed-off-by: Quentin Monnet <[e-mail]>
---
 tools/bpf/bpftool/pids.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

[e-mail] March 20, 2024, 6:20 a.m. UTC | #1

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Wed, 20 Mar 2024 01:22:41 +0000 you wrote:
> When trying to load the pid_iter BPF program used to iterate over the
> PIDs of the processes holding file descriptors to BPF links, we would
> unconditionally silence libbpf in order to keep the output clean if the
> kernel does not support iterators and loading fails.
> 
> Although this is the desirable behaviour in most cases, this may hide
> bugs in the pid_iter program that prevent it from loading, and it makes
> it hard to debug such load failures, even in "debug" mode. Instead, it
> makes more sense to print libbpf's logs when we pass the -d|--debug flag
> to bpftool, so that users get the logs to investigate failures without
> having to edit bpftool's source code.
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpftool: Enable libbpf logs when loading pid_iter in debug mode
    https://git.kernel.org/bpf/bpf-next/c/be24a895149b

You are awesome, thank you!

Patch

diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
index 00c77edb6331..9b898571b49e 100644
--- a/tools/bpf/bpftool/pids.c
+++ b/tools/bpf/bpftool/pids.c
@@ -101,7 +101,6 @@  int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
 	char buf[4096 / sizeof(*e) * sizeof(*e)];
 	struct pid_iter_bpf *skel;
 	int err, ret, fd = -1, i;
-	libbpf_print_fn_t default_print;
 
 	*map = hashmap__new(hash_fn_for_key_as_id, equal_fn_for_key_as_id, NULL);
 	if (IS_ERR(*map)) {
@@ -118,12 +117,18 @@  int build_obj_refs_table(struct hashmap **map, enum bpf_obj_type type)
 
 	skel->rodata->obj_type = type;
 
-	/* we don't want output polluted with libbpf errors if bpf_iter is not
-	 * supported
-	 */
-	default_print = libbpf_set_print(libbpf_print_none);
-	err = pid_iter_bpf__load(skel);
-	libbpf_set_print(default_print);
+	if (!verifier_logs) {
+		libbpf_print_fn_t default_print;
+
+		/* Unless debug information is on, we don't want the output to
+		 * be polluted with libbpf errors if bpf_iter is not supported.
+		 */
+		default_print = libbpf_set_print(libbpf_print_none);
+		err = pid_iter_bpf__load(skel);
+		libbpf_set_print(default_print);
+	} else {
+		err = pid_iter_bpf__load(skel);
+	}
 	if (err) {
 		/* too bad, kernel doesn't support BPF iterators yet */
 		err = 0;