Re: [PATCH bpf-next 0/1] use preserve_static_offset in bpf uapi headers

[Yonghong Song <yonghong.song@linux.dev>]

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.