On Fri, Dec 8, 2023 at 9:30 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
>
> On 12/8/23 6:34 AM, Eduard Zingerman wrote:
> > On Thu, 2023-12-07 at 18:28 -0800, Yonghong Song wrote:
> > [...]
> >> All context types are defined in include/linux/bpf_types.h.
> >> The context type bpf_nf_ctx is missing.
> > convert_ctx_access() is not applied for bpf_nf_ctx. Searching through
> > kernel code shows that BPF programs access this structure directly
> > (net/netfilter/nf_bpf_link.c):
> >
> > static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb,
> > const struct nf_hook_state *s)
> > {
> > const struct bpf_prog *prog = bpf_prog;
> > struct bpf_nf_ctx ctx = {
> > .state = s,
> > .skb = skb,
> > };
> >
> > return bpf_prog_run(prog, &ctx);
> > }
> >
> > I added __bpf_ctx only for types that are subject to convert_ctx_access()
> > transformation. On the other hand, applying it to each context type
> > should not hurt either. Which way would you prefer?
> >
> > [...]
> >
> >>> How to add the same definitions in vmlinux.h is an open question,
> >>> and most likely requires bpftool modification:
> >>> - Hard code generation of __bpf_ctx based on type names?
> >>> - Mark context types with some special
> >>> __attribute__((btf_decl_tag("preserve_static_offset")))
> >>> and convert it to __attribute__((preserve_static_offset))?
> >> The number of context types is limited, I would just go through
> >> the first approach with hard coding the list of ctx types and
> >> mark them with preserve_static_offset attribute in vmlinux.h.
> > Tbh, I'm with Alan here, generic approach seems a tad nicer.
> > Lets collect some more votes :)
>
> I just want to propose to have less work :-) since we are only dealing
> with a few structs in bpf domain. Note that eventually generated
> vmlinux.h will be the same whether we use 'hard code' approach or the
> decl_tag approach. The difference is just how to do it: - dwarf/btf with
> decl tag -> bpftool vmlinux.h gen, or - dwarf/btf without decl tag +
> hardcoded bpf ctx info -> bpftool vmlinux.h gen If we intends to cover
> all uapi data structures (to prevent unnecessary CORE relocation, esp.
> for troublesome bitfield operations), hardcoded approach won't work and
> we may have to go to decl tag approach.
+1 for simplicity of "hard code" approach.
We've stopped adding new uapi "mirror" structs like __sk_buff long ago.
The number of structs that need ctx rewrite will not increase.
