On 8/15/24 6:50 PM, Eduard Zingerman wrote:
On Thu, 2024-08-15 at 17:23 -0700, Eduard Zingerman wrote:
[...]
Re: __retval(), the struct_ops progs is triggered by a SEC("syscall") prog.
Before calling this syscall prog, the st_ops map needs to be attached first. I
think the attach part is missing also? or there is a way?
I think libbpf handles the attachment automatically, I'll double check and reply.
In theory, the following addition to the example I've sent already should work:
struct st_ops_args;
int bpf_kfunc_st_ops_test_prologue(struct st_ops_args *args) __ksym;
SEC("syscall")
__retval(0)
int syscall_prologue(void *ctx)
{
struct st_ops_args args = { -42 };
bpf_kfunc_st_ops_test_prologue(&args);
return args.a;
}
However, the initial value of -42 is not changed, e.g. here is the log:
$ ./test_progs -vvv -t struct_ops_epilogue/syscall_prologue
...
libbpf: loaded kernel BTF from '/sys/kernel/btf/vmlinux'
libbpf: extern (func ksym) 'bpf_kfunc_st_ops_test_prologue': resolved to bpf_testmod [104486]
libbpf: struct_ops init_kern st_ops: type_id:44 kern_type_id:104321 kern_vtype_id:104378
libbpf: struct_ops init_kern st_ops: func ptr test_prologue is set to prog test_prologue from data(+0) to kern_data(+0)
libbpf: struct_ops init_kern st_ops: func ptr test_epilogue is set to prog test_epilogue from data(+8) to kern_data(+8)
libbpf: map 'st_ops': created successfully, fd=5
run_subtest:PASS:unexpected_load_failure 0 nsec
VERIFIER LOG:
=============
...
=============
do_prog_test_run:PASS:bpf_prog_test_run 0 nsec
run_subtest:FAIL:837 Unexpected retval: -42 != 0
#321/3 struct_ops_epilogue/syscall_prologue:FAIL
#321 struct_ops_epilogue:FAIL
So, something goes awry in bpf_kfunc_st_ops_test_prologue():
__bpf_kfunc int bpf_kfunc_st_ops_test_prologue(struct st_ops_args *args)
{
int ret = -1;
mutex_lock(&st_ops_mutex);
if (st_ops && st_ops->test_prologue)
Thanks for checking!
I think the bpf_map__attach_struct_ops() is not done such that st_ops is NULL.
It probably needs another tag in the SEC("syscall") program to tell which st_ops
map should be attached first before executing the "syscall" program.
I like the idea of using the __xlated macro to check the patched prologue, ctx
pointer saving, and epilogue. I will add this test in the respin. I will keep
the current way in this patch to exercise syscall and the ops/func in st_ops for
now. We can iterate on it later and use it as an example on what supports are
needed on the test_loader side for st_ops map testing. On the repetitive-enough
to worth test_loader refactoring side, I suspect some of the existing st_ops
load-success/load-failure tests may be worth to look at also. Thoughts?
ret = st_ops->test_prologue(args);
mutex_unlock(&st_ops_mutex);
return ret;
}
Either st_ops is null or st_ops->test_prologue is null.
However, the log above shows:
libbpf: struct_ops init_kern st_ops: type_id:44 kern_type_id:104321 kern_vtype_id:104378
libbpf: struct_ops init_kern st_ops: func ptr test_prologue is set to prog test_prologue from data(+0) to kern_data(+0)
libbpf: struct_ops init_kern st_ops: func ptr test_epilogue is set to prog test_epilogue from data(+8) to kern_data(+8)
Here libbpf does autoload for st_ops map and populates it, so st_ops->test_prologue should not be null.
Will have some time tomorrow to debug this (or you can give it a shot if you'd like).
