OSDN Git Service

selftests/bpf: test_progs: remove global fail/success counts
authorStanislav Fomichev <sdf@google.com>
Wed, 21 Aug 2019 23:44:25 +0000 (16:44 -0700)
committerDaniel Borkmann <daniel@iogearbox.net>
Tue, 27 Aug 2019 22:35:40 +0000 (00:35 +0200)
Now that we have a global per-test/per-environment state, there
is no longer need to have global fail/success counters (and there
is no need to save/get the diff before/after the test).

Introduce CHECK_FAIL macro (suggested by Andrii) and covert existing tests
to it. CHECK_FAIL uses new test__fail() to record the failure.

Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
22 files changed:
tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
tools/testing/selftests/bpf/prog_tests/flow_dissector.c
tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
tools/testing/selftests/bpf/prog_tests/global_data.c
tools/testing/selftests/bpf/prog_tests/l4lb_all.c
tools/testing/selftests/bpf/prog_tests/map_lock.c
tools/testing/selftests/bpf/prog_tests/pkt_access.c
tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
tools/testing/selftests/bpf/prog_tests/reference_tracking.c
tools/testing/selftests/bpf/prog_tests/spinlock.c
tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
tools/testing/selftests/bpf/prog_tests/tcp_estats.c
tools/testing/selftests/bpf/prog_tests/xdp.c
tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
tools/testing/selftests/bpf/test_progs.c
tools/testing/selftests/bpf/test_progs.h

index fb5840a..5dd6ca1 100644 (file)
@@ -48,8 +48,7 @@ void test_bpf_obj_id(void)
                /* test_obj_id.o is a dumb prog. It should never fail
                 * to load.
                 */
-               if (err)
-                       error_cnt++;
+               CHECK_FAIL(err);
                assert(!err);
 
                /* Insert a magic value to the map */
index 1a1eae3..1c01ee2 100644 (file)
@@ -28,8 +28,6 @@ static int check_load(const char *file, enum bpf_prog_type type)
        attr.prog_flags = BPF_F_TEST_RND_HI32;
        err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
        bpf_object__close(obj);
-       if (err)
-               error_cnt++;
        return err;
 }
 
@@ -105,12 +103,7 @@ void test_bpf_verif_scale(void)
                        continue;
 
                err = check_load(test->file, test->attach_type);
-               if (test->fails) { /* expected to fail */
-                       if (err)
-                               error_cnt--;
-                       else
-                               error_cnt++;
-               }
+               CHECK_FAIL(err && !test->fails);
        }
 
        if (env.verifier_stats)
index 6892b88..aee0cda 100644 (file)
@@ -452,10 +452,8 @@ void test_flow_dissector(void)
 
        err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector",
                            "jmp_table", "last_dissection", &prog_fd, &keys_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        for (i = 0; i < ARRAY_SIZE(tests); i++) {
                struct bpf_flow_keys flow_keys;
index 3d59b3c..eba9a97 100644 (file)
@@ -135,10 +135,7 @@ void test_get_stack_raw_tp(void)
                exp_cnt -= err;
        }
 
-       goto close_prog_noerr;
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        if (!IS_ERR_OR_NULL(link))
                bpf_link__destroy(link);
        if (!IS_ERR_OR_NULL(pb))
index d011079..c680926 100644 (file)
@@ -7,10 +7,8 @@ static void test_global_data_number(struct bpf_object *obj, __u32 duration)
        uint64_t num;
 
        map_fd = bpf_find_map(__func__, obj, "result_number");
