Re: [PATCH v6 0/6] crypto: skcipher - multi-data-unit request splitting

[Leonid Ravich <lravich@amazon.com>]

On Fri, Oct 02, 2026 at 05:47:26PM +1000, Herbert Xu wrote:
> I think the issue is that we're generating the IV twice.  Once
> in the Crypto API and once again in the DM layer.  Not only is
> this slow, but it is actually wrong for decryption.  You're ignoring
> the IVs on the disk.

I don't think either happens in v6, so let me check we are reading
the same code before I change the design.

The IV is generated once per request, not per unit.  In
crypt_convert_block_skcipher() dm-crypt calls iv_gen_ops->generator()
for the first sector of the segment only.  The Crypto API then derives
the IV of each following unit by incrementing the 64-bit little-endian
sector number in the low 8 bytes.  That is the same value the
generator would have produced for that sector, so no generator runs
twice.  Only plain64 and essiv are batched, because those are the
modes where IV(sector + i) is exactly that increment.

On-disk IVs are never ignored, because batching is never enabled when
they exist.  The only case where dm-crypt reads an IV from disk is
the integrity-metadata branch in the same function:

	/* For READs use IV stored in integrity metadata */
	if (cc->integrity_iv_size && bio_data_dir(ctx->bio_in) != WRITE)
		memcpy(org_iv, tag_iv, cc->integrity_iv_size);

CRYPT_MULTI_DATA_UNIT is only set when integrity_iv_size is 0 (and
the target is not AEAD; see crypt_can_batch_units() and its caller
in crypt_ctr_cipher()).  So a device with stored IVs keeps the
one-sector-per-request path, and its reads use the stored IV exactly
as today.

On the cost: the IV part of the ~50 ns is a 16-byte copy and an
increment per unit.  The rest is re-pointing the scatterlist at each
unit and the extra call into the algorithm per unit.  An IV array
would replace the copy and increment with a load from the array, but
the per-unit scatterlist and call overhead would stay.  I have not
measured the components separately; I can if that would help.

> My suggestion is to allocate memory for the IVs.  Of course
> memory allocation can fail, but we have an easy fallback, which
> is to use the existing single-unit path.
>
> IOW if you succeed in allocating memory for storing the IVs,
> then invoke the multi-unit code path, otherwise fall back to
> the single-unit code path which iterates over the sectors one-
> by-one.
>
> To pass the IVs to the Crypto API (or back), just use the existing
> IV pointer and extend it by the number of units.

That said, I can see what an IV array would buy beyond the current
modes.  The Crypto API would no longer need to know how IVs relate
to each other, so every dm-crypt IV mode could batch (lmk, tcw,
eboiv, benbi, and plain64be without a template), and so could
devices that store their IVs in integrity metadata, since dm-crypt
would fill the array from the tags on read.

So the question for you is about scope rather than correctness: do
you want the interface to be "one IV per unit, ivsize * nunits bytes
at req->iv" so that it covers those modes too?  If so, v7 will do it
that way, with the single-unit fallback when the allocation fails.
If plain64 and essiv are enough for now, I would keep the counter
form from v6.

Thanks,
Leonid