Skip to contentSkip to conversation

lib/bpf: Fix log level in verbose mode with libbpf

PATCH In review Paul Chaignon 2 participants 4 messages Feb 2, 2022

Subsystems (3)
Networking Network drivers Library code
lore
Share & tools
lore

Paul Chaignon

author
#0 lore raw

PATCH 1/1 lib/bpf: Fix log level in verbose mode with libbpf

Before commit c04e45d083e8 ("lib/bpf: fix verbose flag when using libbpf"), the verifier logs were not displayed when the verbose flag was passed. Commit c04e45d083e8 fixed that bug but enabled the incorrect log level. This commit fixes it.

The kernel supports two log levels. With level 1, the verifier dumps instructions along each visited path with the verifier state for some. With level 2, only instructions for the last visited path are dumped but each instruction is preceded by its verifier state.

Log level 1 probably makes the most sense for the verbose flag as it has the most comprehensive information (full traversal of the program by the verifier). It also matches the log level when not using libbpf.

Fixes c04e45d083e8 ("lib/bpf: fix verbose flag when using libbpf")
Signed-off-by Paul Chaignon <[email protected]>
---
 lib/bpf_libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/bpf_libbpf.c +1 -1
index 50ef16bd..bb6399bf 100644
@@ -305,7 +305,7 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 
 	attr.obj = obj;
 	if (cfg->verbose)
-		attr.log_level = 2;
+		attr.log_level = 1;
 
 	ret = bpf_object__load_xattr(&attr);
 	if (ret)
Signature · 1 line
2.25.1
#1 lore raw

lib/bpf_libbpf.c static int load_bpf_object(struct bpf_cfg_in *cfg) · line 311

Quoted 13 lines · Paul Chaignon
lib/bpf_libbpf.c +1 -1
index 50ef16bd..bb6399bf 100644
@@ -305,7 +305,7 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 
 	attr.obj = obj;
 	if (cfg->verbose)
-		attr.log_level = 2;
+		attr.log_level = 1;
 
 	ret = bpf_object__load_xattr(&attr);
 	if (ret)

ip and tc do not have verbosity flags, but there is show_details. Why not tie the log_level to that?

author
#2 lore raw
Quoted 12 lines · David Ahern
Quoted 11 lines · Paul Chaignon
lib/bpf_libbpf.c +1 -1
index 50ef16bd..bb6399bf 100644
@@ -305,7 +305,7 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 
 	attr.obj = obj;
 	if (cfg->verbose)
-		attr.log_level = 2;
+		attr.log_level = 1;
Quoted 5 lines

lib/bpf_libbpf.c static int load_bpf_object(struct bpf_cfg_in *cfg) · line 311

Quoted 2 lines
lib/bpf_libbpf.c +0 -0
 	ret = bpf_object__load_xattr(&attr);
 	if (ret)

ip and tc do not have verbosity flags, but there is show_details. Why not tie the log_level to that?

I'm not sure I understand what you're proposing. This code is referring to the "verbose" parameter of the tc filter command, as in e.g.:

tc filter replace dev eth0 ingress bpf da obj prog.o sec sec1 verbose

Are you proposing we replace that parameter with the existing -details option?

#3 lore raw
Quoted 19 lines · Paul Chaignon
Quoted 12 lines · David Ahern
Quoted 11 lines · Paul Chaignon
lib/bpf_libbpf.c +1 -1
index 50ef16bd..bb6399bf 100644
@@ -305,7 +305,7 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 
 	attr.obj = obj;
 	if (cfg->verbose)
-		attr.log_level = 2;
+		attr.log_level = 1;
Quoted 5 lines

lib/bpf_libbpf.c static int load_bpf_object(struct bpf_cfg_in *cfg) · line 311

Quoted 2 lines
lib/bpf_libbpf.c +0 -0
 	ret = bpf_object__load_xattr(&attr);
 	if (ret)

ip and tc do not have verbosity flags, but there is show_details. Why not tie the log_level to that?

Quoted 8 lines

I'm not sure I understand what you're proposing. This code is referring to the "verbose" parameter of the tc filter command, as in e.g.:

tc filter replace dev eth0 ingress bpf da obj prog.o sec sec1 verbose

Are you proposing we replace that parameter with the existing -details option?

just proposing that log_level not be hardcoded and coming from the command line. Connecting with verbose arg makes more sense. How to extend it to let the user control verbosity