-       if (map_fd < 0) {
-               error_cnt++;
+       if (CHECK_FAIL(map_fd < 0))
                return;
-       }
 
        struct {
                char *name;
@@ -44,10 +42,8 @@ static void test_global_data_string(struct bpf_object *obj, __u32 duration)
        char str[32];
 
        map_fd = bpf_find_map(__func__, obj, "result_string");
-       if (map_fd < 0) {
-               error_cnt++;
+       if (CHECK_FAIL(map_fd < 0))
                return;
-       }
 
        struct {
                char *name;
@@ -81,10 +77,8 @@ static void test_global_data_struct(struct bpf_object *obj, __u32 duration)
        struct foo val;
 
        map_fd = bpf_find_map(__func__, obj, "result_struct");
-       if (map_fd < 0) {
-               error_cnt++;
+       if (CHECK_FAIL(map_fd < 0))
                return;
-       }
 
        struct {
                char *name;
@@ -112,16 +106,12 @@ static void test_global_data_rdonly(struct bpf_object *obj, __u32 duration)
        __u8 *buff;
 
        map = bpf_object__find_map_by_name(obj, "test_glo.rodata");
-       if (!map || !bpf_map__is_internal(map)) {
-               error_cnt++;
+       if (CHECK_FAIL(!map || !bpf_map__is_internal(map)))
                return;
-       }
 
        map_fd = bpf_map__fd(map);
-       if (map_fd < 0) {
-               error_cnt++;
+       if (CHECK_FAIL(map_fd < 0))
                return;
-       }
 
        buff = malloc(bpf_map__def(map)->value_size);
        if (buff)
index 20ddca8..eaf6459 100644 (file)
@@ -30,10 +30,8 @@ static void test_l4lb(const char *file)
        u32 *magic = (u32 *)buf;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        map_fd = bpf_find_map(__func__, obj, "vip_map");
        if (map_fd < 0)
@@ -72,10 +70,9 @@ static void test_l4lb(const char *file)
                bytes += stats[i].bytes;
                pkts += stats[i].pkts;
        }
-       if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
-               error_cnt++;
+       if (CHECK_FAIL(bytes != MAGIC_BYTES * NUM_ITER * 2 ||
+                      pkts != NUM_ITER * 2))
                printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
-       }
 out:
        bpf_object__close(obj);
 }
