[bpf-next,4/6] bpf: make BPF_LOG_* flags available in UAPI header

The kernel verifier combines several flags to select what kind of logs
to print to the log buffer provided by users.

In order to make it easier to provide the relevant flags, move the
related #define-s to the UAPI header, so that applications can set for
example: attr->log_level = BPF_LOG_LEVEL1 | BPF_LOG_STATS.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/linux/bpf_verifier.h | 3 ---
 include/uapi/linux/bpf.h     | 5 +++++
 2 files changed, 5 insertions(+), 3 deletions(-)

On Mon, Apr 29, 2019 at 10:52:25AM +0100, Quentin Monnet wrote:
> The kernel verifier combines several flags to select what kind of logs
> to print to the log buffer provided by users.
> 
> In order to make it easier to provide the relevant flags, move the
> related #define-s to the UAPI header, so that applications can set for
> example: attr->log_level = BPF_LOG_LEVEL1 | BPF_LOG_STATS.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  include/linux/bpf_verifier.h | 3 ---
>  include/uapi/linux/bpf.h     | 5 +++++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 1305ccbd8fe6..8160a4bb7ad9 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -253,9 +253,6 @@ static inline bool bpf_verifier_log_full(const struct bpf_verifier_log *log)
>  	return log->len_used >= log->len_total - 1;
>  }
>  
> -#define BPF_LOG_LEVEL1	1
> -#define BPF_LOG_LEVEL2	2
> -#define BPF_LOG_STATS	4
>  #define BPF_LOG_LEVEL	(BPF_LOG_LEVEL1 | BPF_LOG_LEVEL2)
>  #define BPF_LOG_MASK	(BPF_LOG_LEVEL | BPF_LOG_STATS)
>  
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 72336bac7573..f8e3e764aff4 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -335,6 +335,11 @@ struct bpf_stack_build_id {
>  	};
>  };
>  
> +/* verifier log_level values for loading programs, can be combined */
> +#define BPF_LOG_LEVEL1	1
> +#define BPF_LOG_LEVEL2	2
> +#define BPF_LOG_STATS	4

The verifier log levels are kernel implementation details.
They were not exposed before and shouldn't be exposed in the future.
I know that some folks already know about existence of level2 and use it
when the verifier rejects the program, but this is not uapi.
What is being output at level1 and 2 can change.
It's ok for libbpf to use this knowledge of kernel internals,
but it shouldn't be in uapi header.
That was the reason I didn't expose stats=4 in uapi in the first place
when I added that commit.

2019-05-04 23:19 UTC-0700 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Mon, Apr 29, 2019 at 10:52:25AM +0100, Quentin Monnet wrote:
>> The kernel verifier combines several flags to select what kind of logs
>> to print to the log buffer provided by users.
>>
>> In order to make it easier to provide the relevant flags, move the
>> related #define-s to the UAPI header, so that applications can set for
>> example: attr->log_level = BPF_LOG_LEVEL1 | BPF_LOG_STATS.
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> ---
>>  include/linux/bpf_verifier.h | 3 ---
>>  include/uapi/linux/bpf.h     | 5 +++++
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 1305ccbd8fe6..8160a4bb7ad9 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -253,9 +253,6 @@ static inline bool bpf_verifier_log_full(const struct bpf_verifier_log *log)
>>  	return log->len_used >= log->len_total - 1;
>>  }
>>  
>> -#define BPF_LOG_LEVEL1	1
>> -#define BPF_LOG_LEVEL2	2
>> -#define BPF_LOG_STATS	4
>>  #define BPF_LOG_LEVEL	(BPF_LOG_LEVEL1 | BPF_LOG_LEVEL2)
>>  #define BPF_LOG_MASK	(BPF_LOG_LEVEL | BPF_LOG_STATS)
>>  
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 72336bac7573..f8e3e764aff4 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -335,6 +335,11 @@ struct bpf_stack_build_id {
>>  	};
>>  };
>>  
>> +/* verifier log_level values for loading programs, can be combined */
>> +#define BPF_LOG_LEVEL1	1
>> +#define BPF_LOG_LEVEL2	2
>> +#define BPF_LOG_STATS	4
> 
> The verifier log levels are kernel implementation details.
> They were not exposed before and shouldn't be exposed in the future.
> I know that some folks already know about existence of level2 and use it
> when the verifier rejects the program, but this is not uapi.
> What is being output at level1 and 2 can change.
> It's ok for libbpf to use this knowledge of kernel internals,
> but it shouldn't be in uapi header.
> That was the reason I didn't expose stats=4 in uapi in the first place
> when I added that commit.
> 

Ok, in that case I will not move the macros. I take it there is also
little sense in offering different levels for the verifier through the
command line (the "--log-verifier level1,level2,stats" proposed in patch 6).

Since there was no real consensus on libbpf log level syntax either,
I'll resubmit the series (once bpf-next reopens) with just the shortcut
option, that sets all log levels to their maximum but without presenting
the different levels to the users. We can still add other options for
finer control over log levels after that, if they become desirable.

Thanks,
Quentin