On Tue, Jun 13, 2023 at 06:42 AM -07, Yonghong Song wrote:

On 6/13/23 1:50 AM, baomingtong001@208suo.com wrote:

...

Fix the following coccicheck warning: tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c:28:14-17: Unneeded variable: "ret". Return "1". Signed-off-by: Mingtong Bao baomingtong001@208suo.com

---

tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c | 3 +--  1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c index 4a9f63bea66c..7f0146682577 100644 --- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c +++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf6.c @@ -25,10 +25,9 @@ static __noinline  int subprog_tail(struct __sk_buff *skb)  {      /* Don't propagate the constant to the caller */

• volatile int ret = 1;

bpf_tail_call_static(skb, &jmp_table, 0);

• return ret;

• return 1;

Please pay attention to the comment: /* Don't propagate the constant to the caller */ which clearly says 'constant' is not preferred.

The patch introduced this change is: 5e0b0a4c52d30 selftests/bpf: Test tail call counting with bpf2bpf and data on stack

The test intentionally want to: 'Specifically when the size of data allocated on BPF stack is not a multiple on 8.'

Note that with volatile and without volatile, the generated code will be different and it will result in different verification path.

cc Jakub for further clarification.

Yonghong is right. We can't replace it like that.

Compiler is smart and pull up the constant into subprog_tail() caller.

And it doesn't have the slightest idea that bpf_tail_call_static() is actually tail call (destroy frame + jump) and control doesn't return to subprog_tail().

(You can read more about BPF tail calls in [1] and [2] if they are not familiar.)

IOW, we need an r0 store to happen after a call to BPF tail call helper (call 12) to remain in subprog_tail body for the regression test to work:

$ llvm-objdump -d --no-show-raw-insn tailcall_bpf2bpf6.bpf.o

tailcall_bpf2bpf6.bpf.o: file format elf64-bpf

Disassembly of section .text:

0000000000000000 <subprog_tail>: 0: r6 = r1 1: w1 = 1 2: *(u32 *)(r10 - 4) = r1 3: r7 = 0 ll 5: r1 = r6 6: r2 = r7 7: r3 = 0 8: call 12 9: r0 = *(u32 *)(r10 - 4) <-- this must stay 10: exit

You could take a shot at replacing it with inline asm, if you want.

[1] https://docs.cilium.io/en/stable/bpf/architecture/#bpf-to-bpf-calls [2] https://blog.cloudflare.com/assembly-within-bpf-tail-calls-on-x86-and-arm/