Skip to content

crypto: skcipher - multi-data-unit request splitting

Leonid Ravich dm-devel linux-crypto lkml 6 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest6 patches 5 repliesIn review
  2. v55 patches 8 repliesSuperseded
  3. v43 patches 7 repliesSuperseded
Earlier versions · 3
VersionPostedPatchesRepliesStatus
v34 patches2 repliesSuperseded
v24 patches1 replySuperseded
v14 patches1 replySuperseded

Changes in v6 from the cover letter

- the dun() template is gone; the split moved into the mid-API layer (crypto/skcipher.c) and is skipped for algorithms advertising native support.
- data_unit_size renamed unit_size; field, setter and opt-in semantics aligned with the acomp batching series, whose CRYPTO_ALG_REQ_SEG patch is carried here (2/6).
- no endianness knob: the IV is always a little-endian DUN in its low 8 bytes; other on-disk layouts are a template's job. plain64 needs no template, essiv already is one, plain64be is left unbatched pending a plain64be() template.
- blk-crypto-fallback dropped as a consumer (Eric is moving it to lib/crypto); dm-crypt is the only user.
- counter is specified and implemented as 64-bit wrapping at 2^64 with no carry into higher IV bytes, matching dm-crypt's generators; testmgr exercises the wrap.
- per-unit IV copy aligned to MAX_ALGAPI_ALIGNMASK (a misaligned IV would force an internal allocation, which dm-crypt excludes via CRYPTO_ALG_ALLOCATES_MEMORY).
- cond_resched() between units when CRYPTO_TFM_REQ_MAY_SLEEP, so a large batch keeps dm-crypt's per-sector preemption point.

Patches in v6 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/6crypto: skcipher - add per-request unit_sizeinclude/crypto/skcipher.h+46 -00 replies
2/6crypto: acomp - Add bit to indicate segmentation supportinclude/crypto/algapi.h include/crypto/internal/acompress.h include/linux/crypto.h+13 -00 replies
3/6crypto: skcipher - add crypto_skcipher_req_seg() helperinclude/crypto/internal/skcipher.h+5 -00 replies
4/6crypto: skcipher - split multi-unit requests in the API layercrypto/skcipher.c+112 -00 replies
5/6crypto: testmgr - test multi-unit dispatchcrypto/testmgr.c+272 -00 replies
6/6dm crypt: batch a bio segment's sectors via multi-unit requestsdrivers/md/dm-crypt.c+114 -220 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v6 yet.

Read the v6 thread

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

Hi all,

This series lets a caller submit several data units in one skcipher
request: the request carries a unit_size, the IV is the data-unit number of the first unit, and the API layer walks the counter across units. dm-crypt is the first (and in this series the only) user -- it stops issuing one skcipher request per sector and instead hands the cipher a whole contiguous bio segment (8 units for a 4 KiB bio_vec with the default 512 B sector).

v6 is a full rework of v5 along the lines Herbert laid out, so the design section below is mostly new rather than a delta. The v5 dun() template is gone; the split now lives in the mid-API layer and is skipped entirely for algorithms that advertise native multi-unit support.

v5: https://lore.kernel.org/linux-crypto/[email protected]/

Answers to Herbert's v5 questions
=================================

1. "Could you send me the patch so I can take a look?" (per-unit cost)
---------------------------------------------------------------------

Patch 4 is that patch -- the API-layer split, self-contained in crypto/skcipher.c. The cost I measured, and where I believe it goes:

An in-kernel microbench of skcipher_crypt_unit() against the legacy per-unit loop (identical inner AES, VAES-AVX512, r7i.metal) shows a fixed ~48-52 ns per data unit, CV <1%, and it is the *same* ~50 ns for a 512 B unit and a 4096 B unit -- so it is per-call setup, not a crypto effect. Against ~70 ns of VAES work for a 512 B sector that is ~+70% on the crypto call itself; end-to-end in dm-crypt it is ~2% of an ~18 us I/O and does not show up in fio at all (see the Performance section below).

The three things the split does per unit:

* copies the running counter IV into a per-unit scratch buffer

    (req->iv must come back unmodified, and the algorithm is free to
    clobber the IV it is given, so the copy cannot be elided by
    handing the algorithm the counter directly);

* re-slices the source and destination scatterlists to the unit's offset/length, which walks the sglist from the previous unit's position;
* pays one extra call frame plus the save/restore of the fields it mutates on the caller's request.

The sg re-slice is the part I expect can be made cheaper (a stateful walk carried across units rather than an offset-based slice each time), and the IV copy could go away for algorithms that promise not to modify the IV -- but I did not want to invent a new "does not clobber IV" contract unprompted. I would rather hear which of these you had in mind than guess.

