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/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > index 2feb218c001d..6b66cd1aa0b9 100644 > --- a/include/linux/bpf_verifier.h > +++ b/include/linux/bpf_verifier.h > @@ -192,6 +192,9 @@ struct bpf_verifier_env { > u32 subprog_cnt; > }; > > +__printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env, > + const char *fmt, ...); > + > static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env) > { > struct bpf_verifier_state *cur = env->cur_state; > 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 > @@ -169,11 +169,11 @@ struct bpf_call_arg_meta { > static DEFINE_MUTEX(bpf_verifier_lock); > > /* log_level controls verbosity level of eBPF verifier. > - * verbose() is used to dump the verification trace to the log, so the user > - * can figure out what's wrong with the program > + * bpf_verifier_log_write() is used to dump the verification trace to the > log, > + * so the user can figure out what's wrong with the program > */ > -static __printf(2, 3) void verbose(struct bpf_verifier_env *env, > - const char *fmt, ...) > +__printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env, > + const char *fmt, ...) > { > struct bpf_verifer_log *log = &env->log; > unsigned int n; > @@ -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"))); 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 > static bool type_is_pkt_pointer(enum bpf_reg_type type) > { >