2024-02-23 06:39:19

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH 00/15] crypto: Add twopass lskcipher for adiantum

On Thu, Feb 15, 2024 at 04:20:34PM +0800, Herbert Xu wrote:
> On Wed, Feb 14, 2024 at 03:35:17PM -0800, Eric Biggers wrote:
> >
> > Thanks. Can you include an explanation of the high-level context and goals for
> > this work? It's still not clear to me. I'm guessing that the main goal is to
> > get rid of the vaddr => scatterlist => vaddr round trip for software
> > encryption/decryption, which hopefully will improve performance and make the API
> > easier to use? And to do that, all software algorithms need to be converted to
>
> The main goal is to remove the legacy cipher type, and replacing
> it with lskcipher.

What is the benefit of that change?

This series also goes way beyond that, so it seems like there's more going on
here. I do like the support for vaddr; the scatterlist-based APIs have always
been one of the main pain points with the crypto API. But you're claiming
that fixing that isn't actually the goal. So I'm confused.

> > "lskcipher"? Will skcipher API users actually be able to convert to lskcipher,
> > or will they be blocked by people expecting to be able to use hardware crypto
> > accelerators? Would you accept lskcipher being used alongside skcipher?
>
> That's a question for each user to decide.
>
> > Previously you had said you don't want shash being used alongside ahash.
>
> In general, if the amount of data being processed is large, then
> I would expect the use of hardware accelerators to be a possibility
> and therefore choose the SG-based interface.
>
> I wouldn't consider 4K to be large though. So it's really when you
> feed hundreds of kilobytes of data through the algorithm when I would
> recommend against using shash.

dm-verity usually hashes 4K at a time, but that was enough for people to want it
to support hardware accelerators, so it had to be switched to ahash. But, you
objected to my patch that added shash support to dm-verity alongside ahash
(https://lore.kernel.org/dm-devel/[email protected]).

That suggests that adding lskcipher support to dm-crypt and fscrypt alongside
skcipher would similarly not be "allowed". Most users don't use off-CPU
hardware accelerators with those, but some do.

I did get away with (so far) switching fs/verity/ to shash. I'm not sure I
could similarly get away with switching fs/crypto/ to lskcipher. There are
people using the CAAM AES-CBC hardware accelerator with fscrypt.

Before we go through a big effort to convert all these algorithms to lskcipher,
or (more likely based on history) leave everything in a half-finished state, I'd
like to get a good sense that lskcipher will be useful.

- Eric