Skip to content

Commit eb5f484

Browse files
seehearfeelNobody
authored andcommitted
bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
In the current code, the actual max tail call count is 33 which is greater than MAX_TAIL_CALL_CNT (defined as 32), the actual limit is not consistent with the meaning of MAX_TAIL_CALL_CNT, there is some confusion and need to spend some time to think about the reason at the first glance. We can see the historical evolution from commit 04fd61a ("bpf: allow bpf programs to tail-call other bpf programs") and commit f9dabe0 ("bpf: Undo off-by-one in interpreter tail call count limit"). In order to avoid changing existing behavior, the actual limit is 33 now, this is reasonable. After commit 874be05 ("bpf, tests: Add tail call test suite"), we can see there exists failed testcase. On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set: # echo 0 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf # dmesg | grep -w FAIL Tail call error path, max count reached jited:0 ret 34 != 33 FAIL On some archs: # echo 1 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf # dmesg | grep -w FAIL Tail call error path, max count reached jited:1 ret 34 != 33 FAIL Although the above failed testcase has been fixed in commit 18935a7 ("bpf/tests: Fix error in tail call limit tests"), it is still necessary to change the value of MAX_TAIL_CALL_CNT from 32 to 33 to make the code more readable, then do some small changes of the related code. The 32-bit x86 JIT was using a limit of 32, just fix the wrong comments and limit to 33 tail calls as the constant MAX_TAIL_CALL_CNT updated. For the other implementations, no function changes, it does not change the current limit 33, the new value of MAX_TAIL_CALL_CNT can reflect the actual max tail call count, the related tail call testcases in test_bpf module and selftests can work well for the interpreter and the JIT. Here are the test results on x86_64: # uname -m x86_64 # echo 0 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf test_suite=test_tail_calls # dmesg | tail -1 test_bpf: test_tail_calls: Summary: 8 PASSED, 0 FAILED, [0/8 JIT'ed] # rmmod test_bpf # echo 1 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf test_suite=test_tail_calls # dmesg | tail -1 test_bpf: test_tail_calls: Summary: 8 PASSED, 0 FAILED, [8/8 JIT'ed] # rmmod test_bpf # ./test_progs -t tailcalls #142 tailcalls:OK Summary: 1/11 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Tiezhu Yang <[email protected]> Acked-by: Björn Töpel <[email protected]> Tested-by: Johan Almbladh <[email protected]> Acked-by: Johan Almbladh <[email protected]> Tested-by: Ilya Leoshkevich <[email protected]> Acked-by: Ilya Leoshkevich <[email protected]>
1 parent ad6d780 commit eb5f484

File tree

17 files changed

+35
-36
lines changed

17 files changed

+35
-36
lines changed

arch/arm/net/bpf_jit_32.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,8 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
11991199

12001200
/* tmp2[0] = array, tmp2[1] = index */
12011201

1202-
/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
1202+
/*
1203+
* if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
12031204
* goto out;
12041205
* tail_call_cnt++;
12051206
*/
@@ -1208,7 +1209,7 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
12081209
tc = arm_bpf_get_reg64(tcc, tmp, ctx);
12091210
emit(ARM_CMP_I(tc[0], hi), ctx);
12101211
_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
1211-
_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
1212+
_emit(ARM_COND_CS, ARM_B(jmp_offset), ctx);
12121213
emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
12131214
emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
12141215
arm_bpf_put_reg64(tcc, tmp, ctx);

arch/arm64/net/bpf_jit_comp.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
287287
emit(A64_CMP(0, r3, tmp), ctx);
288288
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
289289

290-
/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
290+
/*
291+
* if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
291292
* goto out;
292293
* tail_call_cnt++;
293294
*/
294295
emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
295296
emit(A64_CMP(1, tcc, tmp), ctx);
296-
emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
297+
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
297298
emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
298299

299300
/* prog = array->ptrs[index];

arch/mips/net/bpf_jit_comp32.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,8 +1381,7 @@ void build_prologue(struct jit_context *ctx)
13811381
* 16-byte area in the parent's stack frame. On a tail call, the
13821382
* calling function jumps into the prologue after these instructions.
13831383
*/
1384-
emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO,
1385-
min(MAX_TAIL_CALL_CNT + 1, 0xffff));
1384+
emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));
13861385
emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP);
13871386

13881387
/*

arch/mips/net/bpf_jit_comp64.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx)
552552
* On a tail call, the calling function jumps into the prologue
553553
* after this instruction.
554554
*/
555-
emit(ctx, addiu, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT + 1, 0xffff));
555+
emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));
556556

557557
/* === Entry-point for tail calls === */
558558

arch/powerpc/net/bpf_jit_comp32.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,13 +221,13 @@ static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
221221
PPC_BCC(COND_GE, out);
222222

