2021-07-15 21:32:47

by Daniel Latypov

[permalink] [raw]
Subject: [RFC v1 0/2] crypto: tcrypt: small changes to run under KUnit

tcrypt.c calls itself a "[q]uick & dirty crypto testing module."
One that "will only exist until we have a better testing mechanism."

This RFC seeks to start a discussion if KUnit can fill the role of that
"better testing mechanism."

As-is, these example changes don't make the test code much cleaner.
But they do provide a new way of running the test that's hopefully more
accessible, namely
$ ./tools/testing/kunit/kunit.py run --kunitconfig=crypto
...
[16:53:16] Starting KUnit Kernel ...
[16:53:19] ============================================================
[16:53:19] ======== [PASSED] tcrypt ========
[16:53:19] [PASSED] tcrypt
[16:53:19] ============================================================
[16:53:19] Testing complete. 1 tests run. 0 failed. 0 crashed. 0 skipped.
[16:53:19] Elapsed time: 8.806s total, 0.001s configuring, 5.764s building, 0.000s running

This series contains
* an initial patch with the boilerplate needed to run
under KUnit and track pass/fail status in the `test` context object
instead of passing around an int.
* another change to plumb the `test` context object to other test cases
that could previously fail w/o affecting the overall pass/fail status.

I haven't reformatted the code for now just to make the changes a bit
easier to read and skim over. checkpatch.pl isn't happy about the
spacing in the second patch.

== Other pros ==

If we want, we can go down the path of trying to structure the tests in
more idiomatic KUnit fashion to get some benefits like
* automatically freeing memory at the end of tests when allocated using
kunit_kmalloc() and friends instead of having to maintain labels
* can be made to call arbitrary cleanup functions as well
* this hopefully makes the tests more readable, at the expense of a
bit more runtime overhead dynamically tracking these allocations
* doing this just for the kmalloc's and __get_free_page() directly in
* tcrypt.c saves 100+ lines of code.

* flagging test cases with KASAN issues (and eventually UBSAN)

* a bit easier way to dynamically run subsets of tests via glob
* e.g. kunit.py run 'crypto.*sha512*'
* KUnit currently only supports filtering by the suite ("crypto")
right now, but we should have test-level filtering support soonish.

== Cons ==

The main cons are we'd be slightly changing how these tests are built
and run with these example patches

These changes are mainly
* building the test now requires CONFIG_KUNIT
* Running `insmod` on the module will always return 0, even if tests
failed
* the test instead prints out (K)TAP output to denote pass/fail
* this is the format kselftest uses, but it's not fully standardized

And if we eventually try to restructure the test as mentioned above:
* more disruptive changes to how the tests are run
* we'd have to move away from using the "mode" parameter
* a decent amount of code churn

Daniel Latypov (2):
crypto: tcrypt: minimal conversion to run under KUnit
crypto: tcrypt: call KUNIT_FAIL() instead of pr_err()

crypto/Kconfig | 5 +-
crypto/tcrypt.c | 1063 +++++++++++++++++++++++------------------------
2 files changed, 524 insertions(+), 544 deletions(-)


base-commit: e9338abf0e186336022293d2e454c106761f262b
--
2.32.0.402.g57bb445576-goog