2. "no templates for plain64, the other ones should use a template"
------------------------------------------------------------------

Implemented, with one mode left out. With unit-splitting in use, dm-crypt now always passes the little-endian sector number as the IV and the API layer walks it as a 64-bit little-endian counter in the low 8 bytes; there is no endianness knob anywhere in the API. So:

* plain64 -- batched, no template, and one *fewer* indirect call

                than before (no ->generator per sector).

* essiv -- batched, unchanged: its IV input already is
le64(sector) and the salt encryption already lives in the essiv() template, which is exactly the shape you described.
* plain64be -- not batched in this series. Its on-disk IV is a big-endian counter in the *high* 8 bytes, which is not the low-limb little-endian layout the split walks, so it keeps the existing per-sector path. Batching it wants a plain64be() template that byte-swaps the low limb into the high one. I left that out deliberately: it is a new template with a single user and no measurement behind it, and it is additive on top of this series. Say the word and I will add it as a 7th patch (or a follow-up).
* everything else (plain, benbi, lmk, tcw, eboiv, random, elephant, null) is not a step-of-one counter at all and keeps the per-sector path; those would each need their own template, which is the follow-up work your scheme makes possible.

Net indirect calls, as you predicted: unchanged for essiv, one fewer for plain64, unchanged for the unbatched modes.

3. "Is this going to use the generated IV for reading through dm-crypt? Shouldn't it be using the IV stored on disk instead of generating it again for reading?"
-------------------------------------------------------------------

For the modes this series batches, there is no IV stored on disk to use -- and that is pre-existing dm-crypt behaviour, not something the batching changes. plain64 and essiv are deterministic functions of the sector number (essiv = E_salt(le64(sector))), so read and write derive bit-identically from the same input; dm-crypt has always regenerated them on both paths and there is nowhere in the plain sector-mapped format to store an IV anyway.

The configurations that *do* carry a per-sector IV/tag on disk are the integrity ones (dm-integrity stacking, AEAD, random IV), and those are excluded from batching by crypt_can_batch_units(): the gate requires !crypt_integrity_aead and no integrity metadata, precisely because a stored per-sector IV cannot be walked as a counter. Those keep the per-sector loop, where the stored IV is read as before.

So no behaviour change on reads: same IV, same ciphertext, verified byte-identical against an unpatched kernel (see Verification below).

Design overview
===============

1/6 adds unsigned int unit_size to struct skcipher_request plus
skcipher_request_set_unit_size(), mirroring the acomp field and setter. 0 (the default) is a normal single-unit request; set_tfm() and set_callback() zero it, so the opt-in is explicit and per-operation -- same contract as acomp.

2/6 is Herbert's CRYPTO_ALG_REQ_SEG patch, carried verbatim from the
acomp batching series (see the note at the end of this letter).

3/6 adds crypto_skcipher_req_seg(), the skcipher-side mirror of

     crypto_acomp_req_seg().

4/6 is the split: when unit_size is set and the algorithm does not
advertise CRYPTO_ALG_REQ_SEG, crypto_skcipher_{en,de}crypt() issue one call per unit, advancing a 64-bit little-endian DUN in the low 8 bytes of the IV. The counter wraps at 2^64 and never carries above, so output is bit-identical to the per-unit path across rollover. The caller's request is reused and restored (same tfm, so the request context is already the right size); req->iv is never modified, and each unit gets a private IV copy aligned to MAX_ALGAPI_ALIGNMASK so it keeps the alignment the caller's IV had. The split is synchronous, so a multi-unit request on an async non-native algorithm is rejected -EOPNOTSUPP, and it reschedules between units when the caller allows sleeping. Callers that never set unit_size pay one unlikely() test. The unit_size test runs *before* the lskcipher redirect, so lskcipher-backed modes (cbc) split correctly too.

5/6 extends testmgr: for every self-tested sync skcipher with an
eligible IV, one batched request over a deliberately fragmented scatterlist must produce ciphertext byte-identical to N single-unit requests with counter-walked IVs, then round-trip. The reference counter is written independently of the API layer's, and each unit size is also run with the IV seeded to force a wrap to zero. Covers ivsize 16 (xts) and 32 (Adiantum). The caller's IV must come back unmodified.

6/6 is dm-crypt: set unit_size = cc->sector_size and submit one
request per contiguous bio segment, using only the existing inline single-entry scatterlist -- no per-bio allocation. Gated on a little-endian step-of-one sector counter (plain64, essiv), single tfm, non-aead, sector_size 512 or iv_large_sectors, and no integrity metadata.

Performance
===========

