Re: [PATCH bpf v4 13/14] bpf/tests: Fix error in tail call limit tests

On Tue, Sep 14, 2021 at 2:55 PM Tiezhu Yang [off-list ref] wrote:

On 09/14/2021 08:41 PM, Tiezhu Yang wrote:

quoted

On 09/14/2021 05:18 PM, Johan Almbladh wrote:

quoted

This patch fixes an error in the tail call limit test that caused the
test to fail on for x86-64 JIT. Previously, the register R0 was used to
report the total number of tail calls made. However, after a tail call
fall-through, the value of the R0 register is undefined. Now, all tail
call error path tests instead use context state to store the count.

Fixes: 874be05f525e ("bpf, tests: Add tail call test suite")
Reported-by: Paul Chaignon <redacted>
Reported-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Johan Almbladh <johan.almbladh@anyfinetworks.com>
---
  lib/test_bpf.c | 37 +++++++++++++++++++++++++++----------
  1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 7475abfd2186..ddb9a8089d2e 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c

@@ -12179,10 +12179,15 @@ static __init int test_bpf(void)
  struct tail_call_test {
      const char *descr;
      struct bpf_insn insns[MAX_INSNS];
+    int flags;
      int result;
      int stack_depth;
  };
  +/* Flags that can be passed to tail call test cases */
+#define FLAG_NEED_STATE        BIT(0)
+#define FLAG_RESULT_IN_STATE    BIT(1)
+
  /*
   * Magic marker used in test snippets for tail calls below.
   * BPF_LD/MOV to R2 and R2 with this immediate value is replaced

@@ -12252,32 +12257,38 @@ static struct tail_call_test

tail_call_tests[] = {
      {
          "Tail call error path, max count reached",
          .insns = {
-            BPF_ALU64_IMM(BPF_ADD, R1, 1),
-            BPF_ALU64_REG(BPF_MOV, R0, R1),
+            BPF_LDX_MEM(BPF_W, R2, R1, 0),
+            BPF_ALU64_IMM(BPF_ADD, R2, 1),
+            BPF_STX_MEM(BPF_W, R1, R2, 0),
              TAIL_CALL(0),
              BPF_EXIT_INSN(),
          },
-        .result = MAX_TAIL_CALL_CNT + 1,
+        .flags = FLAG_NEED_STATE | FLAG_RESULT_IN_STATE,
+        .result = (MAX_TAIL_CALL_CNT + 1 + 1) * MAX_TESTRUNS,

Hi Johan,

I have tested this patch,
It should be "MAX_TAIL_CALL_CNT + 1" instead of "MAX_TAIL_CALL_CNT + 1
+ 1"?

Oh, sorry, it is right when MAX_TAIL_CALL_CNT is 32,
I have tested it based on MAX_TAIL_CALL_CNT is 33,
so I need to modify here if MAX_TAIL_CALL_CNT is 33 in my v3 patch.

No worries! I wrote it that way to indicate that there are two +1s.
The first is from the behaviour that actual count (33) = configured
count (32) + 1. The second is for the initial BPF program call, which
increments the counter but is not in itself a tail call.

Tested-by: Tiezhu Yang <yangtiezhu@loongson.cn>

Thanks!
Johan

quoted

[...]

Thanks,
Tiezhu