Re: [PATCH] crypto: geode-aes - switch to skcipher for cbc(aes) fallback

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Oct 04, 2019 at 03:29:33PM +0200, Gert Robben wrote:
> Op 04-10-2019 om 08:16 schreef Ard Biesheuvel:
> > On Thu, 3 Oct 2019 at 23:26, Gert Robben <t2@gert.gr> wrote:
> > > Op 03-10-2019 om 15:39 schreef Ard Biesheuvel:
> > > > Commit 79c65d179a40e145 ("crypto: cbc - Convert to skcipher") updated
> > > > the generic CBC template wrapper from a blkcipher to a skcipher algo,
> > > > to get away from the deprecated blkcipher interface. However, as a side
> > > > effect, drivers that instantiate CBC transforms using the blkcipher as
> > > > a fallback no longer work, since skciphers can wrap blkciphers but not
> > > > the other way around. This broke the geode-aes driver.
> > > > 
> > > > So let's fix it by moving to the sync skcipher interface when allocating
> > > > the fallback.
> > > > 
> > > > Cc: Gert Robben <t2@gert.gr>
> > > > Cc: Jelle de Jong <jelledejong@powercraft.nl>
> > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > ---
> > > > Gert, Jelle,
> > > > 
> > > > If you can, please try this patch and report back to the list if it solves
> > > > the Geode issue for you.
> > > 
> > > Thanks for the patch!
> > > I tried it on Alix 2C2 / Geode LX800 with Linux 5.4-rc1 (also 5.1-5.3 fwiw).
> > > 
> > > At least now openssl doesn't give those errors anymore.
> > > (openssl speed -evp aes-128-cbc -elapsed -engine afalg)
> > > But looking at the results (<6MB/s), apparently it's not using geode-aes
> > > (>30MB/s?).
> > > In dmesg can be seen:
> > > 
> > > alg: skcipher: ecb-aes-geode encryption test failed (wrong result) on
> > > test vector 1, cfg="out-of-place"
> > > alg: skcipher: cbc-aes-geode encryption test failed (wrong result) on
> > > test vector 2, cfg="out-of-place"
> > > Geode LX AES 0000:00:01.2: GEODE AES engine enabled.
> > > 
> > > In /proc/crypto, drivers cbc-aes-geode/ecb-aes-geode are listed with
> > > "selftest: unknown". Driver "geode-aes" has "selftest: passed".
> > > 
> > > I'm happy to test other patches.
> > 
> > Oops, mistake there on my part
> > 
> > Can you replace the two instances of
> > 
> > skcipher_request_set_crypt(req, dst, src, nbytes, desc->info);
> > 
> > with
> > 
> > skcipher_request_set_crypt(req, src, dst, nbytes, desc->info);
> > 
> > please?
> 
> Yes, with that change, now it works in 5.4-rc1:
> 
> # openssl speed -evp aes-128-cbc -elapsed -engine afalg
> - - - 8< - - -
> The 'numbers' are in 1000s of bytes per second processed.
> type             16 bytes     64 bytes    256 bytes   1024 bytes   8192
> bytes  16384 bytes
> aes-128-cbc        125.63k      499.39k     1858.18k     6377.00k 25753.93k
> 31167.08k
> 
> I also quickly tried nginx https, that seems to transfer a file correctly.
> And a bit faster, but not by this much, I have to look into that further.
> For now I assume the kernel part seems to be working fine.
> 
> Thanks, much appreciated!
> Gert


Can you check whether it passes the extra self-tests too?  I.e. enable
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS.

- Eric