Thread (15 messages) flat view 15 messages, 2 authors, 2019-11-20

Re: [PATCH bpf-next 8/8] bpf, testing: add various tail call test cases

From: Andrii Nakryiko <hidden>
Date: 2019-11-20 19:27:04
Also in: bpf

On Mon, Nov 18, 2019 at 5:38 PM Daniel Borkmann [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add several BPF kselftest cases for tail calls which test the various
patch directions (NOP -> JMP, JMP -> JMP, JMP -> NOP), and that multiple
locations are patched.

  # ./test_progs -n 44
  #44 tailcalls:OK
  Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../selftests/bpf/prog_tests/tailcalls.c      | 210 ++++++++++++++++++
 tools/testing/selftests/bpf/progs/tailcall1.c |  48 ++++
 tools/testing/selftests/bpf/progs/tailcall2.c |  59 +++++
 3 files changed, 317 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tailcalls.c
 create mode 100644 tools/testing/selftests/bpf/progs/tailcall1.c
 create mode 100644 tools/testing/selftests/bpf/progs/tailcall2.c
diff --git a/tools/testing/selftests/bpf/prog_tests/tailcalls.c b/tools/testing/selftests/bpf/prog_tests/tailcalls.c
new file mode 100644
index 000000000000..6862bb5f9688
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+static void test_tailcall_1(void)
+{
+       int err, map_fd, prog_fd, main_fd, i, j;
+       struct bpf_map *prog_array;
+       struct bpf_program *prog;
+       struct bpf_object *obj;
+       __u32 retval, duration;
+       char prog_name[32];
+       char buff[128] = {};
+
+       err = bpf_prog_load("tailcall1.o", BPF_PROG_TYPE_SCHED_CLS, &obj,
+                           &prog_fd);
+       if (CHECK_FAIL(err))
+               return;
+
+       prog = bpf_object__find_program_by_title(obj, "classifier");
+       if (CHECK_FAIL(!prog))
+               goto out;
+
+       main_fd = bpf_program__fd(prog);
+       if (CHECK_FAIL(main_fd < 0))
+               goto out;

can this happen if bpf_prog_load succeeded? same for a bunch of
prog_fd checks below.
+
+       prog_array = bpf_object__find_map_by_name(obj, "jmp_table");
+       if (CHECK_FAIL(!prog_array))
+               goto out;
+
[...]
+
+       for (i = 0; i < bpf_map__def(prog_array)->max_entries; i++) {
+               err = bpf_prog_test_run(main_fd, 1, buff, sizeof(buff), 0,
+                                       &duration, &retval, NULL);
+               CHECK(err || retval != i, "tailcall",
+                     "err %d errno %d retval %d\n", err, errno, retval);
+
+               err = bpf_map_delete_elem(map_fd, &i);
+               if (CHECK_FAIL(err))
+                       goto out;
+       }
+
+       /* Testing JMP -> NOP */
nit: this comment should probably go before previous loop?
+       err = bpf_prog_test_run(main_fd, 1, buff, sizeof(buff), 0,
+                               &duration, &retval, NULL);
+       CHECK(err || retval != 3, "tailcall", "err %d errno %d retval %d\n",
+             err, errno, retval);
+
[...]
+       for (i = 0; i < bpf_map__def(prog_array)->max_entries; i++) {
+               j = bpf_map__def(prog_array)->max_entries - 1 - i;
+
+               err = bpf_prog_test_run(main_fd, 1, buff, sizeof(buff), 0,
+                                       &duration, &retval, NULL);
+               CHECK(err || retval != j, "tailcall",
+                     "err %d errno %d retval %d\n", err, errno, retval);
+
+               err = bpf_map_delete_elem(map_fd, &i);
in addition to explicit delete, can you test update to NULL? Also, I
think it might be useful to validate update from NULL to NULL (it's a
separate check in your poke_run code).
+               if (CHECK_FAIL(err))
+                       goto out;
+       }
+
+       err = bpf_prog_test_run(main_fd, 1, buff, sizeof(buff), 0,
+                               &duration, &retval, NULL);
+       CHECK(err || retval != 3, "tailcall", "err %d errno %d retval %d\n",
+             err, errno, retval);
+out:
+       bpf_object__close(obj);
+}
+
[...]
+void test_tailcalls(void)
+{
+       test_tailcall_1();
+       test_tailcall_2();
+}
these could be sub-tests:


if (test__start_subtest("tailcall_1"))
    test_tailcall_1();
if (test__start_subtest("tailcall_2"))
    test_tailcall_2();

though, a bit more descriptive names would be certainly better :)
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/progs/tailcall1.c b/tools/testing/selftests/bpf/progs/tailcall1.c
new file mode 100644
index 000000000000..63531e1a9fa4
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/tailcall1.c
@@ -0,0 +1,48 @@
[...]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help