Skip to content

Commit ebf7f6f

Browse files
seehearfeelborkmann
seehearfeel
authored and
borkmann
committed
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 and thus confusing at 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 would still be good to change the value of MAX_TAIL_CALL_CNT from 32 to 33 to make the code more readable. 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 mips64 JIT, use "ori" instead of "addiu" as suggested by Johan Almbladh. For the riscv JIT, use RV_REG_TCC directly to save one register move as suggested by Björn Töpel. 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]> Signed-off-by: Daniel Borkmann <[email protected]> Tested-by: Johan Almbladh <[email protected]> Tested-by: Ilya Leoshkevich <[email protected]> Acked-by: Björn Töpel <[email protected]> Acked-by: Johan Almbladh <[email protected]> Acked-by: Ilya Leoshkevich <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent e12cd15 commit ebf7f6f

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);

‎arch/x86/net/bpf_jit_comp.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ static void emit_indirect_jump(u8 **pprog, int reg, u8 *ip)
412412
* ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index) ...
413413
* if (index >= array->map.max_entries)
414414
* goto out;
415-
* if (++tail_call_cnt > MAX_TAIL_CALL_CNT)
415+
* if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
416416
* goto out;
417417
* prog = array->ptrs[index];
418418
* if (prog == NULL)
@@ -446,14 +446,14 @@ static void emit_bpf_tail_call_indirect(u8 **pprog, bool *callee_regs_used,
446446
EMIT2(X86_JBE, offset); /* jbe out */
447447

448448
/*
449-
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
449+
* if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
450450
* goto out;
451451
*/
452452
EMIT2_off32(0x8B, 0x85, tcc_off); /* mov eax, dword ptr [rbp - tcc_off] */
453453
EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT); /* cmp eax, MAX_TAIL_CALL_CNT */
454454

455455
offset = ctx->tail_call_indirect_label - (prog + 2 - start);
456-
EMIT2(X86_JA, offset); /* ja out */
456+
EMIT2(X86_JAE, offset); /* jae out */
457457
EMIT3(0x83, 0xC0, 0x01); /* add eax, 1 */
458458
EMIT2_off32(0x89, 0x85, tcc_off); /* mov dword ptr [rbp - tcc_off], eax */
459459

@@ -504,14 +504,14 @@ static void emit_bpf_tail_call_direct(struct bpf_jit_poke_descriptor *poke,
504504
int offset;
505505

506506
/*
507-
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
507+
* if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
508508
* goto out;
509509
*/
510510
EMIT2_off32(0x8B, 0x85, tcc_off); /* mov eax, dword ptr [rbp - tcc_off] */
511511
EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT); /* cmp eax, MAX_TAIL_CALL_CNT */
512512

513513
offset = ctx->tail_call_direct_label - (prog + 2 - start);
514-
EMIT2(X86_JA, offset); /* ja out */
514+
EMIT2(X86_JAE, offset); /* jae out */
515515
EMIT3(0x83, 0xC0, 0x01); /* add eax, 1 */
516516
EMIT2_off32(0x89, 0x85, tcc_off); /* mov dword ptr [rbp - tcc_off], eax */
517517

‎arch/x86/net/bpf_jit_comp32.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
13231323
EMIT2(IA32_JBE, jmp_label(jmp_label1, 2));
13241324

13251325
/*
1326-
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
1326+
* if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
13271327
* goto out;
13281328
*/
13291329
lo = (u32)MAX_TAIL_CALL_CNT;
@@ -1337,7 +1337,7 @@ static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
13371337
/* cmp ecx,lo */
13381338
EMIT3(0x83, add_1reg(0xF8, IA32_ECX), lo);
13391339

1340-
/* ja out */
1340+
/* jae out */
13411341
EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
13421342

13431343
/* add eax,0x1 */

‎include/linux/bpf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,7 @@ struct bpf_array {
10811081
};
10821082

10831083
#define BPF_COMPLEXITY_LIMIT_INSNS 1000000 /* yes. 1M insns */
1084-
#define MAX_TAIL_CALL_CNT 32
1084+
#define MAX_TAIL_CALL_CNT 33
10851085

10861086
#define BPF_F_ACCESS_MASK (BPF_F_RDONLY | \
10871087
BPF_F_RDONLY_PROG | \

‎include/uapi/linux/bpf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1744,7 +1744,7 @@ union bpf_attr {
17441744
* if the maximum number of tail calls has been reached for this
17451745
* chain of programs. This limit is defined in the kernel by the
17461746
* macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
1747-
* which is currently set to 32.
1747+
* which is currently set to 33.
17481748
* Return
17491749
* 0 on success, or a negative error in case of failure.
17501750
*

‎kernel/bpf/core.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1574,7 +1574,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
15741574

15751575
if (unlikely(index >= array->map.max_entries))
15761576
goto out;
1577-
if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
1577+
1578+
if (unlikely(tail_call_cnt >= MAX_TAIL_CALL_CNT))
15781579
goto out;
15791580

15801581
tail_call_cnt++;

‎lib/test_bpf.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14683,7 +14683,7 @@ static struct tail_call_test tail_call_tests[] = {
1468314683
BPF_EXIT_INSN(),
1468414684
},
1468514685
.flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
14686-
.result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
14686+
.result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
1468714687
},
1468814688
{
1468914689
"Tail call count preserved across function calls",
@@ -14705,7 +14705,7 @@ static struct tail_call_test tail_call_tests[] = {
1470514705
},
1470614706
.stack_depth = 8,
1470714707
.flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
14708-
.result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
14708+
.result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
1470914709
},
1471014710
{
1471114711
"Tail call error path, NULL target",

‎tools/include/uapi/linux/bpf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1744,7 +1744,7 @@ union bpf_attr {
17441744
* if the maximum number of tail calls has been reached for this
17451745
* chain of programs. This limit is defined in the kernel by the
17461746
* macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
1747-
* which is currently set to 32.
1747+
* which is currently set to 33.
17481748
* Return
17491749
* 0 on success, or a negative error in case of failure.
17501750
*

0 commit comments

Comments
 (0)