diff mbox series

[bpf-next,v5] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33

Message ID 1635990610-4448-1-git-send-email-yangtiezhu@loongson.cn (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next,v5] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33 | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 11541 this patch: 11541
netdev/cc_maintainers warning 32 maintainers not CCed: mpe@ellerman.id.au linux-mips@vger.kernel.org paulus@samba.org aou@eecs.berkeley.edu benh@kernel.crashing.org liuhangbin@gmail.com brouer@redhat.com linux-arm-kernel@lists.infradead.org linux-s390@vger.kernel.org dave.hansen@linux.intel.com paulburton@kernel.org paul.walmsley@sifive.com linuxppc-dev@lists.ozlabs.org joe@cilium.io linux@armlinux.org.uk palmer@dabbelt.com borntraeger@de.ibm.com tsbogend@alpha.franken.de bp@alien8.de tglx@linutronix.de yoshfuji@linux-ipv6.org agordeev@linux.ibm.com mingo@redhat.com dsahern@kernel.org catalin.marinas@arm.com sparclinux@vger.kernel.org revest@chromium.org x86@kernel.org linux-riscv@lists.infradead.org jackmanb@google.com will@kernel.org hpa@zytor.com
netdev/build_clang success Errors and warnings before: 2378 this patch: 2378
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 11262 this patch: 11262
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next success VM_Test

Commit Message

