2023-12-12 02:32 UTC+0000 ~ Eduard Zingerman <eddyz87@xxxxxxxxx> > When printing vmlinux.h emit attribute preserve_static_offset [0] for > types that are used as context parameters for BPF programs. To avoid > hacking libbpf dump logic emit forward declarations annotated with > attribute. Such forward declarations have to come before structure > definitions. > > Only emit such forward declarations when context types are present in > target BTF (identified by name). > > C language standard wording in section "6.7.2.1 Structure and union > specifiers" [1] is vague, but example in 6.7.2.1.21 explicitly allows > such notation, and it matches clang behavior. > > Here is how 'bpftool btf gen ... format c' looks after this change: > > #ifndef __VMLINUX_H__ > #define __VMLINUX_H__ > > #if !defined(BPF_NO_PRESERVE_STATIC_OFFSET) && \ > __has_attribute(preserve_static_offset) > #pragma clang attribute push \ > (__attribute__((preserve_static_offset)), apply_to = record) > > struct bpf_cgroup_dev_ctx; > ... > > #pragma clang attribute pop > #endif > > ... rest of the output unchanged ... > > This is a follow up for discussion in thread [2]. > > [0] https://clang.llvm.org/docs/AttributeReference.html#preserve-static-offset > [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3088.pdf > [2] https://lore.kernel.org/bpf/20231208000531.19179-1-eddyz87@xxxxxxxxx/ > > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > --- > tools/bpf/bpftool/btf.c | 131 +++++++++++++++++++++++++++++++++++----- > 1 file changed, 116 insertions(+), 15 deletions(-) > > diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c > index 91fcb75babe3..2abe71194afb 100644 > --- a/tools/bpf/bpftool/btf.c > +++ b/tools/bpf/bpftool/btf.c > @@ -460,11 +460,118 @@ static void __printf(2, 0) btf_dump_printf(void *ctx, > vfprintf(stdout, fmt, args); > } > > +static const char * const context_types[] = { > + "bpf_cgroup_dev_ctx", > + "bpf_nf_ctx", > + "bpf_perf_event_data", > + "bpf_raw_tracepoint_args", > + "bpf_sk_lookup", > + "bpf_sock", > + "bpf_sock_addr", > + "bpf_sock_ops", > + "bpf_sockopt", > + "bpf_sysctl", > + "__sk_buff", > + "sk_msg_md", > + "sk_reuseport_md", > + "xdp_md", > + "pt_regs", > +}; Hi, and thanks for this! Apologies for missing the discussion on v1. Reading through the previous thread I see that they were votes in favour of the hard-coded approach, but I would ask folks to please reconsider. I'm not keen on taking this list in bpftool, it doesn't seem to be the right place for that. I understand there's no plan to add new mirror context structs, but if we change policy for whatever reason, we're sure to miss the update in this list and that's a bug in bpftool. If bpftool ever gets ported to Windows and Windows needs support for new structs, that's more juggling to do to support multiple platforms. And if any tool other than bpftool needs to generate vmlinux.h headers in the future, it's back to square one - although by then there might be extra pushback for changing the BTF, if bpftool already does the work. Like Alan, I rather share your own inclination towards the more generic declaration tags approach, if you don't mind the additional work. Quentin
