Re: [RFC PATCH bpf-next 0/7] bpf: Support struct value argument for trampoline base progs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/29/22 11:04 AM, Kui-Feng Lee wrote:
On Thu, 2022-07-28 at 16:30 -0700, Yonghong Song wrote:


On 7/28/22 12:57 PM, Kui-Feng Lee wrote:
On Thu, 2022-07-28 at 10:46 -0700, Yonghong Song wrote:


On 7/28/22 8:46 AM, Kui-Feng Lee wrote:
On Tue, 2022-07-26 at 10:11 -0700, Yonghong Song wrote:
Currently struct arguments are not supported for trampoline
based
progs.
One of major reason is that struct argument may pass by value
which
may
use more than one registers. This breaks trampoline progs
where
each argument is assumed to take one register. bcc community
reported
the
issue ([1]) where struct argument is not supported for fentry
program.
     typedef struct {
           uid_t val;
     } kuid_t;
     typedef struct {
           gid_t val;
     } kgid_t;
     int security_path_chown(struct path *path, kuid_t uid,
kgid_t
gid);
Inside Meta, we also have a use case to attach to
tcp_setsockopt()
     typedef struct {
           union {
                   void            *kernel;
                   void __user     *user;
           };
           bool            is_kernel : 1;
     } sockptr_t;
     int tcp_setsockopt(struct sock *sk, int level, int
optname,
                        sockptr_t optval, unsigned int
optlen);

This patch added struct value support for bpf tracing
programs
which
uses trampoline. struct argument size needs to be 16 or less
so
it can fit in one or two registers. Based on analysis on llvm
and
experiments, atruct argument size greater than 16 will be
passed
as pointer to the struct.

Is it possible to force llvm to always pass a pointer to a
struct
over
8 bytes (the size of single register) for the BPF traget?

This is already the case for bpf target. Any struct parameter (1
byte, 2
bytes, ..., 8 types, ..., 16 bytes, ...) will be passed as a
reference.

But this is not the case for most other architectures. For
example,
for
x86_64, in most cases, struct size <= 16 will be passed with two
registers instead of as a reference.

I ask this question because you modify the signature of a bpf
program
to a pointer to a struct in patch #4.  Is that necessary if the
compiler passes a struct paramter as a reference?

Note that The true bpf program signature is only one.
long bpf_prog(<ctx_type> *ctx)
BPF_PROG is a macro for user friendly purpose.

For example,
+int BPF_PROG(test_struct_arg_1, struct bpf_testmod_struct_arg_2 *a,
int
b, int c)

after macro expansion:
int test_struct_arg_1(unsigned long long *ctx);
static __attribute__((always_inline))
typeof(test_struct_arg_1(0)) ____test_struct_arg_1(
    unsigned long long *ctx,
    struct bpf_testmod_struct_arg_2 *a, int b, int c);
...

If cast the pointer of __test_struct_arg_1() to the type (int
(*)(void*, void*, void*, void*)), test_struct_arg_1() can pass all
arguments to __tst_struct_arg_1() in the type (void *) without changing
the types of struct arguments to the pointer to a struct since all
struct types will be passed as a reference for the BPF target.

For example, the macro above can be expanded into the following block.
   ......
   int test_struct_arg_1(unsigned long long *ctx) {
     ......
     return ((typeof(name(0)) (*)(void*, void*, void*,
void*))&____test_struct_arg_1)(ctx[0], ctx[1], ctx[2], ctx[3]);
     ......

This doesn't really work. Here ctx[0]/ctx[1]/... has type
'unsigned long long' since ctx has type 'unsigned long long *'.

But if later parameter type is 'struct bpf_testmod_struct_arg_2'
in the function prototype and definition. Then implicitly we will
have a type conversion like
   (struct bpf_testmod_struct_arg_2)(unsigned long long)ctx[...]
and the above won't work as you cannot convert a 'unsigned long long'
to a struct.

   }
   ......




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux