Messages in this thread Patch in this message |  | | Subject | Re: [PATCH v2 bpf-next 12/13] bpf: Add tests for new BPF atomic operations | From | Yonghong Song <> | Date | Mon, 30 Nov 2020 19:55:02 -0800 |
| |
On 11/27/20 9:57 AM, Brendan Jackman wrote: > This relies on the work done by Yonghong Song in > https://reviews.llvm.org/D72184 > > Note the hackery in the Makefile that is necessary to avoid breaking > tests for people who haven't yet got a version of Clang supporting > V4. It seems like this hackery ought to be confined to > tools/build/feature - I tried implementing that and found that it > ballooned into an explosion of nightmares at the top of > tools/testing/selftests/bpf/Makefile without actually improving the > clarity of the CLANG_BPF_BUILD_RULE code at all. Hence the simple > $(shell) call... > > Signed-off-by: Brendan Jackman <jackmanb@google.com> > --- > tools/testing/selftests/bpf/Makefile | 12 +- > .../selftests/bpf/prog_tests/atomics_test.c | 329 ++++++++++++++++++ > .../selftests/bpf/progs/atomics_test.c | 124 +++++++ > .../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_sub.c | 44 +++ > .../selftests/bpf/verifier/atomic_xchg.c | 46 +++ > .../selftests/bpf/verifier/atomic_xor.c | 77 ++++ > tools/testing/selftests/bpf/verifier/ctx.c | 2 +- > 11 files changed, 987 insertions(+), 3 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/atomics_test.c > create mode 100644 tools/testing/selftests/bpf/progs/atomics_test.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_sub.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 3d5940cd110d..5eadfd09037d 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -228,6 +228,12 @@ IS_LITTLE_ENDIAN = $(shell $(CC) -dM -E - </dev/null | \ > grep 'define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__') > MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian) > > +# Determine if Clang supports BPF arch v4, and therefore atomics. > +CLANG_SUPPORTS_V4=$(if $(findstring v4,$(shell $(CLANG) --target=bpf -mcpu=? 2>&1)),true,) > +ifeq ($(CLANG_SUPPORTS_V4),true) > + CFLAGS += -DENABLE_ATOMICS_TESTS > +endif > + > CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) > BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \ > -I$(INCLUDE_DIR) -I$(CURDIR) -I$(APIDIR) \ > @@ -250,7 +256,9 @@ define CLANG_BPF_BUILD_RULE > $(call msg,CLNG-LLC,$(TRUNNER_BINARY),$2) > $(Q)($(CLANG) $3 -O2 -target bpf -emit-llvm \ > -c $1 -o - || echo "BPF obj compilation failed") | \ > - $(LLC) -mattr=dwarfris -march=bpf -mcpu=v3 $4 -filetype=obj -o $2 > + $(LLC) -mattr=dwarfris -march=bpf \ > + -mcpu=$(if $(CLANG_SUPPORTS_V4),v4,v3) \ > + $4 -filetype=obj -o $2 > endef > # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32 > define CLANG_NOALU32_BPF_BUILD_RULE > @@ -391,7 +399,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ > $(wildcard progs/btf_dump_test_case_*.c) > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE > -TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) > +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) $(if $(CLANG_SUPPORTS_V4),-DENABLE_ATOMICS_TESTS,)
If the compiler indeed supports cpu v4 (i.e., atomic insns), -DENABLE_ATOMICS_TESTS will be added to TRUNNER_BPF_FLAGS and eventually -DENABLE_ATOMICS_TESTS is also available for no-alu32 test and this will cause compilation error.
I did the following hack to workaround the issue, i.e., only adds the definition to default (alu32) test run.
index 5eadfd09037d..3d1320fd93eb 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -230,9 +230,6 @@ MENDIAN=$(if $(IS_LITTLE_ENDIAN),-mlittle-endian,-mbig-endian)
# Determine if Clang supports BPF arch v4, and therefore atomics. CLANG_SUPPORTS_V4=$(if $(findstring v4,$(shell $(CLANG) --target=bpf -mcpu=? 2>&1)),true,) -ifeq ($(CLANG_SUPPORTS_V4),true) - CFLAGS += -DENABLE_ATOMICS_TESTS -endif
CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG)) BPF_CFLAGS = -g -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \ @@ -255,6 +252,7 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h define CLANG_BPF_BUILD_RULE $(call msg,CLNG-LLC,$(TRUNNER_BINARY),$2) $(Q)($(CLANG) $3 -O2 -target bpf -emit-llvm \ + $(if $(CLANG_SUPPORTS_V4),-DENABLE_ATOMICS_TESTS,) \ -c $1 -o - || echo "BPF obj compilation failed") | \ $(LLC) -mattr=dwarfris -march=bpf \ -mcpu=$(if $(CLANG_SUPPORTS_V4),v4,v3) \ @@ -399,7 +397,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ $(wildcard progs/btf_dump_test_case_*.c) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE -TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) $(if $(CLANG_SUPPORTS_V4),-DENABLE_ATOMICS_TESTS,) +TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) TRUNNER_BPF_LDFLAGS := -mattr=+alu32 $(eval $(call DEFINE_TEST_RUNNER,test_progs))
> TRUNNER_BPF_LDFLAGS := -mattr=+alu32 > $(eval $(call DEFINE_TEST_RUNNER,test_progs)) > > diff --git a/tools/testing/selftests/bpf/prog_tests/atomics_test.c b/tools/testing/selftests/bpf/prog_tests/atomics_test.c > new file mode 100644 > index 000000000000..8ecc0392fdf9 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/atomics_test.c > @@ -0,0 +1,329 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <test_progs.h> > + > +#ifdef ENABLE_ATOMICS_TESTS > + > +#include "atomics_test.skel.h" > + > +static void test_add(void) [...] > + > +#endif /* ENABLE_ATOMICS_TESTS */ > diff --git a/tools/testing/selftests/bpf/progs/atomics_test.c b/tools/testing/selftests/bpf/progs/atomics_test.c > new file mode 100644 > index 000000000000..3139b00937e5 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/atomics_test.c > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > + > +#ifdef ENABLE_ATOMICS_TESTS > + > +__u64 add64_value = 1; > +__u64 add64_result = 0; > +__u32 add32_value = 1; > +__u32 add32_result = 0; > +__u64 add_stack_value_copy = 0; > +__u64 add_stack_result = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(add, int a) > +{ > + __u64 add_stack_value = 1; > + > + add64_result = __sync_fetch_and_add(&add64_value, 2); > + add32_result = __sync_fetch_and_add(&add32_value, 2); > + add_stack_result = __sync_fetch_and_add(&add_stack_value, 2); > + add_stack_value_copy = add_stack_value; > + > + return 0; > +} > + > +__s64 sub64_value = 1; > +__s64 sub64_result = 0; > +__s32 sub32_value = 1; > +__s32 sub32_result = 0; > +__s64 sub_stack_value_copy = 0; > +__s64 sub_stack_result = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(sub, int a) > +{ > + __u64 sub_stack_value = 1; > + > + sub64_result = __sync_fetch_and_sub(&sub64_value, 2); > + sub32_result = __sync_fetch_and_sub(&sub32_value, 2); > + sub_stack_result = __sync_fetch_and_sub(&sub_stack_value, 2); > + sub_stack_value_copy = sub_stack_value; > + > + return 0; > +} > + > +__u64 and64_value = (0x110ull << 32); > +__u64 and64_result = 0; > +__u32 and32_value = 0x110; > +__u32 and32_result = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(and, int a) > +{ > + > + and64_result = __sync_fetch_and_and(&and64_value, 0x011ull << 32); > + and32_result = __sync_fetch_and_and(&and32_value, 0x011); > + > + return 0; > +} > + > +__u64 or64_value = (0x110ull << 32); > +__u64 or64_result = 0; > +__u32 or32_value = 0x110; > +__u32 or32_result = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(or, int a) > +{ > + or64_result = __sync_fetch_and_or(&or64_value, 0x011ull << 32); > + or32_result = __sync_fetch_and_or(&or32_value, 0x011); > + > + return 0; > +} > + > +__u64 xor64_value = (0x110ull << 32); > +__u64 xor64_result = 0; > +__u32 xor32_value = 0x110; > +__u32 xor32_result = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(xor, int a) > +{ > + xor64_result = __sync_fetch_and_xor(&xor64_value, 0x011ull << 32); > + xor32_result = __sync_fetch_and_xor(&xor32_value, 0x011); > + > + return 0; > +}
All above __sync_fetch_and_{add, sub, and, or, xor} produces a return value used later. To test atomic_<op> instructions, it will be good if you can add some tests which ignores the return value. > + > +__u64 cmpxchg64_value = 1; > +__u64 cmpxchg64_result_fail = 0; > +__u64 cmpxchg64_result_succeed = 0; > +__u32 cmpxchg32_value = 1; > +__u32 cmpxchg32_result_fail = 0; > +__u32 cmpxchg32_result_succeed = 0; > +SEC("fentry/bpf_fentry_test1") > +int BPF_PROG(cmpxchg, int a) > +{ > + cmpxchg64_result_fail = __sync_val_compare_and_swap( > + &cmpxchg64_value, 0, 3); > + cmpxchg64_result_succeed = __sync_val_compare_and_swap( > + &cmpxchg64_value, 1, 2); > + > + cmpxchg32_result_fail = __sync_val_compare_and_swap( > + &cmpxchg32_value, 0, 3); > + cmpxchg32_result_succeed = __sync_val_compare_and_swap( > + &cmpxchg32_value, 1, 2); > + > + return 0; > +} > + > +__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) > +{ > + __u64 val64 = 2; > + __u32 val32 = 2; > + > + __atomic_exchange(&xchg64_value, &val64, &xchg64_result, __ATOMIC_RELAXED); > + __atomic_exchange(&xchg32_value, &val32, &xchg32_result, __ATOMIC_RELAXED); > + > + return 0; > +} > + > +#endif /* ENABLE_ATOMICS_TESTS */ [...]
|  |