index ee99368..15993b6 100644 (file)
@@ -8,14 +8,12 @@ static void *parallel_map_access(void *arg)
 
        for (i = 0; i < 10000; i++) {
                err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
-               if (err) {
+               if (CHECK_FAIL(err)) {
                        printf("lookup failed\n");
-                       error_cnt++;
                        goto out;
                }
-               if (vars[0] != 0) {
+               if (CHECK_FAIL(vars[0] != 0)) {
                        printf("lookup #%d var[0]=%d\n", i, vars[0]);
-                       error_cnt++;
                        goto out;
                }
                rnd = vars[1];
@@ -24,7 +22,7 @@ static void *parallel_map_access(void *arg)
                                continue;
                        printf("lookup #%d var[1]=%d var[%d]=%d\n",
                               i, rnd, j, vars[j]);
-                       error_cnt++;
+                       CHECK_FAIL(vars[j] != rnd);
                        goto out;
                }
        }
@@ -42,15 +40,15 @@ void test_map_lock(void)
        void *ret;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
-       if (err) {
+       if (CHECK_FAIL(err)) {
                printf("test_map_lock:bpf_prog_load errno %d\n", errno);
                goto close_prog;
        }
        map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
-       if (map_fd[0] < 0)
+       if (CHECK_FAIL(map_fd[0] < 0))
                goto close_prog;
        map_fd[1] = bpf_find_map(__func__, obj, "array_map");
-       if (map_fd[1] < 0)
+       if (CHECK_FAIL(map_fd[1] < 0))
                goto close_prog;
 
        bpf_map_update_elem(map_fd[0], &key, vars, BPF_F_LOCK);
@@ -67,9 +65,6 @@ void test_map_lock(void)
        for (i = 4; i < 6; i++)
                assert(pthread_join(thread_id[i], &ret) == 0 &&
                       ret == (void *)&map_fd[i - 4]);
-       goto close_prog_noerr;
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        bpf_object__close(obj);
 }
index 4ecfd72..a2537df 100644 (file)
@@ -9,10 +9,8 @@ void test_pkt_access(void)
        int err, prog_fd;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        err = bpf_prog_test_run(prog_fd, 100000, &pkt_v4, sizeof(pkt_v4),
                                NULL, NULL, &retval, &duration);
index ac0d434..5f7aea6 100644 (file)
@@ -9,10 +9,8 @@ void test_pkt_md_access(void)
        int err, prog_fd;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        err = bpf_prog_test_run(prog_fd, 10, &pkt_v4, sizeof(pkt_v4),
                                NULL, NULL, &retval, &duration);
index e60cd5f..faccc66 100644 (file)
@@ -27,10 +27,8 @@ static void test_queue_stack_map_by_type(int type)
                return;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        map_in_fd = bpf_find_map(__func__, obj, "map_in");
        if (map_in_fd < 0)
@@ -43,10 +41,8 @@ static void test_queue_stack_map_by_type(int type)
        /* Push 32 elements to the input map */
        for (i = 0; i < MAP_SIZE; i++) {
                err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
-               if (err) {
-                       error_cnt++;
+               if (CHECK_FAIL(err))
                        goto out;
-               }
        }
 
        /* The eBPF program pushes iph.saddr in the output map,
index 4a4f428..5c78e2b 100644 (file)
@@ -10,10 +10,8 @@ void test_reference_tracking(void)
        int err = 0;
 
        obj = bpf_object__open(file);
-       if (IS_ERR(obj)) {
-               error_cnt++;
+       if (CHECK_FAIL(IS_ERR(obj)))
                return;
-       }
 
        bpf_object__for_each_program(prog, obj) {
                const char *title;
index 114ebe6..d71fb3d 100644 (file)
@@ -11,7 +11,7 @@ void test_spinlock(void)
        void *ret;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
-       if (err) {
+       if (CHECK_FAIL(err)) {
                printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
                goto close_prog;
        }
@@ -21,9 +21,7 @@ void test_spinlock(void)
        for (i = 0; i < 4; i++)
                assert(pthread_join(thread_id[i], &ret) == 0 &&
                       ret == (void *)&prog_fd);
-       goto close_prog_noerr;
+
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        bpf_object__close(obj);
 }
index fc53933..37269d2 100644 (file)
@@ -26,19 +26,19 @@ void test_stacktrace_map(void)
 
        /* find map fds */
        control_map_fd = bpf_find_map(__func__, obj, "control_map");
-       if (control_map_fd < 0)
+       if (CHECK_FAIL(control_map_fd < 0))
                goto disable_pmu;
 
        stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
-       if (stackid_hmap_fd < 0)
+       if (CHECK_FAIL(stackid_hmap_fd < 0))
                goto disable_pmu;
 
        stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
-       if (stackmap_fd < 0)
+       if (CHECK_FAIL(stackmap_fd < 0))
                goto disable_pmu;
 
        stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap");
-       if (stack_amap_fd < 0)
+       if (CHECK_FAIL(stack_amap_fd < 0))
                goto disable_pmu;
 
        /* give some time for bpf program run */
@@ -55,23 +55,20 @@ void test_stacktrace_map(void)
        err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
        if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
                  "err %d errno %d\n", err, errno))
-               goto disable_pmu_noerr;
+               goto disable_pmu;
 
        err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
        if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
                  "err %d errno %d\n", err, errno))
-               goto disable_pmu_noerr;
+               goto disable_pmu;
 
        stack_trace_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
        err = compare_stack_ips(stackmap_fd, stack_amap_fd, stack_trace_len);
        if (CHECK(err, "compare_stack_ips stackmap vs. stack_amap",
                  "err %d errno %d\n", err, errno))
-               goto disable_pmu_noerr;
+               goto disable_pmu;
 
-       goto disable_pmu_noerr;
 disable_pmu:
-       error_cnt++;
-disable_pmu_noerr:
        bpf_link__destroy(link);
 close_prog:
        bpf_object__close(obj);