* dm-crypt fio, r7i.metal-24xl (Sapphire Rapids, VAES-AVX512), tmpfs-backed loop, series vs the same tree without it: no measurable regression across aes-xts-plain64, aes-cbc-essiv:sha256 and aes-xts-plain64be at 512 B, plus a 4096 B non-batching control (median delta +0.0%, and +0.8% read / -0.7% write at the decisive 512 B qd=1 point). Two caveats: this rig is dispatch-bound on this CPU, so a flat result here mostly means the per-unit cost is under the noise floor rather than absent -- hence the microbench in answer 1 above; and the run predates the rework, on a tree that also batched plain64be, which the rework only removes from the batched path.
* No throughput *win* is claimed for software AES. The win is for accelerators that amortise setup across units; the software split exists so the interface works on every existing skcipher today and goes quiet as algorithms gain CRYPTO_ALG_REQ_SEG.

Verification
============

* 19/19 cases of a qemu regression protocol pass on x86_64 and
arm64: builds clean with and without the series, checkpatch --strict clean, testmgr multi-unit cross-check, activation gating (plain64/essiv batched; plain64be, multikey, integrity not batched), round-trips, 4096-sector iv_large_sectors, low-memory (128 MB) run, and blk-crypto-fallback/fscrypt unaffected.
* End-to-end byte equivalence against an unpatched baseline (a8cafdf8c949), whole-device sha256:

    plain64  f462a2ff4ec6f7d1219ef34897793b32d8e19b777cff914a7860289da9044b53
    essiv    d9e8add5c8c1ac75a00bca754d73b222ec7594245d3cafaf336ce967b3dad589

The on-disk format is unchanged.

Changelog
=========

v6 (this posting; addresses Herbert's v5 review):
- the dun() template is gone; the split moved into the mid-API layer (crypto/skcipher.c) and is skipped for algorithms advertising native support.
- data_unit_size renamed unit_size; field, setter and opt-in semantics aligned with the acomp batching series, whose CRYPTO_ALG_REQ_SEG patch is carried here (2/6).
- no endianness knob: the IV is always a little-endian DUN in its low 8 bytes; other on-disk layouts are a template's job. plain64 needs no template, essiv already is one, plain64be is left unbatched pending a plain64be() template.
- blk-crypto-fallback dropped as a consumer (Eric is moving it to lib/crypto); dm-crypt is the only user.
- counter is specified and implemented as 64-bit wrapping at 2^64 with no carry into higher IV bytes, matching dm-crypt's generators; testmgr exercises the wrap.
- per-unit IV copy aligned to MAX_ALGAPI_ALIGNMASK (a misaligned IV would force an internal allocation, which dm-crypt excludes via CRYPTO_ALG_ALLOCATES_MEMORY).
- cond_resched() between units when CRYPTO_TFM_REQ_MAY_SLEEP, so a large batch keeps dm-crypt's per-sector preemption point.

v5 and earlier: see the v5 link above.

A note on patch 2
=================

Patch 2 ("crypto: acomp - Add bit to indicate segmentation support") is Herbert's commit from the acomp batching series, carried here verbatim because the skcipher split needs the CRYPTO_ALG_REQ_SEG bit and that series has not landed yet. It is unmodified apart from my own Signed-off-by and a cherry-pick reference, so it carries its author's authorship but only Herbert's Signed-off-by -- which checkpatch reports as a missing author Signed-off-by. That is inherent to carrying the commit rather than a defect in it; it disappears once the acomp series is upstream and this patch can be dropped from the series. I would rather flag it here than silently rewrite authorship on someone else's commit.

Thanks,
Leonid

Kanchana P Sridhar (1):
crypto: acomp - Add bit to indicate segmentation support

Leonid Ravich (5):
crypto: skcipher - add per-request unit_size
crypto: skcipher - add crypto_skcipher_req_seg() helper
crypto: skcipher - split multi-unit requests in the API layer
crypto: testmgr - test multi-unit dispatch
dm crypt: batch a bio segment's sectors via multi-unit requests

 crypto/skcipher.c                   | 112 ++++++++++++
 crypto/testmgr.c                    | 272 ++++++++++++++++++++++++++++
 drivers/md/dm-crypt.c               | 136 +++++++++++---
 include/crypto/algapi.h             |   5 +
 include/crypto/internal/acompress.h |   5 +
 include/crypto/internal/skcipher.h  |   5 +
 include/crypto/skcipher.h           |  46 +++++
 include/linux/crypto.h              |   3 +
 8 files changed, 562 insertions(+), 22 deletions(-)

base-commit: a8cafdf8c949f17c92eca0045532e88ac0dac30d

Signature · 1 line
2.47.3