Tiezhu Yang Nov. 4, 2021, 1:50 a.m. UTC
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 04fd61ab36ec ("bpf: allow
bpf programs to tail-call other bpf programs") and commit f9dabe016b63
("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 874be05f525e ("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 18935a72eb25
("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.

With this patch, it does not change the current limit 33, MAX_TAIL_CALL_CNT
can reflect the actual max tail call count, the related tailcall testcases
in test_bpf 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 <yangtiezhu@loongson.cn>
Acked-by: Björn Töpel <bjorn@kernel.org>
---

v5: Use RV_REG_TCC directly to save one move for RISC-V,
    suggested by Björn Töpel, thank you.

v4: Use >= as check condition,
    suggested by Daniel Borkmann, thank you.

 arch/arm/net/bpf_jit_32.c         |  5 +++--
 arch/arm64/net/bpf_jit_comp.c     |  5 +++--
 arch/mips/net/bpf_jit_comp32.c    |  2 +-
 arch/mips/net/bpf_jit_comp64.c    |  2 +-
 arch/powerpc/net/bpf_jit_comp32.c |  4 ++--
 arch/powerpc/net/bpf_jit_comp64.c |  4 ++--
 arch/riscv/net/bpf_jit_comp32.c   |  6 ++----
 arch/riscv/net/bpf_jit_comp64.c   |  7 +++----
 arch/s390/net/bpf_jit_comp.c      |  6 +++---
 arch/sparc/net/bpf_jit_comp_64.c  |  2 +-
 arch/x86/net/bpf_jit_comp.c       | 10 +++++-----
 arch/x86/net/bpf_jit_comp32.c     |  4 ++--
 include/linux/bpf.h               |  2 +-
 include/uapi/linux/bpf.h          |  2 +-
 kernel/bpf/core.c                 |  3 ++-
 lib/test_bpf.c                    |  4 ++--
 tools/include/uapi/linux/bpf.h    |  2 +-
 17 files changed, 35 insertions(+), 35 deletions(-)

Comments

Johan Almbladh Nov. 4, 2021, 12:31 p.m. UTC | #1
On Thu, Nov 4, 2021 at 2:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> 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 04fd61ab36ec ("bpf: allow
> bpf programs to tail-call other bpf programs") and commit f9dabe016b63
> ("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 874be05f525e ("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 18935a72eb25
> ("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.
>
> With this patch, it does not change the current limit 33, MAX_TAIL_CALL_CNT
> can reflect the actual max tail call count, the related tailcall testcases
> in test_bpf 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 <yangtiezhu@loongson.cn>
> Acked-by: Björn Töpel <bjorn@kernel.org>

I have run the tail call test suite of test_bpf.ko in QEMU with JITs
for x86, x86-64, mips, mipsel, mips64, mips64el, arm, arm64, s390x,
powerpc, powerpc64 and sparc64, i.e. all JITs in this patch except
RISC-V.
Tested-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>

> ---
>
> v5: Use RV_REG_TCC directly to save one move for RISC-V,
>     suggested by Björn Töpel, thank you.
>
> v4: Use >= as check condition,
>     suggested by Daniel Borkmann, thank you.
>
>  arch/arm/net/bpf_jit_32.c         |  5 +++--
>  arch/arm64/net/bpf_jit_comp.c     |  5 +++--
>  arch/mips/net/bpf_jit_comp32.c    |  2 +-
>  arch/mips/net/bpf_jit_comp64.c    |  2 +-
>  arch/powerpc/net/bpf_jit_comp32.c |  4 ++--
>  arch/powerpc/net/bpf_jit_comp64.c |  4 ++--
>  arch/riscv/net/bpf_jit_comp32.c   |  6 ++----
>  arch/riscv/net/bpf_jit_comp64.c   |  7 +++----
>  arch/s390/net/bpf_jit_comp.c      |  6 +++---
>  arch/sparc/net/bpf_jit_comp_64.c  |  2 +-
>  arch/x86/net/bpf_jit_comp.c       | 10 +++++-----
>  arch/x86/net/bpf_jit_comp32.c     |  4 ++--
>  include/linux/bpf.h               |  2 +-
>  include/uapi/linux/bpf.h          |  2 +-
>  kernel/bpf/core.c                 |  3 ++-
>  lib/test_bpf.c                    |  4 ++--
>  tools/include/uapi/linux/bpf.h    |  2 +-
>  17 files changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index eeb6dc0..e59b41e 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -1199,7 +1199,8 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
>
>         /* tmp2[0] = array, tmp2[1] = index */
>
> -       /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +       /*
> +        * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
>          *      goto out;
>          * tail_call_cnt++;
>          */
> @@ -1208,7 +1209,7 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
>         tc = arm_bpf_get_reg64(tcc, tmp, ctx);
>         emit(ARM_CMP_I(tc[0], hi), ctx);
>         _emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
> -       _emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
> +       _emit(ARM_COND_CS, ARM_B(jmp_offset), ctx);
>         emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
>         emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
>         arm_bpf_put_reg64(tcc, tmp, ctx);
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 3a8a714..356fb21 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -287,13 +287,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
>         emit(A64_CMP(0, r3, tmp), ctx);
>         emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
>
> -       /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +       /*
> +        * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
>          *     goto out;
>          * tail_call_cnt++;
>          */
>         emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
>         emit(A64_CMP(1, tcc, tmp), ctx);
> -       emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
> +       emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
>         emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
>
>         /* prog = array->ptrs[index];
> diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
> index bd996ed..a7b424f 100644
> --- a/arch/mips/net/bpf_jit_comp32.c
> +++ b/arch/mips/net/bpf_jit_comp32.c
> @@ -1382,7 +1382,7 @@ void build_prologue(struct jit_context *ctx)
>          * calling function jumps into the prologue after these instructions.
>          */
>         emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO,
> -            min(MAX_TAIL_CALL_CNT + 1, 0xffff));
> +            min(MAX_TAIL_CALL_CNT, 0xffff));
>         emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP);

Good, this is the intended way to adjust the tail call limit to 33 for
the MIPS JITs.

>
>         /*
> diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
> index 815ade7..5ef8778 100644
> --- a/arch/mips/net/bpf_jit_comp64.c
> +++ b/arch/mips/net/bpf_jit_comp64.c
> @@ -552,7 +552,7 @@ void build_prologue(struct jit_context *ctx)
>          * On a tail call, the calling function jumps into the prologue
>          * after this instruction.
>          */
> -       emit(ctx, addiu, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT + 1, 0xffff));
> +       emit(ctx, addiu, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));

One minor thing that I would like to fix here. Please change the above
to use "ori" instead of "addiu" so it is consistent with the 32-bit
JIT:

emit(ctx, ori, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));

For MIPS, with this change:
Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>

