linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v1 0/2] crypto: tcrypt: small changes to run under KUnit
@ 2021-07-15 21:31 Daniel Latypov
  2021-07-15 21:31 ` [RFC v1 1/2] crypto: tcrypt: minimal conversion " Daniel Latypov
  2021-07-15 21:31 ` [RFC v1 2/2] crypto: tcrypt: call KUNIT_FAIL() instead of pr_err() Daniel Latypov
  0 siblings, 2 replies; 7+ messages in thread
From: Daniel Latypov @ 2021-07-15 21:31 UTC (permalink / raw)
  To: herbert
  Cc: davem, linux-crypto, brendanhiggins, davidgow, linux-kernel,
	kunit-dev, Daniel Latypov

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-30  5:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 21:31 [RFC v1 0/2] crypto: tcrypt: small changes to run under KUnit Daniel Latypov
2021-07-15 21:31 ` [RFC v1 1/2] crypto: tcrypt: minimal conversion " Daniel Latypov
2021-07-23  6:43   ` Herbert Xu
2021-07-23 19:31     ` Daniel Latypov
2021-07-30  2:55       ` Herbert Xu
2021-07-30  5:33         ` David Gow
2021-07-15 21:31 ` [RFC v1 2/2] crypto: tcrypt: call KUNIT_FAIL() instead of pr_err() Daniel Latypov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).