Re: [PATCH bpf-next v4 10/11] bpf: Add tests for new BPF atomic operations

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

 





On 12/7/20 8:07 AM, Brendan Jackman wrote:
The prog_test that's added depends on Clang/LLVM features added by
Yonghong in commit 286daafd6512 (was https://reviews.llvm.org/D72184 ).

Note the use of a define called ENABLE_ATOMICS_TESTS: this is used
to:

  - Avoid breaking the build for people on old versions of Clang
  - Avoid needing separate lists of test objects for no_alu32, where
    atomics are not supported even if Clang has the feature.

The atomics_test.o BPF object is built unconditionally both for
test_progs and test_progs-no_alu32. For test_progs, if Clang supports
atomics, ENABLE_ATOMICS_TESTS is defined, so it includes the proper
test code. Otherwise, progs and global vars are defined anyway, as
stubs; this means that the skeleton user code still builds.

The atomics_test.o userspace object is built once and used for both
test_progs and test_progs-no_alu32. A variable called skip_tests is
defined in the BPF object's data section, which tells the userspace
object whether to skip the atomics test.

Signed-off-by: Brendan Jackman <jackmanb@xxxxxxxxxx>

Ack with minor comments below.

Acked-by: Yonghong Song <yhs@xxxxxx>

---
  tools/testing/selftests/bpf/Makefile          |  10 +
  .../selftests/bpf/prog_tests/atomics.c        | 246 ++++++++++++++++++
  tools/testing/selftests/bpf/progs/atomics.c   | 154 +++++++++++
  .../selftests/bpf/verifier/atomic_and.c       |  77 ++++++
  .../selftests/bpf/verifier/atomic_cmpxchg.c   |  96 +++++++
  .../selftests/bpf/verifier/atomic_fetch_add.c | 106 ++++++++
  .../selftests/bpf/verifier/atomic_or.c        |  77 ++++++
  .../selftests/bpf/verifier/atomic_xchg.c      |  46 ++++
  .../selftests/bpf/verifier/atomic_xor.c       |  77 ++++++
  9 files changed, 889 insertions(+)
  create mode 100644 tools/testing/selftests/bpf/prog_tests/atomics.c
  create mode 100644 tools/testing/selftests/bpf/progs/atomics.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_and.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_cmpxchg.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_fetch_add.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_or.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_xchg.c
  create mode 100644 tools/testing/selftests/bpf/verifier/atomic_xor.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ac25ba5d0d6c..13bc1d736164 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -239,6 +239,12 @@ BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) 			\
  	     -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR)			\
  	     -I$(abspath $(OUTPUT)/../usr/include)
+# BPF atomics support was added to Clang in llvm-project commit 286daafd6512
+# (release 12.0.0).
+BPF_ATOMICS_SUPPORTED = $(shell \
+	echo "int x = 0; int foo(void) { return __sync_val_compare_and_swap(&x, 1, 2); }" \
+	| $(CLANG) -x cpp-output -S -target bpf -mcpu=v3 - -o /dev/null && echo 1 || echo 0)

'-x c' here more intuitive?

+
  CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
  	       -Wno-compare-distinct-pointer-types
@@ -399,11 +405,15 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
  		       $(wildcard progs/btf_dump_test_case_*.c)
  TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
  TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
+ifeq ($(BPF_ATOMICS_SUPPORTED),1)
+  TRUNNER_BPF_CFLAGS += -DENABLE_ATOMICS_TESTS
+endif
  TRUNNER_BPF_LDFLAGS := -mattr=+alu32
  $(eval $(call DEFINE_TEST_RUNNER,test_progs))
# Define test_progs-no_alu32 test runner.
  TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE
+TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
  TRUNNER_BPF_LDFLAGS :=
  $(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32))
diff --git a/tools/testing/selftests/bpf/prog_tests/atomics.c b/tools/testing/selftests/bpf/prog_tests/atomics.c
new file mode 100644
index 000000000000..c841a3abc2f7
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/atomics.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#include "atomics.skel.h"
+
+static void test_add(struct atomics *skel)
+{
+	int err, prog_fd;
+	__u32 duration = 0, retval;
+	struct bpf_link *link;
+
+	link = bpf_program__attach(skel->progs.add);
+	if (CHECK(IS_ERR(link), "attach(add)", "err: %ld\n", PTR_ERR(link)))
+		return;
+
+	prog_fd = bpf_program__fd(skel->progs.add);
+	err = bpf_prog_test_run(prog_fd, 1, NULL, 0,
+				NULL, NULL, &retval, &duration);
+	if (CHECK(err || retval, "test_run add",
+		  "err %d errno %d retval %d duration %d\n", err, errno, retval, duration))
+		goto cleanup;
+
+	ASSERT_EQ(skel->data->add64_value, 3, "add64_value");
+	ASSERT_EQ(skel->bss->add64_result, 1, "add64_result");
+
+	ASSERT_EQ(skel->data->add32_value, 3, "add32_value");
+	ASSERT_EQ(skel->bss->add32_result, 1, "add32_result");
+
+	ASSERT_EQ(skel->bss->add_stack_value_copy, 3, "add_stack_value");
+	ASSERT_EQ(skel->bss->add_stack_result, 1, "add_stack_result");
+
+	ASSERT_EQ(skel->data->add_noreturn_value, 3, "add_noreturn_value");
+
+cleanup:
+	bpf_link__destroy(link);
+}
+
[...]
+
+__u64 xchg64_value = 1;
+__u64 xchg64_result = 0;
+__u32 xchg32_value = 1;
+__u32 xchg32_result = 0;
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(xchg, int a)
+{
+#ifdef ENABLE_ATOMICS_TESTS
+	__u64 val64 = 2;
+	__u32 val32 = 2;
+
+	__atomic_exchange(&xchg64_value, &val64, &xchg64_result, __ATOMIC_RELAXED);
+	__atomic_exchange(&xchg32_value, &val32, &xchg32_result, __ATOMIC_RELAXED);

Interesting to see this also works. I guess we probably won't advertise this, right? Currently for LLVM, the memory ordering parameter is ignored.

+#endif
+
+	return 0;
+}
[...]



[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