>
>         /* === Entry-point for tail calls === */
>
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index 0da31d4..8a4faa0 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -221,13 +221,13 @@ static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
>         PPC_BCC(COND_GE, out);
>
>         /*
> -        * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +        * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
>          *   goto out;
>          */
>         EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CALL_CNT));
>         /* tail_call_cnt++; */
>         EMIT(PPC_RAW_ADDIC(_R0, _R0, 1));
> -       PPC_BCC(COND_GT, out);
> +       PPC_BCC(COND_GE, out);
>
>         /* prog = array->ptrs[index]; */
>         EMIT(PPC_RAW_RLWINM(_R3, b2p_index, 2, 0, 29));
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 8b5157c..8571aaf 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -228,12 +228,12 @@ static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
>         PPC_BCC(COND_GE, out);
>
>         /*
> -        * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +        * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
>          *   goto out;
>          */
>         PPC_BPF_LL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
>         EMIT(PPC_RAW_CMPLWI(b2p[TMP_REG_1], MAX_TAIL_CALL_CNT));
> -       PPC_BCC(COND_GT, out);
> +       PPC_BCC(COND_GE, out);
>
>         /*
>          * tail_call_cnt++;
> diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
> index e649742..529a83b 100644
> --- a/arch/riscv/net/bpf_jit_comp32.c
> +++ b/arch/riscv/net/bpf_jit_comp32.c
> @@ -799,11 +799,10 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
>         emit_bcc(BPF_JGE, lo(idx_reg), RV_REG_T1, off, ctx);
>
>         /*
> -        * temp_tcc = tcc - 1;
> -        * if (tcc < 0)
> +        * if (--tcc < 0)
>          *   goto out;
>          */
> -       emit(rv_addi(RV_REG_T1, RV_REG_TCC, -1), ctx);
> +       emit(rv_addi(RV_REG_TCC, RV_REG_TCC, -1), ctx);
>         off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
>         emit_bcc(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
>
> @@ -829,7 +828,6 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
>         if (is_12b_check(off, insn))
>                 return -1;
>         emit(rv_lw(RV_REG_T0, off, RV_REG_T0), ctx);
> -       emit(rv_addi(RV_REG_TCC, RV_REG_T1, 0), ctx);
>         /* Epilogue jumps to *(t0 + 4). */
>         __build_epilogue(true, ctx);
>         return 0;
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 2ca345c..f4466b7 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -327,12 +327,12 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
>         off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
>         emit_branch(BPF_JGE, RV_REG_A2, RV_REG_T1, off, ctx);
>
> -       /* if (TCC-- < 0)
> +       /* if (--TCC < 0)
>          *     goto out;
>          */
> -       emit_addi(RV_REG_T1, tcc, -1, ctx);
> +       emit_addi(RV_REG_TCC, tcc, -1, ctx);
>         off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
> -       emit_branch(BPF_JSLT, tcc, RV_REG_ZERO, off, ctx);
> +       emit_branch(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
>
>         /* prog = array->ptrs[index];
>          * if (!prog)
> @@ -352,7 +352,6 @@ static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
>         if (is_12b_check(off, insn))
>                 return -1;
>         emit_ld(RV_REG_T3, off, RV_REG_T2, ctx);
> -       emit_mv(RV_REG_TCC, RV_REG_T1, ctx);
>         __build_epilogue(true, ctx);
>         return 0;
>  }
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index 1a374d0..3553cfc 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -1369,7 +1369,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
>                                  jit->prg);
>
>                 /*
> -                * if (tail_call_cnt++ > MAX_TAIL_CALL_CNT)
> +                * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>                  *         goto out;
>                  */
>
> @@ -1381,9 +1381,9 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
>                 EMIT4_IMM(0xa7080000, REG_W0, 1);
>                 /* laal %w1,%w0,off(%r15) */
>                 EMIT6_DISP_LH(0xeb000000, 0x00fa, REG_W1, REG_W0, REG_15, off);
> -               /* clij %w1,MAX_TAIL_CALL_CNT,0x2,out */
> +               /* clij %w1,MAX_TAIL_CALL_CNT-1,0x2,out */
>                 patch_2_clij = jit->prg;
> -               EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT,
> +               EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT - 1,
>                                  2, jit->prg);
>
>                 /*
> diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
> index 9a2f20c..0bfe1c7 100644
> --- a/arch/sparc/net/bpf_jit_comp_64.c
> +++ b/arch/sparc/net/bpf_jit_comp_64.c
> @@ -867,7 +867,7 @@ static void emit_tail_call(struct jit_ctx *ctx)
>         emit(LD32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
>         emit_cmpi(tmp, MAX_TAIL_CALL_CNT, ctx);
>  #define OFFSET2 13
> -       emit_branch(BGU, ctx->idx, ctx->idx + OFFSET2, ctx);
> +       emit_branch(BGEU, ctx->idx, ctx->idx + OFFSET2, ctx);
>         emit_nop(ctx);
>
>         emit_alu_K(ADD, tmp, 1, ctx);
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 726700f..6318479 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -412,7 +412,7 @@ static void emit_indirect_jump(u8 **pprog, int reg, u8 *ip)
>   * ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index) ...
>   *   if (index >= array->map.max_entries)
>   *     goto out;
> - *   if (++tail_call_cnt > MAX_TAIL_CALL_CNT)
> + *   if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>   *     goto out;
>   *   prog = array->ptrs[index];
>   *   if (prog == NULL)
> @@ -446,14 +446,14 @@ static void emit_bpf_tail_call_indirect(u8 **pprog, bool *callee_regs_used,
>         EMIT2(X86_JBE, offset);                   /* jbe out */
>
>         /*
> -        * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +        * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>          *      goto out;
>          */
>         EMIT2_off32(0x8B, 0x85, tcc_off);         /* mov eax, dword ptr [rbp - tcc_off] */
>         EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT);     /* cmp eax, MAX_TAIL_CALL_CNT */
>
>         offset = ctx->tail_call_indirect_label - (prog + 2 - start);
> -       EMIT2(X86_JA, offset);                    /* ja out */
> +       EMIT2(X86_JAE, offset);                   /* jae out */
>         EMIT3(0x83, 0xC0, 0x01);                  /* add eax, 1 */
>         EMIT2_off32(0x89, 0x85, tcc_off);         /* mov dword ptr [rbp - tcc_off], eax */
>
> @@ -504,14 +504,14 @@ static void emit_bpf_tail_call_direct(struct bpf_jit_poke_descriptor *poke,
>         int offset;
>
>         /*
> -        * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +        * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>          *      goto out;
>          */
>         EMIT2_off32(0x8B, 0x85, tcc_off);             /* mov eax, dword ptr [rbp - tcc_off] */
>         EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT);         /* cmp eax, MAX_TAIL_CALL_CNT */
>
>         offset = ctx->tail_call_direct_label - (prog + 2 - start);
> -       EMIT2(X86_JA, offset);                        /* ja out */
> +       EMIT2(X86_JAE, offset);                       /* jae out */
>         EMIT3(0x83, 0xC0, 0x01);                      /* add eax, 1 */
>         EMIT2_off32(0x89, 0x85, tcc_off);             /* mov dword ptr [rbp - tcc_off], eax */
>
> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index da9b7cf..429a89c 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -1323,7 +1323,7 @@ static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
>         EMIT2(IA32_JBE, jmp_label(jmp_label1, 2));
>
>         /*
> -        * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
> +        * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>          *     goto out;
>          */
>         lo = (u32)MAX_TAIL_CALL_CNT;
> @@ -1337,7 +1337,7 @@ static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
>         /* cmp ecx,lo */
>         EMIT3(0x83, add_1reg(0xF8, IA32_ECX), lo);
>
> -       /* ja out */
> +       /* jae out */
>         EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
>
>         /* add eax,0x1 */

