Hi,
On 1/3/2024 2:41 AM, Eduard Zingerman wrote:
> On Sat, 2023-12-23 at 18:40 +0800, Hou Tao wrote:
>> From: Hou Tao <houtao1@xxxxxxxxxx>
>>
>> The test uses bpf_prog_get_info_by_fd() to obtain the xlated
>> instructions of the program first. Since these instructions have
>> already been rewritten by the verifier, the tests then checks whether
>> the rewritten instructions are as expected.
>>
>> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> Thank you for adding this test, one nitpick below.
>
> [...]
>
>> +#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
>> +private(kptr) struct bin_data __kptr * ptr;
>> +
>> +SEC("tc")
>> +int kptr_xchg_inline(void *ctx)
>> +{
>> + void *old;
>> +
>> + old = bpf_kptr_xchg(&ptr, NULL);
>> + if (old)
>> + bpf_obj_drop(old);
>> +
>> + return 0;
>> +}
> This is highly unlikely, but in theory nothing guarantees that LLVM
> would generate code exactly as expected by pattern in test_kptr_xchg_inline().
> It would be more fail-proof to use inline assembly and a naked
> function instead of C code, e.g.:
>
> SEC("tc")
> __naked int kptr_xchg_inline(void)
> {
> asm volatile (
> "r1 = %[ptr] ll;"
> "r2 = 0;"
> "call %[bpf_kptr_xchg];"
> "if r0 == 0 goto 1f;"
> "r1 = r0;"
> "r2 = 0;"
> "call %[bpf_obj_drop_impl];"
> "1:"
> "r0 = 0;"
> "exit;"
> :
> : __imm_addr(ptr),
> __imm(bpf_kptr_xchg),
> __imm(bpf_obj_drop_impl)
> : __clobber_all
> );
> }
>
> /* BTF FUNC records are not generated for kfuncs referenced
> * from inline assembly. These records are necessary for
> * libbpf to link the program. The function below is a hack
> * to ensure that BTF FUNC records are generated.
> */
> void __btf_root(void)
> {
> bpf_obj_drop(NULL);
> }
>
> wdyt?
Sure, will update the C code to the inline assembly in v3. And thanks
for the review and the suggestions.
