| Message ID | 20210902171929.3922667-5-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: implement variadic printk helper | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next | fail | VM_Test |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: songliubraving@fb.com kpsingh@kernel.org kafai@fb.com john.fastabend@gmail.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 89 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Thu, Sep 2, 2021 at 10:23 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > Instead of being a thin wrapper which calls into bpf_trace_printk, > libbpf's bpf_printk convenience macro now chooses between > bpf_trace_printk and bpf_trace_vprintk. If the arg count (excluding > format string) is >3, use bpf_trace_vprintk, otherwise use the older > helper. > > The motivation behind this added complexity - instead of migrating > entirely to bpf_trace_vprintk - is to maintain good developer experience > for users compiling against new libbpf but running on older kernels. > Users who are passing <=3 args to bpf_printk will see no change in their > bytecode. > > __bpf_vprintk functions similarly to BPF_SEQ_PRINTF and BPF_SNPRINTF > macros elsewhere in the file - it allows use of bpf_trace_vprintk > without manual conversion of varargs to u64 array. Previous > implementation of bpf_printk macro is moved to __bpf_printk for use by > the new implementation. > > This does change behavior of bpf_printk calls with >3 args in the "new > libbpf, old kernels" scenario. Before this patch, attempting to use 4 > args to bpf_printk results in a compile-time error. After this patch, > using bpf_printk with 4 args results in a trace_vprintk helper call > being emitted and a load-time failure on older kernels. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- Minor nit below, otherwise looks good: Acked-by: Andrii Nakryiko <andrii@kernel.org> > tools/lib/bpf/bpf_helpers.h | 45 ++++++++++++++++++++++++++++++------- > 1 file changed, 37 insertions(+), 8 deletions(-) > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > index b9987c3efa3c..a7e73be6dac4 100644 > --- a/tools/lib/bpf/bpf_helpers.h > +++ b/tools/lib/bpf/bpf_helpers.h > @@ -14,14 +14,6 @@ > #define __type(name, val) typeof(val) *name > #define __array(name, val) typeof(val) *name[] > > -/* Helper macro to print out debug messages */ > -#define bpf_printk(fmt, ...) \ > -({ \ > - char ____fmt[] = fmt; \ > - bpf_trace_printk(____fmt, sizeof(____fmt), \ > - ##__VA_ARGS__); \ > -}) > - > /* > * Helper macro to place programs, maps, license in > * different sections in elf_bpf file. Section names > @@ -224,4 +216,41 @@ enum libbpf_tristate { > ___param, sizeof(___param)); \ > }) > > +/* Helper macro to print out debug messages */ this comment probably should have been moved into the new bpf_printk() macro... > +#define __bpf_printk(fmt, ...) \ > +({ \ > + char ____fmt[] = fmt; \ > + bpf_trace_printk(____fmt, sizeof(____fmt), \ > + ##__VA_ARGS__); \ > +}) > + > +/* > + * __bpf_vprintk wraps the bpf_trace_vprintk helper with variadic arguments > + * instead of an array of u64. > + */ > +#define __bpf_vprintk(fmt, args...) \ > +({ \ > + static const char ___fmt[] = fmt; \ > + unsigned long long ___param[___bpf_narg(args)]; \ > + \ > + _Pragma("GCC diagnostic push") \ > + _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \ > + ___bpf_fill(___param, args); \ > + _Pragma("GCC diagnostic pop") \ > + \ > + bpf_trace_vprintk(___fmt, sizeof(___fmt), \ > + ___param, sizeof(___param)); \ > +}) > + > +/* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args > + * Otherwise use __bpf_vprintk > + */ > +#define ___bpf_pick_printk(...) \ > + ___bpf_nth(_, ##__VA_ARGS__, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ > + __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ > + __bpf_vprintk, __bpf_vprintk, __bpf_printk /*3*/, __bpf_printk /*2*/,\ > + __bpf_printk /*1*/, __bpf_printk /*0*/) > + > +#define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args) > + > #endif > -- > 2.30.2 >
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h index b9987c3efa3c..a7e73be6dac4 100644 --- a/tools/lib/bpf/bpf_helpers.h +++ b/tools/lib/bpf/bpf_helpers.h @@ -14,14 +14,6 @@ #define __type(name, val) typeof(val) *name #define __array(name, val) typeof(val) *name[] -/* Helper macro to print out debug messages */ -#define bpf_printk(fmt, ...) \ -({ \ - char ____fmt[] = fmt; \ - bpf_trace_printk(____fmt, sizeof(____fmt), \ - ##__VA_ARGS__); \ -}) - /* * Helper macro to place programs, maps, license in * different sections in elf_bpf file. Section names @@ -224,4 +216,41 @@ enum libbpf_tristate { ___param, sizeof(___param)); \ }) +/* Helper macro to print out debug messages */ +#define __bpf_printk(fmt, ...) \ +({ \ + char ____fmt[] = fmt; \ + bpf_trace_printk(____fmt, sizeof(____fmt), \ + ##__VA_ARGS__); \ +}) + +/* + * __bpf_vprintk wraps the bpf_trace_vprintk helper with variadic arguments + * instead of an array of u64. + */ +#define __bpf_vprintk(fmt, args...) \ +({ \ + static const char ___fmt[] = fmt; \ + unsigned long long ___param[___bpf_narg(args)]; \ + \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \ + ___bpf_fill(___param, args); \ + _Pragma("GCC diagnostic pop") \ + \ + bpf_trace_vprintk(___fmt, sizeof(___fmt), \ + ___param, sizeof(___param)); \ +}) + +/* Use __bpf_printk when bpf_printk call has 3 or fewer fmt args + * Otherwise use __bpf_vprintk + */ +#define ___bpf_pick_printk(...) \ + ___bpf_nth(_, ##__VA_ARGS__, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ + __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, __bpf_vprintk, \ + __bpf_vprintk, __bpf_vprintk, __bpf_printk /*3*/, __bpf_printk /*2*/,\ + __bpf_printk /*1*/, __bpf_printk /*0*/) + +#define bpf_printk(fmt, args...) ___bpf_pick_printk(args)(fmt, ##args) + #endif
Instead of being a thin wrapper which calls into bpf_trace_printk, libbpf's bpf_printk convenience macro now chooses between bpf_trace_printk and bpf_trace_vprintk. If the arg count (excluding format string) is >3, use bpf_trace_vprintk, otherwise use the older helper. The motivation behind this added complexity - instead of migrating entirely to bpf_trace_vprintk - is to maintain good developer experience for users compiling against new libbpf but running on older kernels. Users who are passing <=3 args to bpf_printk will see no change in their bytecode. __bpf_vprintk functions similarly to BPF_SEQ_PRINTF and BPF_SNPRINTF macros elsewhere in the file - it allows use of bpf_trace_vprintk without manual conversion of varargs to u64 array. Previous implementation of bpf_printk macro is moved to __bpf_printk for use by the new implementation. This does change behavior of bpf_printk calls with >3 args in the "new libbpf, old kernels" scenario. Before this patch, attempting to use 4 args to bpf_printk results in a compile-time error. After this patch, using bpf_printk with 4 args results in a trace_vprintk helper call being emitted and a load-time failure on older kernels. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- tools/lib/bpf/bpf_helpers.h | 45 ++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-)