crypto: skcipher - multi-data-unit request splitting
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v6 Latest6 patches 5 repliesIn review
- v55 patches 8 repliesSuperseded
- v43 patches 7 repliesSuperseded
Earlier versions · 3
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v3 | 4 patches | 2 replies | Superseded | |
| v2 | 4 patches | 1 reply | Superseded | |
| v1 | 4 patches | 1 reply | Superseded |
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 files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/6 | crypto: skcipher - add per-request unit_sizeinclude/ | +46 | 0 replies | |
| 2/6 | crypto: acomp - Add bit to indicate segmentation supportinclude/ include/ include/ | +13 | 0 replies | |
| 3/6 | crypto: skcipher - add crypto_skcipher_req_seg() helperinclude/ | +5 | 0 replies | |
| 4/6 | crypto: skcipher - split multi-unit requests in the API layercrypto/ | +112 | 0 replies | |
| 5/6 | crypto: testmgr - test multi-unit dispatchcrypto/ | +272 | 0 replies | |
| 6/6 | dm crypt: batch a bio segment's sectors via multi-unit requestsdrivers/ | +114 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v6 yet.
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 d9e8add5c8c1ac75a00bca754d73b222ec7594245d3cafaf336ce967b3dad589The 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