On 8/16/24 1:27 PM, Eduard Zingerman wrote:
On Fri, 2024-08-16 at 10:27 -0700, Martin KaFai Lau wrote:
[...]
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?
You are correct, this happens because bpf_map__attach_struct_ops() is
not called. Fortunately, the change for test_loader.c is not very big.
Please check two patches in the attachment.
The patch looks good. I tried and it works. I will add it in the next respin.
That will help to cover the __xlated check on the instructions generated by
gen_pro/epilogue and also check the syscall return value for the common case.
Except the tail_call test which needs to load a struct_ops program that does
bpf_tail_call and another struct_ops program that was used in the prog_array.
These two struct_ops programs need to be used in two separate struct_ops maps to
be able to load. The way that test_loader attaching all maps in your patch will
fail because bpf_testmod does not support attaching more than one struct_ops map.
I don't want to further polish on the tail_call testing side. I will stay with
the current way to do the tail_call test which also allows the more regular
trampoline "unsigned long long *ctx" for the main struct_ops prog and also
allows using ctx_in in the SEC("syscall") prog.
Thanks.