index fbfa8e7..404a549 100644 (file)
@@ -26,15 +26,15 @@ void test_stacktrace_map_raw_tp(void)
 
        /* find map fds */
        control_map_fd = bpf_find_map(__func__, obj, "control_map");
-       if (control_map_fd < 0)
+       if (CHECK_FAIL(control_map_fd < 0))
                goto close_prog;
 
        stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
-       if (stackid_hmap_fd < 0)
+       if (CHECK_FAIL(stackid_hmap_fd < 0))
                goto close_prog;
 
        stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
-       if (stackmap_fd < 0)
+       if (CHECK_FAIL(stackmap_fd < 0))
                goto close_prog;
 
        /* give some time for bpf program run */
@@ -58,10 +58,7 @@ void test_stacktrace_map_raw_tp(void)
                  "err %d errno %d\n", err, errno))
                goto close_prog;
 
-       goto close_prog_noerr;
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        if (!IS_ERR_OR_NULL(link))
                bpf_link__destroy(link);
        bpf_object__close(obj);
index 958a3d8..1bdc1d8 100644 (file)
@@ -70,9 +70,6 @@ void test_task_fd_query_rawtp(void)
        if (CHECK(!err, "check_results", "fd_type %d len %u\n", fd_type, len))
                goto close_prog;
 
-       goto close_prog_noerr;
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        bpf_object__close(obj);
 }
index f9b70e8..3f131b8 100644 (file)
@@ -62,14 +62,9 @@ static void test_task_fd_query_tp_core(const char *probe_name,
                  fd_type, buf))
                goto close_pmu;
 
-       close(pmu_fd);
-       goto close_prog_noerr;
-
 close_pmu:
        close(pmu_fd);
 close_prog:
-       error_cnt++;
-close_prog_noerr:
        bpf_object__close(obj);
 }
 
index bb8759d..594307d 100644 (file)
@@ -10,10 +10,8 @@ void test_tcp_estats(void)
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
        CHECK(err, "", "err %d errno %d\n", err, errno);
-       if (err) {
-               error_cnt++;
+       if (err)
                return;
-       }
 
        bpf_object__close(obj);
 }
index a741672..dcb5eca 100644 (file)
@@ -16,10 +16,8 @@ void test_xdp(void)
        int err, prog_fd, map_fd;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        map_fd = bpf_find_map(__func__, obj, "vip2tnl");
        if (map_fd < 0)
index 922aa0a..3744196 100644 (file)
@@ -10,10 +10,8 @@ void test_xdp_adjust_tail(void)
        int err, prog_fd;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
                                buf, &size, &retval, &duration);
index 15f7c27..c9404e6 100644 (file)
@@ -31,10 +31,8 @@ void test_xdp_noinline(void)
        u32 *magic = (u32 *)buf;
 
        err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
-       if (err) {
-               error_cnt++;
+       if (CHECK_FAIL(err))
                return;
-       }
 
        map_fd = bpf_find_map(__func__, obj, "vip_map");
        if (map_fd < 0)
@@ -73,8 +71,8 @@ void test_xdp_noinline(void)
                bytes += stats[i].bytes;
                pkts += stats[i].pkts;
        }
-       if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
-               error_cnt++;
+       if (CHECK_FAIL(bytes != MAGIC_BYTES * NUM_ITER * 2 ||
+                      pkts != NUM_ITER * 2)) {
                printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
                       bytes, pkts);
        }
index e545dfb..e5892cb 100644 (file)
@@ -8,14 +8,12 @@
 
 /* defined in test_progs.h */
 struct test_env env;
