| Message ID | 20220202181146.GA75915@Mem (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2] lib/bpf: Fix log level in verbose mode with libbpf | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 2/2/22 11:11 AM, Paul Chaignon wrote: > diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c > index 50ef16bd..bb6399bf 100644 > --- a/lib/bpf_libbpf.c > +++ b/lib/bpf_libbpf.c > @@ -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?
On Wed, Feb 02, 2022 at 12:10:03PM -0700, David Ahern wrote: > On 2/2/22 11:11 AM, Paul Chaignon wrote: > > diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c > > index 50ef16bd..bb6399bf 100644 > > --- a/lib/bpf_libbpf.c > > +++ b/lib/bpf_libbpf.c > > @@ -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? 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?
On 2/2/22 2:07 PM, Paul Chaignon wrote: > On Wed, Feb 02, 2022 at 12:10:03PM -0700, David Ahern wrote: >> On 2/2/22 11:11 AM, Paul Chaignon wrote: >>> diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c >>> index 50ef16bd..bb6399bf 100644 >>> --- a/lib/bpf_libbpf.c >>> +++ b/lib/bpf_libbpf.c >>> @@ -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? > > 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
diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c index 50ef16bd..bb6399bf 100644 --- a/lib/bpf_libbpf.c +++ b/lib/bpf_libbpf.c @@ -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)
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 <paul@isovalent.com> --- lib/bpf_libbpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)