223223
/*
224-
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
224+
* if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
225225
* goto out;
226226
*/
227227
EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CALL_CNT));
228228
/* tail_call_cnt++; */
229229
EMIT(PPC_RAW_ADDIC(_R0, _R0, 1));
230-
PPC_BCC(COND_GT, out);
230+
PPC_BCC(COND_GE, out);
231231

232232
/* prog = array->ptrs[index]; */
233233
EMIT(PPC_RAW_RLWINM(_R3, b2p_index, 2, 0, 29));

arch/powerpc/net/bpf_jit_comp64.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,12 +228,12 @@ static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
228228
PPC_BCC(COND_GE, out);
229229

230230
/*
231-
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
231+
* if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
232232
* goto out;
233233
*/
234234
PPC_BPF_LL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
235235
EMIT(PPC_RAW_CMPLWI(b2p[TMP_REG_1], MAX_TAIL_CALL_CNT));
236-
PPC_BCC(COND_GT, out);
236+
PPC_BCC(COND_GE, out);
237237

238238
/*
239239
* tail_call_cnt++;

arch/riscv/net/bpf_jit_comp32.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -799,11 +799,10 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
799799
emit_bcc(BPF_JGE, lo(idx_reg), RV_REG_T1, off, ctx);
800800

801801
/*
802-
* temp_tcc = tcc - 1;
803-
* if (tcc < 0)
802+
* if (--tcc < 0)
804803
* goto out;
805804
*/
806-
emit(rv_addi(RV_REG_T1, RV_REG_TCC, -1), ctx);
805+
emit(rv_addi(RV_REG_TCC, RV_REG_TCC, -1), ctx);
807806
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
808807
emit_bcc(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
809808

@@ -829,7 +828,6 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
829828
if (is_12b_check(off, insn))
830829
return -1;
831830
emit(rv_lw(RV_REG_T0, off, RV_REG_T0), ctx);
832-
emit(rv_addi(RV_REG_TCC, RV_REG_T1, 0), ctx);
833831
/* Epilogue jumps to *(t0 + 4). */
834832
__build_epilogue(true, ctx);
835833
return 0;

arch/riscv/net/bpf_jit_comp64.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,12 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
327327
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
328328
emit_branch(BPF_JGE, RV_REG_A2, RV_REG_T1, off, ctx);
329329

330-
/* if (TCC-- < 0)
330+
/* if (--TCC < 0)
331331
* goto out;
332332
*/
333-
emit_addi(RV_REG_T1, tcc, -1, ctx);
333+
emit_addi(RV_REG_TCC, tcc, -1, ctx);
334334
off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
335-
emit_branch(BPF_JSLT, tcc, RV_REG_ZERO, off, ctx);
335+
emit_branch(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
336336

337337
/* prog = array->ptrs[index];
338338
* if (!prog)
@@ -352,7 +352,6 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
352352
if (is_12b_check(off, insn))
353353
return -1;
354354
emit_ld(RV_REG_T3, off, RV_REG_T2, ctx);
355-
emit_mv(RV_REG_TCC, RV_REG_T1, ctx);
356355
__build_epilogue(true, ctx);
357356
return 0;
358357
}

arch/s390/net/bpf_jit_comp.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
13691369
jit->prg);
13701370

13711371
/*
1372-
* if (tail_call_cnt++ > MAX_TAIL_CALL_CNT)
1372+
* if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
13731373
* goto out;
13741374
*/
13751375

@@ -1381,9 +1381,9 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
13811381
EMIT4_IMM(0xa7080000, REG_W0, 1);
13821382
/* laal %w1,%w0,off(%r15) */
13831383
EMIT6_DISP_LH(0xeb000000, 0x00fa, REG_W1, REG_W0, REG_15, off);
1384-
/* clij %w1,MAX_TAIL_CALL_CNT,0x2,out */
1384+
/* clij %w1,MAX_TAIL_CALL_CNT-1,0x2,out */
13851385
patch_2_clij = jit->prg;
1386-
EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT,
1386+
EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT - 1,
13871387
2, jit->prg);
13881388

13891389
/*

arch/sparc/net/bpf_jit_comp_64.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ static void emit_tail_call(struct jit_ctx *ctx)
867867
emit(LD32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
868868
emit_cmpi(tmp, MAX_TAIL_CALL_CNT, ctx);
869869
#define OFFSET2 13
870-
emit_branch(BGU, ctx->idx, ctx->idx + OFFSET2, ctx);
870+
emit_branch(BGEU, ctx->idx, ctx->idx + OFFSET2, ctx);
871871
emit_nop(ctx);
872872

873873
emit_alu_K(ADD, tmp, 1, ctx);

0 commit comments

Comments
 (0)