Re: [RFC PATCH 1/1] lib/crypto: tests: Add KUnit tests for AES

[Eric Biggers <ebiggers@kernel.org>]

Thanks for writing this!

On Wed, Jan 14, 2026 at 04:31:38PM +0100, Holger Dengler wrote:
> diff --git a/lib/crypto/tests/Kconfig b/lib/crypto/tests/Kconfig
> index 4970463ea0aa..f34e79093275 100644
> --- a/lib/crypto/tests/Kconfig
> +++ b/lib/crypto/tests/Kconfig
> @@ -118,6 +118,18 @@ config CRYPTO_LIB_SHA3_KUNIT_TEST
>  	  including SHA3-224, SHA3-256, SHA3-384, SHA3-512, SHAKE128 and
>  	  SHAKE256.
>  
> +config CRYPTO_LIB_AES_KUNIT_TEST
> +	tristate "KUnit tests for AES" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS || CRYPTO_SELFTESTS
> +	select CRYPTO_LIB_BENCHMARK_VISIBLE
> +	select CRYPTO_LIB_AES
> +	help
> +	  KUnit tests for the AES library functions, including known answer
> +	  tests and benchmarks for encrypt/decrypt with all key sizes. The
> +	  test suite does not contain any key generation test, nor any error
> +	  cases.

It should go first in the file, to maintain the existing alphabetical
order.

> diff --git a/lib/crypto/tests/Makefile b/lib/crypto/tests/Makefile
> index f4262379f56c..72234e965cdc 100644
> --- a/lib/crypto/tests/Makefile
> +++ b/lib/crypto/tests/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_CRYPTO_LIB_SHA1_KUNIT_TEST) += sha1_kunit.o
>  obj-$(CONFIG_CRYPTO_LIB_SHA256_KUNIT_TEST) += sha224_kunit.o sha256_kunit.o
>  obj-$(CONFIG_CRYPTO_LIB_SHA512_KUNIT_TEST) += sha384_kunit.o sha512_kunit.o
>  obj-$(CONFIG_CRYPTO_LIB_SHA3_KUNIT_TEST) += sha3_kunit.o
> +obj-$(CONFIG_CRYPTO_LIB_AES_KUNIT_TEST) += aes_kunit.o

Likewise in the Makefile.

> diff --git a/lib/crypto/tests/aes_kunit.c b/lib/crypto/tests/aes_kunit.c
> new file mode 100644
> index 000000000000..057ddc3a1b1f
> --- /dev/null
> +++ b/lib/crypto/tests/aes_kunit.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <kunit/test.h>
> +
> +#include "aes-testvecs.h"
> +
> +#define AES_KAT(bits, func, from, to)				\
> +static void aes##bits##_kat_##func(struct kunit *test)		\
> +{								\
> +	const u8 *in = AES##bits##_KAT.from;			\
> +	u8 out[AES_BLOCK_SIZE];					\
> +	struct aes_key aes_key;					\
> +								\
> +	if (aes_preparekey(&aes_key, AES##bits##_KAT.key.b,	\
> +			   AES##bits##_KAT.key.len))		\
> +		kunit_skip(test, "no key");			\

Skipping on failure seems wrong.

> +#define KB		(1024)
> +#define MB		(KB * KB)
> +#define NS_PER_SEC	(1000000000ULL)

If you'd like to use named constants for these, note that the kernel
headers already have constants SZ_1K, SZ_1M, and NSEC_PER_SEC.  So these
local definitions aren't needed.

> +
> +#define AES_BENCHMARK(bits)					\
> +static void aes##bits##_benchmark(struct kunit *test)		\
> +{								\
> +	const size_t num_iters = 10000000;			\
> +	const u8 *cipher = AES##bits##_KAT.cipher;		\
> +	const u8 *plain = AES##bits##_KAT.plain;		\
> +	u8 out[AES_BLOCK_SIZE];					\
> +	struct aes_key aes_key;					\
> +	u64 t_enc, t_dec;					\
> +								\
> +	if (!IS_ENABLED(CONFIG_CRYPTO_LIB_BENCHMARK))		\
> +		kunit_skip(test, "not enabled");		\
> +								\
> +	if (aes_preparekey(&aes_key, AES##bits##_KAT.key.b,	\
> +			   AES##bits##_KAT.key.len))		\
> +		kunit_skip(test, "no key");			\
> +								\
> +	/* warm-up enc */					\
> +	for (size_t i = 0; i < 1000; i++)			\
> +		aes_encrypt(&aes_key, out, plain);		\
> +								\
> +	preempt_disable();					\
> +	t_enc = ktime_get_ns();					\
> +								\
> +	for (size_t i = 0; i < num_iters; i++)			\
> +		aes_encrypt(&aes_key, out, plain);		\
> +								\
> +	t_enc = ktime_get_ns() - t_enc;				\
> +	preempt_enable();					\
> +								\
> +	/* warm-up dec */					\
> +	for (size_t i = 0; i < 1000; i++)			\
> +		aes_decrypt(&aes_key, out, cipher);		\
> +								\
> +	preempt_disable();					\
> +	t_dec = ktime_get_ns();					\
> +								\
> +	for (size_t i = 0; i < num_iters; i++)			\
> +		aes_decrypt(&aes_key, out, cipher);		\
> +								\
> +	t_dec = ktime_get_ns() - t_dec;				\
> +	preempt_enable();					\
> +								\
> +	kunit_info(test, "enc (iter. %zu, duration %lluns)",	\
> +		   num_iters, t_enc);				\
> +	kunit_info(test, "enc (len=%zu): %llu MB/s",		\
> +		   (size_t)AES_BLOCK_SIZE,			\
> +		   div64_u64((u64)AES_BLOCK_SIZE * num_iters * NS_PER_SEC, \
> +			     (t_enc ?: 1) * MB));		\
> +								\
> +	kunit_info(test, "dec (iter. %zu, duration %lluns)",	\
> +		   num_iters, t_dec);				\
> +	kunit_info(test, "dec (len=%zu): %llu MB/s",		\
> +		   (size_t)AES_BLOCK_SIZE,			\
> +		   div64_u64((u64)AES_BLOCK_SIZE * num_iters * NS_PER_SEC, \
> +			     (t_dec ?: 1) * MB));		\
> +}
> +
> +AES_KAT(128, encrypt, plain, cipher);
> +AES_KAT(192, encrypt, plain, cipher);
> +AES_KAT(256, encrypt, plain, cipher);
> +AES_KAT(128, decrypt, cipher, plain);
> +AES_KAT(192, decrypt, cipher, plain);
> +AES_KAT(256, decrypt, cipher, plain);
> +AES_BENCHMARK(128);
> +AES_BENCHMARK(192);
> +AES_BENCHMARK(256);

The heavy use of macros doesn't seem that helpful here.  The API is
already unified, where we have aes_preparekey(), aes_encrypt(), and
aes_decrypt() that handle all of AES-128, AES-192, and AES-256.  So we
don't need entirely different code to test each variant.

We could just write helper functions, e.g. aes_test() and
aes_benchmark().  They would take in a pointer to a test vector, and the
individual KUnit case functions would call them.

See lib/crypto/tests/mldsa_kunit.c which does something similar.

- Eric