Re: [PATCH bpf-next 7/7] x86: bpf: Fix comments on tail call count limiting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/9/21 11:34 AM, Johan Almbladh wrote:
Before, the comments in the 32-bit eBPF JIT claimed that up to
MAX_TAIL_CALL_CNT + 1 tail calls were allowed, when in fact the
implementation was using the correct limit of MAX_TAIL_CALL_CNT.
Now, the comments are in line with what the code actually does.

Signed-off-by: Johan Almbladh <johan.almbladh@xxxxxxxxxxxxxxxxx>
---
  arch/x86/net/bpf_jit_comp32.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index 3bfda5f502cb..8db9ab11abda 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -1272,7 +1272,7 @@ static void emit_epilogue(u8 **pprog, u32 stack_depth)
   * ... 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)
@@ -1307,7 +1307,7 @@ static void emit_bpf_tail_call(u8 **pprog)
  	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;
@@ -1321,7 +1321,7 @@ static void emit_bpf_tail_call(u8 **pprog)
  	/* cmp ecx,lo */
  	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), lo);
- /* ja out */
+	/* jae out */
  	EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));

You have me confused here ... b61a28cf11d6 ("bpf: Fix off-by-one in tail call count
limiting") from bpf-next says '[interpreter is now] in line with the behavior of the
x86 JITs'. From the latter I assumed you implicitly refer to x86-64. Which one did you
test specifically wrt the prior statement? It looks like x86-64 vs x86-32 differ:

  [...]
  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 */
  EMIT2(X86_JA, OFFSET2);                   /* ja out */
  EMIT3(0x83, 0xC0, 0x01);                  /* add eax, 1 */
  EMIT2_off32(0x89, 0x85, tcc_off);         /* mov dword ptr [rbp - tcc_off], eax */
  [...]

So it's ja vs jae ... unless I need more coffee? ;)

  	/* add eax,0x1 */





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux