Re: [RFC v3 0/3] use preserve_static_offset in bpf uapi headers

[Quentin Monnet <quentin@isovalent.com>]

2023-12-20 20:19 UTC+0000 ~ Eduard Zingerman <eddyz87@gmail.com>
> On Wed, 2023-12-20 at 11:20 -0800, Alexei Starovoitov wrote:
>> On Wed, Dec 20, 2023 at 5:34 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>>> This RFC does not handle type pt_regs used for kprobes/
>>> This type is defined in architecture specific headers like
>>> arch/x86/include/asm/ptrace.h and is hidden behind typedef
>>> bpf_user_pt_regs_t in include/uapi/asm-generic/bpf_perf_event.h.
>>> There are two ways to handle struct pt_regs:
>>> 1. Modify all architecture specific ptrace.h files to use __bpf_ctx;
>>> 2. Add annotated forward declaration for pt_regs in
>>>    include/uapi/asm-generic/bpf_perf_event.h, e.g. as follows:
>>>
>>>     #if __has_attribute(preserve_static_offset) && defined(__bpf__)
>>>     #define __bpf_ctx __attribute__((preserve_static_offset))
>>>     #else
>>>     #define __bpf_ctx
>>>     #endif
>>>
>>>     struct __bpf_ctx pt_regs;
>>>
>>>     #undef __bpf_ctx
>>>
>>>     #include <linux/ptrace.h>
>>>
>>>     /* Export kernel pt_regs structure */
>>>     typedef struct pt_regs bpf_user_pt_regs_t;
>>>
>>> Unfortunately, it might be the case that option (2) is not sufficient,
>>> as at-least BPF selftests access pt_regs either via vmlinux.h or by
>>> directly including ptrace.h.
>>>
>>> If option (1) is to be implemented, it feels unreasonable to continue
>>> copying definition of __bpf_ctx macro from file to file.
>>> Given absence of common uapi exported headers between bpf.h and
>>> bpf_perf_event.h/ptrace.h, it looks like a new uapi header would have
>>> to be added, e.g. include/uapi/bpf_compiler.h.
>>> For the moment this header would contain only the definition for
>>> __bpf_ctx, and would be included in bpf.h, nf_bpf_link.h and
>>> architecture specific ptrace.h.
>>>
>>> Please advise.
>>
>> I'm afraid option 1 is a non starter. bpf quirks cannot impose
>> such heavy tax on the kernel.
>>
>> Option 2 is equally hacky.
>>
>> I think we should do what v2 did and hard code pt_regs in bpftool.
> 
> I agree on (1).
> As for (2), I use the same hack in current patch for bpftool to avoid
> hacking main logic of BPF dump, it works and is allowed by C language
> standard (albeit in vague terms, but example is present).
> Unfortunately (2) does not propagate to vmlinux.h.
> 
> Quentin, Alan, what do you think about hard-coding only pt_regs?


It sounds like an acceptable compromise.

Quentin