On Thu, Aug 11, 2022 at 10:24 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> In C, struct value can be passed as a function argument.
> For small structs, struct value may be passed in
> one or more registers. For trampoline based bpf programs,
> This would cause complication since one-to-one mapping between
> function argument and arch argument register is not valid
> any more.
>
> To support struct value argument and make bpf programs
> easy to write, the bpf program function parameter is
> changed from struct type to a pointer to struct type.
> The following is a simplified example.
>
> In one of later selftests, we have a bpf_testmod function:
> struct bpf_testmod_struct_arg_2 {
> long a;
> long b;
> };
> noinline int
> bpf_testmod_test_struct_arg_2(int a, struct bpf_testmod_struct_arg_2 b, int c) {
> bpf_testmod_test_struct_arg_result = a + b.a + b.b + c;
> return bpf_testmod_test_struct_arg_result;
> }
>
> When a bpf program is attached to the bpf_testmod function
> bpf_testmod_test_struct_arg_2(), the bpf program may look like
> SEC("fentry/bpf_testmod_test_struct_arg_2")
> int BPF_PROG(test_struct_arg_3, int a, struct bpf_testmod_struct_arg_2 *b, int c)
> {
> t2_a = a;
> t2_b_a = b->a;
> t2_b_b = b->b;
> t2_c = c;
> return 0;
> }
>
> Basically struct value becomes a pointer to the struct.
> The trampoline stack will be increased to store the stack values and
> the pointer to these values will be saved in the stack slot corresponding
> to that argument. For x86_64, the struct size is limited up to 16 bytes
> so the struct can fit in one or two registers. The struct size of more
> than 16 bytes is not supported now as our current use case is
> for sockptr_t in the argument. We could handle such large struct's
> in the future if we have concrete use cases.
>
> The main changes are in save_regs() and restore_regs(). The following
> illustrated the trampoline asm codes for save_regs() and restore_regs().
> save_regs():
> /* first argument */
> mov DWORD PTR [rbp-0x18],edi
> /* second argument: struct, save actual values and put the pointer to the slot */
> lea rax,[rbp-0x40]
> mov QWORD PTR [rbp-0x10],rax
> mov QWORD PTR [rbp-0x40],rsi
> mov QWORD PTR [rbp-0x38],rdx
> /* third argument */
> mov DWORD PTR [rbp-0x8],esi
> restore_regs():
> mov edi,DWORD PTR [rbp-0x18]
> mov rsi,QWORD PTR [rbp-0x40]
> mov rdx,QWORD PTR [rbp-0x38]
> mov esi,DWORD PTR [rbp-0x8]
Not sure whether it was discussed before, but
why cannot we adjust the bpf side instead?
Technically struct passing between bpf progs was never
officially supported. llvm generates something.
Probably always passes by reference, but we can adjust
that behavior without breaking any programs because
we don't have bpf libraries. Programs are fully contained
in one or few files. libbpf can do static linking, but
without any actual libraries the chance of breaking
backward compat is close to zero.
Can we teach llvm to pass sizeof(struct) <= 16 in
two bpf registers?
Then we wouldn't need to have a discrepancy between
kernel function prototype and bpf fentry prog proto.
Both will have struct by value in the same spot.
The trampoline generation will be simpler for x86 and
its runtime faster too.
The other architectures that pass small structs by reference
can do a bit more work in the trampoline: copy up to 16 byte
and bpf prog side will see it as they were passed in 'registers'.
wdyt?