So the 32-bit x86 JIT was still using a limit of 32.

For the other implementations, the patch is a non-functional change.
Here, the patch corrects the comments AND corrects the behaviour as
the constant MAX_TAIL_CALL_CNT updated. The 32-bit x86 is now aligned
to 33 as well. I think this deserves being mentioned in the commit
message too.

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 2be6dfd..4bce7ee 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1075,7 +1075,7 @@ struct bpf_array {
>  };
>
>  #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
> -#define MAX_TAIL_CALL_CNT 32
> +#define MAX_TAIL_CALL_CNT 33
>
>  #define BPF_F_ACCESS_MASK      (BPF_F_RDONLY |         \
>                                  BPF_F_RDONLY_PROG |    \
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index ba5af15..b12cfce 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1744,7 +1744,7 @@ union bpf_attr {
>   *             if the maximum number of tail calls has been reached for this
>   *             chain of programs. This limit is defined in the kernel by the
>   *             macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
> - *             which is currently set to 32.
> + *             which is currently set to 33.
>   *     Return
>   *             0 on success, or a negative error in case of failure.
>   *
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 327e399..870881d 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1567,7 +1567,8 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
>
>                 if (unlikely(index >= array->map.max_entries))
>                         goto out;
> -               if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
> +
> +               if (unlikely(tail_call_cnt >= MAX_TAIL_CALL_CNT))
>                         goto out;
>
>                 tail_call_cnt++;
> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> index adae395..0c5cb2d 100644
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c
> @@ -14683,7 +14683,7 @@ static struct tail_call_test tail_call_tests[] = {
>                         BPF_EXIT_INSN(),
>                 },
>                 .flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
> -               .result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
> +               .result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
>         },
>         {
>                 "Tail call count preserved across function calls",
> @@ -14705,7 +14705,7 @@ static struct tail_call_test tail_call_tests[] = {
>                 },
>                 .stack_depth = 8,
>                 .flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
> -               .result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
> +               .result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
>         },
>         {
>                 "Tail call error path, NULL target",
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index ba5af15..b12cfce 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1744,7 +1744,7 @@ union bpf_attr {
>   *             if the maximum number of tail calls has been reached for this
>   *             chain of programs. This limit is defined in the kernel by the
>   *             macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
> - *             which is currently set to 32.
> + *             which is currently set to 33.
>   *     Return
>   *             0 on success, or a negative error in case of failure.
>   *
> --
> 2.1.0
>
Ilya Leoshkevich Nov. 4, 2021, 1:36 p.m. UTC | #2
On Thu, 2021-11-04 at 09:50 +0800, Tiezhu Yang wrote:
> 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 04fd61ab36ec ("bpf:
> allow
> bpf programs to tail-call other bpf programs") and commit
> f9dabe016b63
> ("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 874be05f525e ("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
> 18935a72eb25
> ("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.
> 
> With this patch, it does not change the current limit 33,
> MAX_TAIL_CALL_CNT
> can reflect the actual max tail call count, the related tailcall
> testcases
> in test_bpf 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 <yangtiezhu@loongson.cn>
> Acked-by: Björn Töpel <bjorn@kernel.org>
> ---
> 
> v5: Use RV_REG_TCC directly to save one move for RISC-V,
>     suggested by Björn Töpel, thank you.
> 
> v4: Use >= as check condition,
>     suggested by Daniel Borkmann, thank you.
> 
>  arch/arm/net/bpf_jit_32.c         |  5 +++--
>  arch/arm64/net/bpf_jit_comp.c     |  5 +++--
>  arch/mips/net/bpf_jit_comp32.c    |  2 +-
>  arch/mips/net/bpf_jit_comp64.c    |  2 +-
>  arch/powerpc/net/bpf_jit_comp32.c |  4 ++--
>  arch/powerpc/net/bpf_jit_comp64.c |  4 ++--
>  arch/riscv/net/bpf_jit_comp32.c   |  6 ++----
>  arch/riscv/net/bpf_jit_comp64.c   |  7 +++----
>  arch/s390/net/bpf_jit_comp.c      |  6 +++---
>  arch/sparc/net/bpf_jit_comp_64.c  |  2 +-
>  arch/x86/net/bpf_jit_comp.c       | 10 +++++-----
>  arch/x86/net/bpf_jit_comp32.c     |  4 ++--
>  include/linux/bpf.h               |  2 +-
>  include/uapi/linux/bpf.h          |  2 +-
>  kernel/bpf/core.c                 |  3 ++-
>  lib/test_bpf.c                    |  4 ++--
>  tools/include/uapi/linux/bpf.h    |  2 +-
>  17 files changed, 35 insertions(+), 35 deletions(-)

[...]

> diff --git a/arch/s390/net/bpf_jit_comp.c
> b/arch/s390/net/bpf_jit_comp.c
> index 1a374d0..3553cfc 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -1369,7 +1369,7 @@ static noinline int bpf_jit_insn(struct bpf_jit
> *jit, struct bpf_prog *fp,
>                                  jit->prg);
>  
>                 /*
> -                * if (tail_call_cnt++ > MAX_TAIL_CALL_CNT)
> +                * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
>                  *         goto out;
>                  */
>  
> @@ -1381,9 +1381,9 @@ static noinline int bpf_jit_insn(struct bpf_jit
> *jit, struct bpf_prog *fp,
>                 EMIT4_IMM(0xa7080000, REG_W0, 1);
>                 /* laal %w1,%w0,off(%r15) */
>                 EMIT6_DISP_LH(0xeb000000, 0x00fa, REG_W1, REG_W0,
> REG_15, off);
> -               /* clij %w1,MAX_TAIL_CALL_CNT,0x2,out */
> +               /* clij %w1,MAX_TAIL_CALL_CNT-1,0x2,out */
>                 patch_2_clij = jit->prg;
> -               EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1,
> MAX_TAIL_CALL_CNT,
> +               EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1,
> MAX_TAIL_CALL_CNT - 1,
>                                  2, jit->prg);
>  
>                 /*

For the s390 part:

Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>

[...]
diff mbox series

Patch

diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index eeb6dc0..e59b41e 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1199,7 +1199,8 @@  static int emit_bpf_tail_call(struct jit_ctx *ctx)
 
 	/* tmp2[0] = array, tmp2[1] = index */
 
-	/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	/*
+	 * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
 	 *	goto out;
 	 * tail_call_cnt++;
 	 */
@@ -1208,7 +1209,7 @@  static int emit_bpf_tail_call(struct jit_ctx *ctx)
 	tc = arm_bpf_get_reg64(tcc, tmp, ctx);
 	emit(ARM_CMP_I(tc[0], hi), ctx);
 	_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
-	_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
+	_emit(ARM_COND_CS, ARM_B(jmp_offset), ctx);
 	emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
 	emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
 	arm_bpf_put_reg64(tcc, tmp, ctx);
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 3a8a714..356fb21 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -287,13 +287,14 @@  static int emit_bpf_tail_call(struct jit_ctx *ctx)
 	emit(A64_CMP(0, r3, tmp), ctx);
 	emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
 
-	/* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	/*
+	 * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
 	 *     goto out;
 	 * tail_call_cnt++;
 	 */
 	emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
 	emit(A64_CMP(1, tcc, tmp), ctx);
-	emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
+	emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
 	emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
 
 	/* prog = array->ptrs[index];
diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
index bd996ed..a7b424f 100644
--- a/arch/mips/net/bpf_jit_comp32.c
+++ b/arch/mips/net/bpf_jit_comp32.c
@@ -1382,7 +1382,7 @@  void build_prologue(struct jit_context *ctx)
 	 * calling function jumps into the prologue after these instructions.
 	 */
 	emit(ctx, ori, MIPS_R_T9, MIPS_R_ZERO,
-	     min(MAX_TAIL_CALL_CNT + 1, 0xffff));
+	     min(MAX_TAIL_CALL_CNT, 0xffff));
 	emit(ctx, sw, MIPS_R_T9, 0, MIPS_R_SP);
 
 	/*
diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
index 815ade7..5ef8778 100644
--- a/arch/mips/net/bpf_jit_comp64.c
+++ b/arch/mips/net/bpf_jit_comp64.c
@@ -552,7 +552,7 @@  void build_prologue(struct jit_context *ctx)
 	 * On a tail call, the calling function jumps into the prologue
 	 * after this instruction.
 	 */
-	emit(ctx, addiu, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT + 1, 0xffff));
+	emit(ctx, addiu, tc, MIPS_R_ZERO, min(MAX_TAIL_CALL_CNT, 0xffff));
 
 	/* === Entry-point for tail calls === */
 
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index 0da31d4..8a4faa0 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -221,13 +221,13 @@  static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
 	PPC_BCC(COND_GE, out);
 
 	/*
-	 * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	 * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
 	 *   goto out;
 	 */
 	EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CALL_CNT));
 	/* tail_call_cnt++; */
 	EMIT(PPC_RAW_ADDIC(_R0, _R0, 1));
-	PPC_BCC(COND_GT, out);
+	PPC_BCC(COND_GE, out);
 
 	/* prog = array->ptrs[index]; */
 	EMIT(PPC_RAW_RLWINM(_R3, b2p_index, 2, 0, 29));
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 8b5157c..8571aaf 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -228,12 +228,12 @@  static int bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 o
 	PPC_BCC(COND_GE, out);
 
 	/*
-	 * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	 * if (tail_call_cnt >= MAX_TAIL_CALL_CNT)
 	 *   goto out;
 	 */
 	PPC_BPF_LL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
 	EMIT(PPC_RAW_CMPLWI(b2p[TMP_REG_1], MAX_TAIL_CALL_CNT));
-	PPC_BCC(COND_GT, out);
+	PPC_BCC(COND_GE, out);
 
 	/*
 	 * tail_call_cnt++;
diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
index e649742..529a83b 100644
--- a/arch/riscv/net/bpf_jit_comp32.c
+++ b/arch/riscv/net/bpf_jit_comp32.c
@@ -799,11 +799,10 @@  static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
 	emit_bcc(BPF_JGE, lo(idx_reg), RV_REG_T1, off, ctx);
 
 	/*
-	 * temp_tcc = tcc - 1;
-	 * if (tcc < 0)
+	 * if (--tcc < 0)
 	 *   goto out;
 	 */
-	emit(rv_addi(RV_REG_T1, RV_REG_TCC, -1), ctx);
+	emit(rv_addi(RV_REG_TCC, RV_REG_TCC, -1), ctx);
 	off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
 	emit_bcc(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
 
@@ -829,7 +828,6 @@  static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
 	if (is_12b_check(off, insn))
 		return -1;
 	emit(rv_lw(RV_REG_T0, off, RV_REG_T0), ctx);
-	emit(rv_addi(RV_REG_TCC, RV_REG_T1, 0), ctx);
 	/* Epilogue jumps to *(t0 + 4). */
 	__build_epilogue(true, ctx);
 	return 0;
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 2ca345c..f4466b7 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -327,12 +327,12 @@  static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
 	off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
 	emit_branch(BPF_JGE, RV_REG_A2, RV_REG_T1, off, ctx);
 
-	/* if (TCC-- < 0)
+	/* if (--TCC < 0)
 	 *     goto out;
 	 */
-	emit_addi(RV_REG_T1, tcc, -1, ctx);
+	emit_addi(RV_REG_TCC, tcc, -1, ctx);
 	off = ninsns_rvoff(tc_ninsn - (ctx->ninsns - start_insn));
-	emit_branch(BPF_JSLT, tcc, RV_REG_ZERO, off, ctx);
+	emit_branch(BPF_JSLT, RV_REG_TCC, RV_REG_ZERO, off, ctx);
 
 	/* prog = array->ptrs[index];
 	 * if (!prog)
@@ -352,7 +352,6 @@  static int emit_bpf_tail_call(int insn, struct rv_jit_context *ctx)
 	if (is_12b_check(off, insn))
 		return -1;
 	emit_ld(RV_REG_T3, off, RV_REG_T2, ctx);
-	emit_mv(RV_REG_TCC, RV_REG_T1, ctx);
 	__build_epilogue(true, ctx);
 	return 0;
 }
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 1a374d0..3553cfc 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1369,7 +1369,7 @@  static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
 				 jit->prg);
 
 		/*
-		 * if (tail_call_cnt++ > MAX_TAIL_CALL_CNT)
+		 * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
 		 *         goto out;
 		 */
 
@@ -1381,9 +1381,9 @@  static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
 		EMIT4_IMM(0xa7080000, REG_W0, 1);
 		/* laal %w1,%w0,off(%r15) */
 		EMIT6_DISP_LH(0xeb000000, 0x00fa, REG_W1, REG_W0, REG_15, off);
-		/* clij %w1,MAX_TAIL_CALL_CNT,0x2,out */
+		/* clij %w1,MAX_TAIL_CALL_CNT-1,0x2,out */
 		patch_2_clij = jit->prg;
-		EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT,
+		EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1, MAX_TAIL_CALL_CNT - 1,
 				 2, jit->prg);
 
 		/*
diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index 9a2f20c..0bfe1c7 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -867,7 +867,7 @@  static void emit_tail_call(struct jit_ctx *ctx)
 	emit(LD32 | IMMED | RS1(SP) | S13(off) | RD(tmp), ctx);
 	emit_cmpi(tmp, MAX_TAIL_CALL_CNT, ctx);
 #define OFFSET2 13
-	emit_branch(BGU, ctx->idx, ctx->idx + OFFSET2, ctx);
+	emit_branch(BGEU, ctx->idx, ctx->idx + OFFSET2, ctx);
 	emit_nop(ctx);
 
 	emit_alu_K(ADD, tmp, 1, ctx);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 726700f..6318479 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -412,7 +412,7 @@  static void emit_indirect_jump(u8 **pprog, int reg, u8 *ip)
  * ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index) ...
  *   if (index >= array->map.max_entries)
  *     goto out;
- *   if (++tail_call_cnt > MAX_TAIL_CALL_CNT)
+ *   if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
  *     goto out;
  *   prog = array->ptrs[index];
  *   if (prog == NULL)
@@ -446,14 +446,14 @@  static void emit_bpf_tail_call_indirect(u8 **pprog, bool *callee_regs_used,
 	EMIT2(X86_JBE, offset);                   /* jbe out */
 
 	/*
-	 * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	 * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
 	 *	goto out;
 	 */
 	EMIT2_off32(0x8B, 0x85, tcc_off);         /* mov eax, dword ptr [rbp - tcc_off] */
 	EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT);     /* cmp eax, MAX_TAIL_CALL_CNT */
 
 	offset = ctx->tail_call_indirect_label - (prog + 2 - start);
