Hi Daniel,
2018-01-10 11:32 UTC+0100 ~ Daniel Borkmann <dan...@iogearbox.net>
> On 01/10/2018 06:07 AM, Jakub Kicinski wrote:
>> From: Quentin Monnet <quentin.mon...@netronome.com>
>>
>> Rename the BPF verifier `verbose()` to `bpf_verifier_log_write()` and
>> export it, so that other components (in particular, drivers for BPF
>> offload) can reuse the user buffer log to dump error messages at
>> verification time.
>>
>> Renaming `verbose()` was necessary in order to avoid a name so generic
>> to be exported to the global namespace. However to prevent too much pain
>> for backports, the calls to `verbose()` in the kernel BPF verifier were
>> not changed. Instead, add a `#define verbose bpf_verifier_log_write`.
>> Another solution could consist in making a wrapper around `verbose()`,
>> but since it is a variadic function, I don't see a clean way without
>> creating two identical wrappers, one for the verifier and one to export.
>> I opted for a simple #define for now.
>>
>> Signed-off-by: Quentin Monnet <quentin.mon...@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicin...@netronome.com>
>> ---
>> include/linux/bpf_verifier.h | 3 +++
>> kernel/bpf/verifier.c | 14 ++++++++++----
>> 2 files changed, 13 insertions(+), 4 deletions(-)
>>
[...]
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d921ab387b0b..e674944811d1 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
[...]
>> @@ -197,6 +197,12 @@ static __printf(2, 3) void verbose(struct
>> bpf_verifier_env *env,
>> else
>> log->ubuf = NULL;
>> }
>> +EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
>> +/* Historically bpf_verifier_log_write was called verbose, but the name was
>> too
>> + * generic for symbol export. The function was renamed, but not the calls in
>> + * the verifier to avoid complicating backports. Hence the #define below.
>> + */
>> +#define verbose bpf_verifier_log_write
>
> Series looks good to me, one minor nit (or rather question) below:
>
> Any objections to properly use function aliasing instead of define?
>
> E.g. something along these lines:
>
> static __printf(2, 3) void verbose(struct bpf_verifier_env *env,
> const char *fmt, ...)
> __attribute__((alias("bpf_verifier_log_write")));
No objection at all, I simply did not know I could do that.
> By the way, for some strange reason the series didn't make it into patchwork:
>
> http://patchwork.ozlabs.org/project/netdev/list/?state=*&page=1
>
> I can only see patch 1/14 did arrive, although I got the whole series in my
> inbox:
>
> http://patchwork.ozlabs.org/patch/857939/
>
> Could you try to resend? Hopefully it will appear on 2nd try.
>
> Thanks,
> Daniel
I'll send a v2 of the series with the fix for function aliasing,
hopefully patchwork will take it. Thanks a lot!
Quentin
