On Mon, 7 Dec 2020 at 19:46, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Sun, Dec 06, 2020 at 11:45:23PM +0100, Ard Biesheuvel wrote:
> > Follow the same approach as the arm64 driver for implementing a version
> > of AES-NI in CBC mode that supports ciphertext stealing. Compared to the
> > generic CTS template wrapped around the existing cbc-aes-aesni skcipher,
> > this results in a ~2x speed increase for relatively short inputs (less
> > than 256 bytes), which is relevant given that AES-CBC with ciphertext
> > stealing is used for filename encryption in the fscrypt layer. For larger
> > inputs, the speedup is still significant (~25% on decryption, ~6% on
> > encryption).
> >
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> > Full tcrypt benchmark results for cts(cbc-aes-aesni) vs cts-cbc-aes-aesni
> > after the diff (Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz)
> >
> > arch/x86/crypto/aesni-intel_asm.S | 87 +++++++++++++
> > arch/x86/crypto/aesni-intel_glue.c | 133 ++++++++++++++++++++
> > 2 files changed, 220 insertions(+)
>
> This is passing the self-tests (including the extra tests), and it's definitely
> faster, and would be useful for fscrypt. I did my own benchmarks and got
>
> Encryption:
>
> Message size Before (MB/s) After (MB/s)
> ------------ ------------- ------------
> 32 136.83 273.04
> 64 230.03 262.04
> 128 372.92 487.71
> 256 541.41 652.95
>
> Decryption:
>
> Message size Before (MB/s) After (MB/s)
> ------------ ------------- ------------
> 32 121.95 280.04
> 64 208.72 279.72
> 128 397.98 635.79
> 256 723.09 1105.05
>
> (This was with "Intel(R) Xeon(R) Gold 6154 CPU @ 3.00GHz")
>
> So feel free to add:
>
> Tested-by: Eric Biggers <ebiggers@xxxxxxxxxx>
>
Thanks!
> I might not have time to fully review this, but one comment below:
>
> > +static int cts_cbc_encrypt(struct skcipher_request *req)
> > +{
> [...]
> > +static int cts_cbc_decrypt(struct skcipher_request *req)
> > +{
> [...]
> > #ifdef CONFIG_X86_64
> > + }, {
> > + .base = {
> > + .cra_name = "__cts(cbc(aes))",
> > + .cra_driver_name = "__cts-cbc-aes-aesni",
> > + .cra_priority = 400,
> > + .cra_flags = CRYPTO_ALG_INTERNAL,
> > + .cra_blocksize = AES_BLOCK_SIZE,
> > + .cra_ctxsize = CRYPTO_AES_CTX_SIZE,
> > + .cra_module = THIS_MODULE,
> > + },
> > + .min_keysize = AES_MIN_KEY_SIZE,
> > + .max_keysize = AES_MAX_KEY_SIZE,
> > + .ivsize = AES_BLOCK_SIZE,
> > + .walksize = 2 * AES_BLOCK_SIZE,
> > + .setkey = aesni_skcipher_setkey,
> > + .encrypt = cts_cbc_encrypt,
> > + .decrypt = cts_cbc_decrypt,
>
> The algorithm is conditional on CONFIG_X86_64, but the function definitions
> aren't.
>
> It needs to be one way or the other, otherwise there will be a compiler warning
> on 32-bit builds.
>
Ah yes, thanks for spotting that. I couldn't make up my mind whether
to bother with 32-bit support or not, but I think I'll just add it, as
it is rather straight-forward.