-	EMIT2(X86_JA, offset);                    /* ja out */
+	EMIT2(X86_JAE, offset);                   /* jae out */
 	EMIT3(0x83, 0xC0, 0x01);                  /* add eax, 1 */
 	EMIT2_off32(0x89, 0x85, tcc_off);         /* mov dword ptr [rbp - tcc_off], eax */
 
@@ -504,14 +504,14 @@  static void emit_bpf_tail_call_direct(struct bpf_jit_poke_descriptor *poke,
 	int offset;
 
 	/*
-	 * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	 * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
 	 *	goto out;
 	 */
 	EMIT2_off32(0x8B, 0x85, tcc_off);             /* mov eax, dword ptr [rbp - tcc_off] */
 	EMIT3(0x83, 0xF8, MAX_TAIL_CALL_CNT);         /* cmp eax, MAX_TAIL_CALL_CNT */
 
 	offset = ctx->tail_call_direct_label - (prog + 2 - start);
-	EMIT2(X86_JA, offset);                        /* ja out */
+	EMIT2(X86_JAE, offset);                       /* jae out */
 	EMIT3(0x83, 0xC0, 0x01);                      /* add eax, 1 */
 	EMIT2_off32(0x89, 0x85, tcc_off);             /* mov dword ptr [rbp - tcc_off], eax */
 
diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index da9b7cf..429a89c 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -1323,7 +1323,7 @@  static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
 	EMIT2(IA32_JBE, jmp_label(jmp_label1, 2));
 
 	/*
-	 * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+	 * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
 	 *     goto out;
 	 */
 	lo = (u32)MAX_TAIL_CALL_CNT;
@@ -1337,7 +1337,7 @@  static void emit_bpf_tail_call(u8 **pprog, u8 *ip)
 	/* cmp ecx,lo */
 	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), lo);
 
