[PATCH] D133361: [BPF] Attribute btf_decl_tag("ctx") for structs

Mon Sep 4 14:21:31 PDT 2023

ast added a comment.

In D133361#4637008 <https://reviews.llvm.org/D133361#4637008>, @eddyz87 wrote:

> In D133361#4629292 <https://reviews.llvm.org/D133361#4629292>, @ast wrote:
>
>> ...
>> Right. Such recursive propagation of PAI is necessary. For btf_tag we cannot do it. Always propagating it won't be correct.
>> New preserve_const_field_offset would need to be propagated too and we actually have nested unions.
>> __bpf_md_ptr is such example. btf_tag wouldn't propagate into that union, but attr(preserve_const_field_offset) should.
>
> Hi Alexei,
>
> It just occurred to me that such an attribute would also require DWARF and BTF encoding in order to get reflected in vmlinux.h (which we already have for btf_decl_tag). Given this I think we can rename decl tag "ctx" to `btf_decl_tag("preserve_const_field_offset")` but we should still keep it a `btf_decl_tag`. I'll try to replace usage of `bpf_context_marker` intrinsic by metadata, if that fails will just rename the intrinsic to `preserve_const_field_offset`.
> What do you think? (Sorry, I should have thought this through last week).

I still feel that new attr is much cleaner from llvm implementation/design perspective and vmlinux.h inconvenience should be low priority in this considerations.
Since ctx only applies to uapi/bpf.h header the users don't have to use vmlinux.h. I know that today we have pains combining uapi headers and vmlinux.h and several solutions were
proposed. None were accepted yet, but that shouldn't mean that we should sacrifice llvm implementation due to orthogonal issues.
As a temporary workaround for vmlinux.h we can have uapi/bpf.h to do attr(preserve_const_field_offset) _and_ btf_decl_tag("bpf_ctx_struct"). Then teach pahole to emit attr(preserve_const_field_offset)
when it sees btf_decl_tag("bpf_ctx_struct") in vmlinux BTF.
Other workarounds are possible.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133361/new/

https://reviews.llvm.org/D133361