[PATCH iproute2] lib/bpf: Fix log level in verbose mode with libbpf

[PATCH iproute2] lib/bpf: Fix log level in verbose mode with libbpf

[Paul Chaignon]

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(-)

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)
-- 
2.25.1

Re: [PATCH iproute2] lib/bpf: Fix log level in verbose mode with libbpf

[David Ahern]

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?

Re: [PATCH iproute2] lib/bpf: Fix log level in verbose mode with libbpf

[Paul Chaignon]

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?

Re: [PATCH iproute2] lib/bpf: Fix log level in verbose mode with libbpf

[David Ahern]

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