Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191005091110.12556-1-ard.biesheuvel@linaro.org> <b18ff289-ceca-d934-6583-caf8d2916bcb@gert.gr>
In-Reply-To: <b18ff289-ceca-d934-6583-caf8d2916bcb@gert.gr>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Tue, 8 Oct 2019 13:38:13 +0200
Message-ID: <CAKv+Gu9yoROn8+vmE=_c-gTm2H1WGTJsPb+yj1YBv6+-P6ei_A@mail.gmail.com>
Subject: Re: [PATCH v2] crypto: geode-aes - switch to skcipher for cbc(aes) fallback
To:     Gert Robben <t2@gert.gr>
Cc:     "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>,
        Jelle de Jong <jelledejong@powercraft.nl>,
        Eric Biggers <ebiggers@kernel.org>,
        Florian Bezdeka <florian@bezdeka.de>,
        Herbert Xu <herbert@gondor.apana.org.au>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Sat, 5 Oct 2019 at 18:15, Gert Robben <t2@gert.gr> wrote:
>
> Op 05-10-2019 om 11:11 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. At the same time, align with the generic API for ECB and
> > CBC by rejecting inputs that are not a multiple of the AES block size.
> >
> > Fixes: 79c65d179a40e145 ("crypto: cbc - Convert to skcipher")
> > Cc: <stable@vger.kernel.org> # v4.20+ ONLY
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > v2: pass dst and src scatterlist in the right order
> >      reject inputs that are not a multiple of the block size
>
> Yes, with this patch, the CRYPTO_MANAGER_EXTRA_TESTS output nothing
> (apart from "extra crypto tests enabled").
> All items in /proc/crypto have "selftest: passed" mentioned.
> "openssl speed -evp aes-128-cbc -elapsed -engine afalg" reaches the
> proper speed.
> And nginx (correctly) transfers files about 40% faster than without
> geode-aes.
>
> I didn't think about testing ecb before, because I don't use it.
> Now that I did, I tried the same openssl benchmark for ecb.
> But that only reaches software AES speed, and "time" also shows the work
> is being done in "user" instead of "sys" (see below).
> Yet I see no errors.
> (Maybe this is normal/expected, so I didn't look much further into it).
>
> Thank you,
> Gert
>
> # time openssl speed -evp aes-128-cbc -elapsed -engine afalg
> - - - 8< - - -
> type             16 bytes     64 bytes    256 bytes   1024 bytes   8192
> bytes  16384 bytes
> aes-128-cbc        135.82k      539.29k     2087.90k     7491.16k
> 29221.69k    34943.67k
>
> real    0m18.081s
> user    0m0.516s
> sys     0m17.541s
>
> # time openssl speed -evp aes-128-ecb -elapsed -engine afalg
> - - - 8< - - -
> type             16 bytes     64 bytes    256 bytes   1024 bytes   8192
> bytes  16384 bytes
> aes-128-ecb       4480.65k     5137.66k     5336.94k     5410.19k
> 5409.91k     5409.91k
>
> real    0m18.084s
> user    0m18.046s
> sys     0m0.012s
>

It seems likely that the ECB code in OpenSSL is not invoking the afalg
code at all. Since ECB is just the bare block cipher applied to each
block in the input, I wonder if it even uses a skcipher like interface
internally.