-int error_cnt, pass_cnt;
 
 struct prog_test_def {
        const char *test_name;
        int test_num;
        void (*run_test)(void);
        bool force_log;
-       int pass_cnt;
        int error_cnt;
        int skip_cnt;
        bool tested;
@@ -24,7 +22,6 @@ struct prog_test_def {
        int subtest_num;
 
        /* store counts before subtest started */
-       int old_pass_cnt;
        int old_error_cnt;
 };
 
@@ -68,7 +65,7 @@ static void skip_account(void)
 void test__end_subtest()
 {
        struct prog_test_def *test = env.test;
-       int sub_error_cnt = error_cnt - test->old_error_cnt;
+       int sub_error_cnt = test->error_cnt - test->old_error_cnt;
 
        if (sub_error_cnt)
                env.fail_cnt++;
@@ -105,8 +102,7 @@ bool test__start_subtest(const char *name)
                return false;
 
        test->subtest_name = name;
-       env.test->old_pass_cnt = pass_cnt;
-       env.test->old_error_cnt = error_cnt;
+       env.test->old_error_cnt = env.test->error_cnt;
 
        return true;
 }
@@ -120,6 +116,11 @@ void test__skip(void)
        env.test->skip_cnt++;
 }
 
+void test__fail(void)
+{
+       env.test->error_cnt++;
+}
+
 struct ipv4_packet pkt_v4 = {
        .eth.h_proto = __bpf_constant_htons(ETH_P_IP),
        .iph.ihl = 5,
@@ -144,7 +145,7 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name)
        map = bpf_object__find_map_by_name(obj, name);
        if (!map) {
                printf("%s:FAIL:map '%s' not found\n", test, name);
-               error_cnt++;
+               test__fail();
                return -1;
        }
        return bpf_map__fd(map);
@@ -503,8 +504,6 @@ int main(int argc, char **argv)
        stdio_hijack();
        for (i = 0; i < prog_test_cnt; i++) {
                struct prog_test_def *test = &prog_test_defs[i];
-               int old_pass_cnt = pass_cnt;
-               int old_error_cnt = error_cnt;
 
                env.test = test;
                test->test_num = i + 1;
@@ -519,8 +518,6 @@ int main(int argc, char **argv)
                        test__end_subtest();
 
                test->tested = true;
-               test->pass_cnt = pass_cnt - old_pass_cnt;
-               test->error_cnt = error_cnt - old_error_cnt;
                if (test->error_cnt)
                        env.fail_cnt++;
                else
@@ -540,5 +537,5 @@ int main(int argc, char **argv)
        free(env.test_selector.num_set);
        free(env.subtest_selector.num_set);
 
-       return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
+       return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
 }
index 9defd35..33da849 100644 (file)
@@ -38,8 +38,6 @@ typedef __u16 __sum16;
 #include "trace_helpers.h"
 #include "flow_dissector_load.h"
 
-struct prog_test_def;
-
 struct test_selector {
        const char *name;
        bool *num_set;
@@ -67,13 +65,12 @@ struct test_env {
        int skip_cnt; /* skipped tests */
 };
 
-extern int error_cnt;
-extern int pass_cnt;
 extern struct test_env env;
 
 extern void test__force_log();
 extern bool test__start_subtest(const char *name);
 extern void test__skip(void);
+extern void test__fail(void);
 
 #define MAGIC_BYTES 123
 
@@ -96,17 +93,25 @@ extern struct ipv6_packet pkt_v6;
 #define _CHECK(condition, tag, duration, format...) ({                 \
        int __ret = !!(condition);                                      \
        if (__ret) {                                                    \
-               error_cnt++;                                            \
+               test__fail();                                           \
                printf("%s:FAIL:%s ", __func__, tag);                   \
                printf(format);                                         \
        } else {                                                        \
-               pass_cnt++;                                             \
                printf("%s:PASS:%s %d nsec\n",                          \
                       __func__, tag, duration);                        \
        }                                                               \
        __ret;                                                          \
 })
 
+#define CHECK_FAIL(condition) ({                                       \
+       int __ret = !!(condition);                                      \
+       if (__ret) {                                                    \
+               test__fail();                                           \
+               printf("%s:FAIL:%d ", __func__, __LINE__);              \
+       }                                                               \
+       __ret;                                                          \
+})
+
 #define CHECK(condition, tag, format...) \
        _CHECK(condition, tag, duration, format)
 #define CHECK_ATTR(condition, tag, format...) \