-	/* ja out */
+	/* jae out */
 	EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
 
 	/* add eax,0x1 */
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2be6dfd..4bce7ee 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1075,7 +1075,7 @@  struct bpf_array {
 };
 
 #define BPF_COMPLEXITY_LIMIT_INSNS      1000000 /* yes. 1M insns */
-#define MAX_TAIL_CALL_CNT 32
+#define MAX_TAIL_CALL_CNT 33
 
 #define BPF_F_ACCESS_MASK	(BPF_F_RDONLY |		\
 				 BPF_F_RDONLY_PROG |	\
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba5af15..b12cfce 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1744,7 +1744,7 @@  union bpf_attr {
  * 		if the maximum number of tail calls has been reached for this
  * 		chain of programs. This limit is defined in the kernel by the
  * 		macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
- * 		which is currently set to 32.
+ *		which is currently set to 33.
  * 	Return
  * 		0 on success, or a negative error in case of failure.
  *
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 327e399..870881d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1567,7 +1567,8 @@  static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn)
 
 		if (unlikely(index >= array->map.max_entries))
 			goto out;
-		if (unlikely(tail_call_cnt > MAX_TAIL_CALL_CNT))
+
+		if (unlikely(tail_call_cnt >= MAX_TAIL_CALL_CNT))
 			goto out;
 
 		tail_call_cnt++;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index adae395..0c5cb2d 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -14683,7 +14683,7 @@  static struct tail_call_test tail_call_tests[] = {
 			BPF_EXIT_INSN(),
 		},
 		.flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
-		.result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
+		.result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
 	},
 	{
 		"Tail call count preserved across function calls",
@@ -14705,7 +14705,7 @@  static struct tail_call_test tail_call_tests[] = {
 		},
 		.stack_depth = 8,
 		.flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
-		.result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,
+		.result = (MAX_TAIL_CALL_CNT + 1) * MAX_TESTRUNS,
 	},
 	{
 		"Tail call error path, NULL target",
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index ba5af15..b12cfce 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1744,7 +1744,7 @@  union bpf_attr {
  * 		if the maximum number of tail calls has been reached for this
  * 		chain of programs. This limit is defined in the kernel by the
  * 		macro **MAX_TAIL_CALL_CNT** (not accessible to user space),
- * 		which is currently set to 32.
+ *		which is currently set to 33.
  * 	Return
  * 		0 on success, or a negative error in case of failure.
  *