On Mon, Dec 05, 2022 at 05:11:58PM -0800, Andrii Nakryiko wrote:
> +
> +
> +typedef const void *(*skel_elf_bytes_fn)(size_t *sz);
> +
> +extern void verification_tester__run_subtests(struct verification_tester *tester,
> + const char *skel_name,
> + skel_elf_bytes_fn elf_bytes_factory);
> +
> +extern void tester_fini(struct verification_tester *tester);
> +
> +#define RUN_VERIFICATION_TESTS(skel) ({ \
> + struct verification_tester tester = {}; \
> + \
> + verification_tester__run_subtests(&tester, #skel, skel##__elf_bytes); \
> + tester_fini(&tester); \
> +})
Looking great, but couldn't resist to bikeshed a bit here.
It looks like generic testing facility. Maybe called RUN_TESTS ?
> +
> +#endif /* __TEST_PROGS_H */
> diff --git a/tools/testing/selftests/bpf/verifier_tester.c b/tools/testing/selftests/bpf/verifier_tester.c
verifier_tester name also doesn't quite fit imo.
These tests not necessarily trying to test just the verifier.
They test BTF, kfuncs and everything that kernel has to check during the loading.
In other words they test this:
> + err = bpf_object__load(tobj);
> + if (spec.expect_failure) {
> + if (!ASSERT_ERR(err, "unexpected_load_success")) {
> + emit_verifier_log(tester->log_buf, false /*force*/);
> + goto tobj_cleanup;
> + }
> + } else {
> + if (!ASSERT_OK(err, "unexpected_load_failure")) {
> + emit_verifier_log(tester->log_buf, true /*force*/);
> + goto tobj_cleanup;
> + }
> + }
maybe call it
+struct test_loader {
+ char *log_buf;
+ size_t log_buf_sz;
+
+ struct bpf_object *obj;
+};
?
and the file test_loader.c ?
Nicely shorter than verification_tester__ prefix...
