[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260114230430.GB1449008@google.com>
Date: Wed, 14 Jan 2026 23:04:30 +0000
From: Eric Biggers <ebiggers@...nel.org>
To: Holger Dengler <dengler@...ux.ibm.com>
Cc: Ard Biesheuvel <ardb@...nel.org>,
"Jason A . Donenfeld" <Jason@...c4.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Harald Freudenberger <freude@...ux.ibm.com>,
linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org
Subject: Re: [RFC PATCH 1/1] lib/crypto: tests: Add KUnit tests for AES
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
Powered by blists - more mailing lists