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