crypto: skcipher - multi-data-unit request splitting
Subsystems (3)
v6 cover 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
PATCH v6 1/6 crypto: skcipher - add per-request unit_size
Add a unit_size field to struct skcipher_request, mirroring the acomp unit_size (and its setter), so a caller can submit several data units in one request: cryptlen / unit_size units sharing one starting IV, with per-unit IVs derived from the IV as a 64-bit little-endian data-unit-number counter held in its low 8 bytes.
unit_size == 0 (the default) means a normal single-unit request; skcipher_request_set_tfm() and the on-stack request initializer zero the field, so existing callers and reused requests are unaffected.
--- include/crypto/skcipher.h | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
include/crypto/skcipher.h +46 -0
@@ -31,6 +31,17 @@ struct scatterlist;/** * struct skcipher_request - Symmetric key cipher request * @cryptlen: Number of bytes to encrypt or decrypt +* @unit_size: Size in bytes of each data unit, or 0 for a +* single-unit request (the default). When non-zero, must be a +* multiple of the cipher block size and @cryptlen must be a +* positive multiple of it. The data-unit number is a 64-bit +* little-endian counter in the low 8 bytes of @iv, incremented +* once per unit and wrapping at 2^64; the remaining IV bytes are +* left unchanged (any other on-disk IV layout is produced by a +* template wrapping the algorithm). @iv itself is not modified +* by the request. An algorithm that advertises +* CRYPTO_ALG_REQ_SEG handles the whole request natively and must +* follow this same counter and @iv-preservation convention. * @iv: Initialisation Vector * @src: Source SG list * @dst: Destination SG list
@@ -39,6 +50,7 @@ struct scatterlist;*/ struct skcipher_request {unsigned int cryptlen; +unsigned int unit_size;u8 *iv;
@@ -225,6 +237,7 @@ struct lskcipher_alg { struct skcipher_request *name = \ (((struct skcipher_request *)__##name##_desc)->base.tfm = \ crypto_sync_skcipher_tfm((_tfm)), \ + ((struct skcipher_request *)__##name##_desc)->unit_size = 0, \ (void *)__##name##_desc)/**
@@ -819,6 +832,8 @@ static inline void skcipher_request_set_tfm(struct skcipher_request *req, struct crypto_skcipher *tfm) { req->base.tfm =crypto_skcipher_tfm (tfm); +/* New tfm, new request: default to single-unit. */ + req->unit_size =0 ; } static inline void skcipher_request_set_sync_tfm(struct skcipher_request *req,
@@ -908,6 +923,10 @@ static inline void skcipher_request_set_callback(struct skcipher_request *req, req->base.complete = compl; req->base.data = data; req->base.flags = flags; +/* Reset the per-op multi-unit control; a reused request defaults to +* single-unit until skcipher_request_set_unit_size() opts back in. +*/ + req->unit_size =0 ; }/**
@@ -937,5 +956,32 @@ static inline void skcipher_request_set_crypt( req->iv = iv; } +/** +* skcipher_request_set_unit_size() - submit as multiple data units +* @req: request handle +* @unit_size: unit size in bytes (a multiple of the cipher block size), +* or 0 to disable +* +* Process @req as @cryptlen / @unit_size data units sharing one starting +* @iv, with per-unit IVs derived by treating @iv as a wide counter (the +* data-unit-number convention). @cryptlen must be a positive multiple of +* @unit_size. If the algorithm does not handle multiple units natively, +* the API transparently splits the request into one call per unit; that +* split additionally requires an ivsize that is a non-zero multiple of 8 +* and at most 32 bytes, and rejects a violating request with -EINVAL. An +* algorithm advertising CRYPTO_ALG_REQ_SEG receives the whole request and +* enforces its own constraints. +* +* This function must be called after skcipher_request_set_tfm() and +* skcipher_request_set_callback(), both of which reset @req->unit_size +* to 0. +*/ +static inline void +skcipher_request_set_unit_size (struct skcipher_request *req, +unsigned int unit_size) +{ + req->unit_size = unit_size; +} +#endif /* _CRYPTO_SKCIPHER_H */
Signature · 1 line
2.47.3
PATCH v6 2/6 crypto: acomp - Add bit to indicate segmentation support
In reply to Leonid RavichFrom: Kanchana P Sridhar <[email protected]>
This patch adds segmentation support for compression.
Add a bit to the crypto_alg flags to indicate support for segmentation. Also add a helper for acomp to test whether a given tfm supports segmentation.
--- include/crypto/algapi.h | 5 +++++ include/crypto/internal/acompress.h | 5 +++++ include/linux/crypto.h | 3 +++ 3 files changed, 13 insertions(+)
include/crypto/algapi.h +5 -0
@@ -280,6 +280,11 @@ static inline bool crypto_tfm_req_virt(struct crypto_tfm *tfm)return tfm->__crt_alg->cra_flags &CRYPTO_ALG_REQ_VIRT ; } +static inline bool crypto_tfm_req_seg (struct crypto_tfm *tfm) +{ +return tfm->__crt_alg->cra_flags &CRYPTO_ALG_REQ_SEG ; +} +static inline u32 crypto_request_flags (struct crypto_async_request *req) {return req->flags & ~CRYPTO_TFM_REQ_ON_STACK ;
include/crypto/internal/acompress.h +5 -0
@@ -189,6 +189,11 @@ static inline bool crypto_acomp_req_virt(struct crypto_acomp *tfm)return crypto_tfm_req_virt (&tfm->base); } +static inline bool crypto_acomp_req_seg (struct crypto_acomp *tfm) +{ +return crypto_tfm_req_seg (&tfm->base); +} +void crypto_acomp_free_streams (struct crypto_acomp_streams *s);int crypto_acomp_alloc_streams (struct crypto_acomp_streams *s);
include/linux/crypto.h +3 -0
@@ -139,6 +139,9 @@/* Set if the algorithm cannot have a fallback (e.g., phmac). */ #define CRYPTO_ALG_NO_FALLBACK 0x00080000 +/* Set if the algorithm supports segmentation. */ +#define CRYPTO_ALG_REQ_SEG 0x00100000 +/* The high bits 0xff000000 are reserved for type-specific flags. */ /*
Signature · 1 line
2.47.3
PATCH v6 3/6 crypto: skcipher - add crypto_skcipher_req_seg() helper
In reply to Leonid RavichThin skcipher wrapper over crypto_tfm_req_seg() (mirroring crypto_acomp_req_seg()), so the skcipher mid-layer can ask whether an algorithm advertises native multi-unit (CRYPTO_ALG_REQ_SEG) support.
--- include/crypto/internal/skcipher.h | 5 +++++ 1 file changed, 5 insertions(+)
include/crypto/internal/skcipher.h +5 -0
@@ -249,6 +249,11 @@ static inline bool crypto_skcipher_tested(struct crypto_skcipher *tfm)return tfm_base->__crt_alg->cra_flags &CRYPTO_ALG_TESTED ; } +static inline bool crypto_skcipher_req_seg (struct crypto_skcipher *tfm) +{ +return crypto_tfm_req_seg (&tfm->base); +} +static inline void *skcipher_request_ctx (struct skcipher_request *req) {return req->__ctx;
Signature · 1 line
2.47.3
PATCH v6 4/6 crypto: skcipher - split multi-unit requests in the API layer
In reply to Leonid RavichWhen a caller sets skcipher_request::unit_size and the algorithm does not advertise CRYPTO_ALG_REQ_SEG, transparently split the request in crypto_skcipher_encrypt/decrypt(): one call per data unit, advancing the IV between units as a 64-bit little-endian data-unit-number counter held in the low 8 bytes (the dm-crypt plain64/essiv convention). The counter wraps at 2^64 and never carries into the higher IV bytes, so the output is bit-identical to the per-unit path across the counter rollover; any other on-disk IV format is produced by a template wrapping the algorithm. An algorithm with native multi-unit support gets the whole request unchanged. The eventual goal is for underlying algorithms to gain native support so this path stops triggering.
The split reuses the caller's request for each unit (same tfm, so the request context is already sized) and restores it before returning; req->iv is never modified -- each unit gets a private IV copy the algorithm may clobber. That copy is 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; it reschedules between units when the caller allows sleeping (CRYPTO_TFM_REQ_MAY_SLEEP), since a large batch would otherwise run without a preemption point.
Callers that never set unit_size pay one unlikely() test; the split runs before the lskcipher redirect so lskcipher-backed modes (e.g. cbc) are split correctly too.
--- crypto/skcipher.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)
crypto/skcipher.c +112 -0
@@ -432,6 +432,112 @@ int crypto_skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key, }EXPORT_SYMBOL_GPL (crypto_skcipher_setkey); +/* Bounds the on-stack per-unit IV buffers: 16 covers xts, 32 Adiantum. */ +#define SKCIPHER_MAX_UNIT_IVSIZE 32 + +/* +* Advance the per-unit IV to the next data unit. The data-unit number is a +* 64-bit little-endian counter held in the low 8 bytes of @iv (matching +* dm-crypt's plain64/essiv sector generators, which the caller feeds in as a +* little-endian sector number). It wraps at 2^64 and never carries into the +* higher IV bytes, so batched output stays bit-identical to the per-unit path +* across the counter rollover; any other on-disk IV format is produced by a +* template wrapping the algorithm, not here. +*/ +static void skcipher_unit_iv_next (u8 *iv) +{ +__le64 lo; + +memcpy (&lo, iv,sizeof (lo)); + lo =cpu_to_le64 (le64_to_cpu (lo) +1 ); +memcpy (iv, &lo,sizeof (lo)); +} + +/* +* Transparently split a multi-unit request for an algorithm with no native +* multi-unit support: one call per unit, walking the IV as a wide counter. +* The caller's request is reused for each unit (same tfm, so the request +* context is already correctly sized) and fully restored before returning. +* @req->iv is never modified; each unit gets a private copy the algorithm +* may write back in place (e.g. xts). +*/ +static int skcipher_crypt_unit (struct skcipher_request *req,bool enc) +{ +struct crypto_skcipher *tfm =crypto_skcipher_reqtfm (req); +struct skcipher_alg *alg =crypto_skcipher_alg (tfm); +const unsigned int unit = req->unit_size; +const unsigned int total = req->cryptlen; +const unsigned int ivsize =crypto_skcipher_ivsize (tfm); +bool inplace = req->src == req->dst; +struct scatterlist *o_src = req->src, *o_dst = req->dst; +struct scatter_walk src_walk, dst_walk; +struct scatterlist src_sg[2 ], dst_sg[2 ]; +u8 iv_ctr[SKCIPHER_MAX_UNIT_IVSIZE ]; +/* Becomes req->iv: keep the alignment the caller's IV would have had. */ +u8 iv_unit[SKCIPHER_MAX_UNIT_IVSIZE ]__aligned(MAX_ALGAPI_ALIGNMASK + 1) ; +u8 *o_iv = req->iv; +unsigned int off; +int err =0 ; + +if (!total || !IS_ALIGNED (unit,crypto_skcipher_blocksize (tfm)) || + (total % unit) || !ivsize || + !IS_ALIGNED (ivsize,sizeof (__le64)) || + ivsize >SKCIPHER_MAX_UNIT_IVSIZE ) +return -EINVAL ; + +/* The split is synchronous; only a native (REQ_SEG) alg may be async. */ +if (alg->co.base.cra_flags &CRYPTO_ALG_ASYNC ) +return -EOPNOTSUPP ; + +/* iv_ctr is the counter; iv_unit is the per-unit copy. */ +memcpy (iv_ctr, req->iv, ivsize); + +sg_init_table (src_sg,2 ); +scatterwalk_start (&src_walk, req->src); +if (!inplace) { +sg_init_table (dst_sg,2 ); +scatterwalk_start (&dst_walk, req->dst); + } + + req->unit_size =0 ; + req->cryptlen = unit; + +for (off =0 ; off < total; off += unit) { +scatterwalk_get_sglist (&src_walk, src_sg); +scatterwalk_skip (&src_walk, unit); + req->src = src_sg; +if (inplace) { + req->dst = src_sg; + }else { +scatterwalk_get_sglist (&dst_walk, dst_sg); +scatterwalk_skip (&dst_walk, unit); + req->dst = dst_sg; + } + +memcpy (iv_unit, iv_ctr, ivsize); + req->iv = iv_unit; + err = enc ?crypto_skcipher_encrypt (req) : +crypto_skcipher_decrypt (req); +if (err) +break ; + +skcipher_unit_iv_next (iv_ctr); +/* +* Match dm-crypt's per-sector reschedule, but only when the +* caller allows sleeping (the split can run in atomic context). +*/ +if (req->base.flags &CRYPTO_TFM_REQ_MAY_SLEEP ) +cond_resched (); + } + + req->src = o_src; + req->dst = o_dst; + req->iv = o_iv; + req->cryptlen = total; + req->unit_size = unit; +return err; +} +int crypto_skcipher_encrypt (struct skcipher_request *req) {struct crypto_skcipher *tfm =crypto_skcipher_reqtfm (req);
@@ -439,6 +545,9 @@ int crypto_skcipher_encrypt(struct skcipher_request *req)if (crypto_skcipher_get_flags (tfm) &CRYPTO_TFM_NEED_KEY )return -ENOKEY ; +/* Must precede the lskcipher redirect, which ignores unit_size. */ +if (unlikely (req->unit_size) && !crypto_skcipher_req_seg (tfm)) +return skcipher_crypt_unit (req, true);if (alg->co.base.cra_type != &crypto_skcipher_type)return crypto_lskcipher_encrypt_sg (req);return alg->encrypt (req);
@@ -452,6 +561,9 @@ int crypto_skcipher_decrypt(struct skcipher_request *req)if (crypto_skcipher_get_flags (tfm) &CRYPTO_TFM_NEED_KEY )return -ENOKEY ; +/* Must precede the lskcipher redirect, which ignores unit_size. */ +if (unlikely (req->unit_size) && !crypto_skcipher_req_seg (tfm)) +return skcipher_crypt_unit (req, false);if (alg->co.base.cra_type != &crypto_skcipher_type)return crypto_lskcipher_decrypt_sg (req);return alg->decrypt (req);
Signature · 1 line
2.47.3
PATCH v6 5/6 crypto: testmgr - test multi-unit dispatch
In reply to Leonid RavichCross-check multi-unit dispatch against an independent single-unit reference for every self-tested sync skcipher with an eligible IV:
one batched request (unit_size set) over a deliberately fragmented scatterlist must produce ciphertext byte-identical to N single-unit requests with counter-walked IVs, then round-trip.
The reference increments the IV as a 64-bit little-endian counter in the low 8 bytes -- independent of the API layer's implementation, so the two agree only if the carry-and-wrap is right -- and each unit size is additionally run with an IV seeded to force the counter to wrap back to zero. The caller's IV must come back unmodified. Covers ivsize 16 (xts) and 32 (Adiantum).
--- crypto/testmgr.c | 272 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 272 insertions(+)
crypto/testmgr.c +272 -0
@@ -3211,6 +3211,274 @@ static int test_skcipher(int enc, const struct cipher_test_suite *suite,return 0 ; } +/* Upper bound on the IVs the multi-unit split accepts (16: xts; 32: Adiantum). */ +#define TEST_MDU_MAX_IVSIZE 32 + +/* +* Increment the IV as a 64-bit little-endian data-unit-number counter in the +* low 8 bytes (byte 0 the LSB), wrapping at 2^64 with no carry above -- the +* dm-crypt plain64/essiv convention. Deliberately independent of +* skcipher_unit_iv_next()'s implementation, so the two only agree if the +* carry-and-wrap is right. +*/ +static void test_mdu_iv_inc (u8 *iv) +{ +int i; + +for (i =0 ; i <8 ; i++) +if (++iv[i]) +break ; +} + +/* +* Seed @iv so the low 64-bit counter (bytes [0,8)) is all-ones but its +* least-significant byte: the 2nd increment wraps the counter back to zero, +* exercising the rollover. Bytes outside the low 8 keep their value. +*/ +static void test_mdu_iv_boundary (u8 *iv) +{ +unsigned int i; + +for (i =0 ; i <8 ; i++) + iv[i] =0xff ; + iv[0 ] =0xfe ; +} + +/* Encrypt one du_size block with a plain single-DU request (the reference). */ +static int test_mdu_ref_encrypt (struct crypto_skcipher *tfm,const u8 *in, +u8 *out,unsigned int du_size,const u8 *iv, +unsigned int ivsize) +{ +struct skcipher_request *req; +struct scatterlist sg_in; +DECLARE_CRYPTO_WAIT (wait); +u8 ivbuf[TEST_MDU_MAX_IVSIZE ]; +int err; + + req =skcipher_request_alloc (tfm,GFP_KERNEL ); +if (!req) +return -ENOMEM ; +memcpy (ivbuf, iv, ivsize); +memcpy (out, in, du_size); +sg_init_one (&sg_in, out, du_size); +skcipher_request_set_callback (req,CRYPTO_TFM_REQ_MAY_BACKLOG | +CRYPTO_TFM_REQ_MAY_SLEEP , + crypto_req_done, &wait); +skcipher_request_set_crypt (req, &sg_in, &sg_in, du_size, ivbuf); + err =crypto_wait_req (crypto_skcipher_encrypt (req), &wait); +skcipher_request_free (req); +return err; +} + +/* +* Build an SG over @buf with du_size-unaligned entries, so the splitter's +* per-DU views cross SG entries and exercise the scatter_walk cursor. +*/ +static void test_mdu_sg_fragment (struct scatterlist *sg,unsigned int nents, +u8 *buf,unsigned int total) +{ +unsigned int chunk = total / nents; +unsigned int off =0 , i; + +sg_init_table (sg, nents); +for (i =0 ; i < nents; i++) { +unsigned int len = (i == nents -1 ) ? total - off : chunk; + +sg_set_buf (&sg[i], buf + off, len); + off += len; + } +} + +#define TEST_MDU_NR_UNITS 4 +#define TEST_MDU_NR_FRAGS 5 +/* +* Verify a batched (unit_size-set) request on @tfm is byte-equal to an +* independent N x single-unit reference on the same tfm with +* little-endian-walked IVs, over a fragmented SG, then round-trips. +* @iv_orig is the ivsize-byte starting IV (the caller varies it to exercise +* both a random IV and one seeded to cross a carry boundary). +*/ +static int test_skcipher_multi_du_one (struct crypto_skcipher *tfm, +unsigned int du_size, +const u8 *iv_orig) +{ +const char *driver =crypto_skcipher_driver_name (tfm); +const unsigned int total = du_size *TEST_MDU_NR_UNITS ; +const unsigned int ivsize =crypto_skcipher_ivsize (tfm); +const u32 flags =CRYPTO_TFM_REQ_MAY_BACKLOG |CRYPTO_TFM_REQ_MAY_SLEEP ; +struct skcipher_request *req =NULL ; +struct scatterlist sg[TEST_MDU_NR_FRAGS ], sg_out[TEST_MDU_NR_FRAGS ]; +DECLARE_CRYPTO_WAIT (wait); +u8 iv_work[TEST_MDU_MAX_IVSIZE ], iv_ref[TEST_MDU_MAX_IVSIZE ]; +u8 *plain =NULL , *buf =NULL , *ref =NULL , *obuf =NULL ; +unsigned int u; +int err; + + plain =kmalloc (total,GFP_KERNEL ); + buf =kmalloc (total,GFP_KERNEL ); + ref =kmalloc (total,GFP_KERNEL ); + obuf =kmalloc (total,GFP_KERNEL ); + req =skcipher_request_alloc (tfm,GFP_KERNEL ); +if (!plain || !buf || !ref || !obuf || !req) { + err = -ENOMEM ; + goto out; + } + +get_random_bytes (plain, total); + +/* Reference: per-unit single requests, counter-walked IVs. */ +memcpy (iv_ref, iv_orig, ivsize); +for (u =0 ; u <TEST_MDU_NR_UNITS ; u++) { + err =test_mdu_ref_encrypt (tfm, plain + u * du_size, + ref + u * du_size, du_size, iv_ref, + ivsize); +if (err) { +pr_err ("alg: skcipher: %s multi-DU ref encrypt failed (du=%u): %d\n" , + driver, du_size, err); + goto out; + } +test_mdu_iv_inc (iv_ref); + } + +/* Batched: one request over a fragmented SG. */ +memcpy (buf, plain, total); +memcpy (iv_work, iv_orig, ivsize); +test_mdu_sg_fragment (sg,TEST_MDU_NR_FRAGS , buf, total); +skcipher_request_set_callback (req, flags, crypto_req_done, &wait); +skcipher_request_set_crypt (req, sg, sg, total, iv_work); +skcipher_request_set_unit_size (req, du_size); + err =crypto_wait_req (crypto_skcipher_encrypt (req), &wait); +if (err) { +pr_err ("alg: skcipher: %s multi-DU encrypt failed (du=%u): %d\n" , + driver, du_size, err); + goto out; + } +if (memcmp (buf, ref, total) !=0 ) { +pr_err ("alg: skcipher: %s multi-DU ciphertext differs from single-DU reference (du=%u)\n" , + driver, du_size); + err = -EBADMSG ; + goto out; + } +/* req->iv must be unchanged after multi-DU dispatch. */ +if (memcmp (iv_work, iv_orig, ivsize) !=0 ) { +pr_err ("alg: skcipher: %s multi-DU encrypt mutated caller IV (du=%u)\n" , + driver, du_size); + err = -EBADMSG ; + goto out; + } + +/* Out-of-place: distinct dst SG, fragmented differently from src. */ +memcpy (buf, plain, total); +memset (obuf,0 , total); +test_mdu_sg_fragment (sg,TEST_MDU_NR_FRAGS , buf, total); +test_mdu_sg_fragment (sg_out,TEST_MDU_NR_FRAGS -2 , obuf, total); +skcipher_request_set_callback (req, flags, crypto_req_done, &wait); +skcipher_request_set_crypt (req, sg, sg_out, total, iv_work); +skcipher_request_set_unit_size (req, du_size); + err =crypto_wait_req (crypto_skcipher_encrypt (req), &wait); +if (err) { +pr_err ("alg: skcipher: %s multi-DU out-of-place encrypt failed (du=%u): %d\n" , + driver, du_size, err); + goto out; + } +if (memcmp (obuf, ref, total) !=0 ) { +pr_err ("alg: skcipher: %s multi-DU out-of-place ciphertext differs (du=%u)\n" , + driver, du_size); + err = -EBADMSG ; + goto out; + } + +/* Round-trip the batched ciphertext back to plaintext. */ +memcpy (buf, ref, total); +test_mdu_sg_fragment (sg,TEST_MDU_NR_FRAGS , buf, total); +skcipher_request_set_callback (req, flags, crypto_req_done, &wait); +skcipher_request_set_crypt (req, sg, sg, total, iv_work); +skcipher_request_set_unit_size (req, du_size); + err =crypto_wait_req (crypto_skcipher_decrypt (req), &wait); +if (err) { +pr_err ("alg: skcipher: %s multi-DU decrypt failed (du=%u): %d\n" , + driver, du_size, err); + goto out; + } +if (memcmp (buf, plain, total) !=0 ) { +pr_err ("alg: skcipher: %s multi-DU round-trip mismatch (du=%u)\n" , + driver, du_size); + err = -EBADMSG ; + } + +out: +skcipher_request_free (req); +kfree (obuf); +kfree (ref); +kfree (buf); +kfree (plain); +return err; +} + +/* +* Cross-check multi-unit dispatch against a single-unit reference on @tfm +* over all unit sizes. Returns 0 on success or skip; -EBADMSG on a real +* mismatch. +*/ +static int test_skcipher_multi_du_sizes (struct crypto_skcipher *tfm) +{ +static const unsigned int du_sizes[] = {512 ,1024 ,2048 ,4096 }; +unsigned int ivsize =crypto_skcipher_ivsize (tfm); +u8 iv[TEST_MDU_MAX_IVSIZE ]; +unsigned int j; +int err =0 ; + +for (j =0 ; j <ARRAY_SIZE (du_sizes); j++) { +/* A random starting IV. */ +get_random_bytes (iv, ivsize); + err =test_skcipher_multi_du_one (tfm, du_sizes[j], iv); +if (err) +break ; +/* And one seeded to carry across a 64-bit limb. */ +get_random_bytes (iv, ivsize); +test_mdu_iv_boundary (iv); + err =test_skcipher_multi_du_one (tfm, du_sizes[j], iv); +if (err) +break ; +cond_resched (); + } +return err; +} + +/* +* Cross-check multi-unit dispatch against a single-unit reference for every +* eligible ivsize (16: xts; 32: Adiantum). +*/ +static int test_skcipher_multi_du (struct crypto_skcipher *tfm) +{ +unsigned int ivsize =crypto_skcipher_ivsize (tfm); +unsigned int blocksize =crypto_skcipher_blocksize (tfm); +u8 keybuf[128 ]; +unsigned int keylen; +int err; + +if (noslowtests) +return 0 ; + +/* Mirror the API-layer split's eligibility; skip what it rejects. */ +if (!ivsize || ivsize %sizeof (__le64) || ivsize >TEST_MDU_MAX_IVSIZE ) +return 0 ; +if (!blocksize ||512 % blocksize) +return 0 ;/* unit sizes below are 512-multiples */ +if (crypto_skcipher_alg (tfm)->co.base.cra_flags &CRYPTO_ALG_ASYNC ) +return 0 ;/* the transparent split is sync-only */ + + keylen =crypto_skcipher_min_keysize (tfm); +if (keylen >sizeof (keybuf)) +return 0 ;/* unusually large key; skip rather than overflow */ +get_random_bytes (keybuf, keylen); + err =crypto_skcipher_setkey (tfm, keybuf, keylen); +if (err) +return 0 ;/* weak/rejected key (e.g. XTS equal halves): skip */ + +return test_skcipher_multi_du_sizes (tfm); +} +static int alg_test_skcipher (const struct alg_test_desc *desc,const char *driver,u32 type,u32 mask) {
@@ -3259,6 +3527,10 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,if (err) goto out; + err =test_skcipher_multi_du (tfm); +if (err) + goto out; + err =test_skcipher_vs_generic_impl (desc->generic_driver, req, tsgls); out:free_cipher_test_sglists (tsgls);
Signature · 1 line
2.47.3
PATCH v6 6/6 dm crypt: batch a bio segment's sectors via multi-unit requests
In reply to Leonid RavichFor eligible configurations, submit one skcipher request per contiguous bio segment instead of one per sector: set skcipher_request::unit_size = cc->sector_size and hand the crypto API the whole segment (e.g. the default 512-byte sector with a 4 KiB bio_vec -> one request of 8 data units), using only the existing inline single-entry scatterlist -- no per-bio allocation.
Eligible means the per-sector IV is a little-endian data-unit-number counter in the low 64 bits with a step of exactly one per unit: plain64 and essiv (essiv qualifies because its IV input is le64(sector) -- the salt encryption lives in the essiv() template), single-tfm, non-aead, sector_size 512 or iv_large_sectors, and no integrity metadata (per-sector tags/IVs need the per-sector loop). plain64be is not
batched: its on-disk IV is a big-endian counter in the high 8 bytes, not the little-endian low-limb layout the API-layer split walks, so it keeps the per-sector path (batching it would need a template producing that layout). Everything else likewise keeps the existing one-sector-per-request path unchanged. Since the API-layer transparent split is synchronous, an async cipher batches only if it handles multi-unit requests natively (CRYPTO_ALG_REQ_SEG).
Batching is byte-for-byte identical to the per-sector path: ciphertext verified bit-identical to an unpatched baseline for plain64 and essiv.
--- drivers/md/dm-crypt.c | 136 +++++++++++++++++++++++++++++++++++------- 1 file changed, 114 insertions(+), 22 deletions(-)
drivers/md/dm-crypt.c +114 -22
@@ -115,6 +115,15 @@ struct crypt_iv_operations { struct dm_crypt_request *dmreq);void (*post)(struct crypt_config *cc,u8 *iv,struct dm_crypt_request *dmreq); + +/* +* Set for IV modes whose per-sector IV is a little-endian +* data-unit-number counter (IV(s+i) == IV(s)+i) placed in the low +* 64-bit limb, enabling multi-unit batching via the skcipher API-layer +* split. Clear for non-counter modes (lmk, tcw, ...) and for counter +* modes whose on-disk IV is not that layout (e.g. plain64be). +*/ +bool unit_counter; };struct iv_benbi_private {
@@ -151,6 +160,7 @@ enum cipher_flags {CRYPT_IV_LARGE_SECTORS ,/* Calculate IV from sector_size, not 512B sectors */ CRYPT_ENCRYPT_PREPROCESS ,/* Must preprocess data for encryption (elephant) */ CRYPT_KEY_MAC_SIZE_SET ,/* The integrity_key_size option was used */ + CRYPT_MULTI_DATA_UNIT, /* Batch a bio segment's sectors per crypto request */ };/*
@@ -1018,15 +1028,23 @@ static const struct crypt_iv_operations crypt_iv_plain_ops = { };static const struct crypt_iv_operations crypt_iv_plain64_ops = { - .generator = crypt_iv_plain64_gen + .generator = crypt_iv_plain64_gen, + .unit_counter = true, };static const struct crypt_iv_operations crypt_iv_plain64be_ops = { - .generator = crypt_iv_plain64be_gen + .generator = crypt_iv_plain64be_gen, +/* +* No unit_counter: the big-endian, high-limb on-disk layout is not the +* little-endian low-limb counter the API-layer split walks. Batching +* it needs a template producing this layout; unbatched for now. +*/ };static const struct crypt_iv_operations crypt_iv_essiv_ops = { - .generator = crypt_iv_essiv_gen + .generator = crypt_iv_essiv_gen, +/* IV input is le64(sector); the salt-encrypt lives in essiv(). */ + .unit_counter = true, };static const struct crypt_iv_operations crypt_iv_benbi_ops = {
@@ -1349,21 +1367,51 @@ static int crypt_convert_block_aead(struct crypt_config *cc,return r; } +/* +* Bytes to process in one skcipher request: a whole contiguous segment when +* batching (multi-data-unit), else one sector. 0 means an unusable +* (sub-sector / misaligned) segment. +*/ +static unsigned int crypt_skcipher_len (struct crypt_config *cc, +const struct bio_vec *bv_in, +const struct bio_vec *bv_out) +{ +const unsigned int sector_size = cc->sector_size; + +if (test_bit (CRYPT_MULTI_DATA_UNIT , &cc->cipher_flags)) +return round_down (min (bv_in->bv_len, bv_out->bv_len), + sector_size); + +/* Reject unexpected unaligned bio. */ +if (unlikely (bv_in->bv_len & (sector_size -1 ))) +return 0 ; +return sector_size; +} + +/* +* Encrypt/decrypt one bio segment (one sector, or a whole segment when +* batching) and report the bytes done in *out_processed. The integrity / +* preprocess / post handling is inert when batching (crypt_can_batch_units() +* excludes those configs). +*/ static int crypt_convert_block_skcipher (struct crypt_config *cc,struct convert_context *ctx,struct skcipher_request *req, -unsigned int tag_offset) +unsigned int tag_offset, +unsigned int *out_processed) {struct bio_vec bv_in =bio_iter_iovec (ctx->bio_in, ctx->iter_in);struct bio_vec bv_out =bio_iter_iovec (ctx->bio_out, ctx->iter_out); +const unsigned int sector_size = cc->sector_size;struct scatterlist *sg_in, *sg_out;struct dm_crypt_request *dmreq;u8 *iv, *org_iv, *tag_iv;__le64 *sector; +unsigned int len;int r =0 ; -/* Reject unexpected unaligned bio. */ -if (unlikely (bv_in.bv_len & (cc->sector_size -1 ))) + len =crypt_skcipher_len (cc, &bv_in, &bv_out); +if (unlikely (!len))return -EIO ; dmreq =dmreq_of_req (cc, req);
@@ -1386,10 +1434,10 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc, sg_out = &dmreq->sg_out[0 ];sg_init_table (sg_in,1 ); -sg_set_page (sg_in, bv_in.bv_page, cc->sector_size, bv_in.bv_offset); +sg_set_page (sg_in, bv_in.bv_page, len, bv_in.bv_offset);sg_init_table (sg_out,1 ); -sg_set_page (sg_out, bv_out.bv_page, cc->sector_size, bv_out.bv_offset); +sg_set_page (sg_out, bv_out.bv_page, len, bv_out.bv_offset);if (cc->iv_gen_ops) {/* For READs use IV stored in integrity metadata */
@@ -1410,7 +1458,9 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,memcpy (iv, org_iv, cc->iv_size); } -skcipher_request_set_crypt (req, sg_in, sg_out, cc->sector_size, iv); +skcipher_request_set_crypt (req, sg_in, sg_out, len, iv); +if (test_bit (CRYPT_MULTI_DATA_UNIT , &cc->cipher_flags)) +skcipher_request_set_unit_size (req, sector_size);if (bio_data_dir (ctx->bio_in) ==WRITE ) r =crypto_skcipher_encrypt (req);
@@ -1420,9 +1470,10 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post) cc->iv_gen_ops->post (cc, org_iv, dmreq); -bio_advance_iter (ctx->bio_in, &ctx->iter_in, cc->sector_size); -bio_advance_iter (ctx->bio_out, &ctx->iter_out, cc->sector_size); +bio_advance_iter (ctx->bio_in, &ctx->iter_in, len); +bio_advance_iter (ctx->bio_out, &ctx->iter_out, len); + *out_processed = len;return r; }
@@ -1509,13 +1560,25 @@ static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_crypt_free_req_skcipher (cc, req, base_bio); } +/* +* Advance the IV-sector and integrity-tag cursors by @processed bytes; the +* bio iterators are advanced by the per-block helpers themselves. +*/ +static void crypt_convert_advance (struct crypt_config *cc, +struct convert_context *ctx, +unsigned int processed) +{ + ctx->cc_sector += processed >>SECTOR_SHIFT ; + ctx->tag_offset += processed / cc->sector_size; +} +/* * Encrypt / decrypt data from one bio to another one (can be the same one) */ static blk_status_t crypt_convert (struct crypt_config *cc,struct convert_context *ctx,bool atomic,bool reset_pending) { -unsigned int sector_step = cc->sector_size >>SECTOR_SHIFT ; +unsigned int processed;int r;/*
@@ -1536,10 +1599,12 @@ static blk_status_t crypt_convert(struct crypt_config *cc,atomic_inc (&ctx->cc_pending); + processed = cc->sector_size;if (crypt_integrity_aead (cc)) r =crypt_convert_block_aead (cc, ctx, ctx->r.req_aead, ctx->tag_offset);else - r =crypt_convert_block_skcipher (cc, ctx, ctx->r.req, ctx->tag_offset); + r =crypt_convert_block_skcipher (cc, ctx, ctx->r.req, + ctx->tag_offset, &processed);switch (r) {/*
@@ -1559,8 +1624,7 @@ static blk_status_t crypt_convert(struct crypt_config *cc,* exit and continue processing in a workqueue */ ctx->r.req =NULL ; - ctx->tag_offset++; - ctx->cc_sector += sector_step; +crypt_convert_advance (cc, ctx, processed);return BLK_STS_DEV_RESOURCE ; } }else {
@@ -1574,16 +1638,14 @@ static blk_status_t crypt_convert(struct crypt_config *cc,*/ case -EINPROGRESS : ctx->r.req =NULL ; - ctx->tag_offset++; - ctx->cc_sector += sector_step; +crypt_convert_advance (cc, ctx, processed);continue ;/* * The request was already processed (synchronously). */ case 0 :atomic_dec (&ctx->cc_pending); - ctx->cc_sector += sector_step; - ctx->tag_offset++; +crypt_convert_advance (cc, ctx, processed);if (!atomic)cond_resched ();continue ;
@@ -2345,12 +2407,28 @@ static int crypt_alloc_tfms_aead(struct crypt_config *cc, char *ciphermode)return 0 ; } +/* +* Whether multi-unit batching applies: a counter IV mode (unit_counter set), +* single-tfm, non-aead, and a per-unit IV step of exactly one (512B sectors +* or iv_large_sectors). The IV must also satisfy the API split's counter +* constraints (non-zero multiple of 8, <= 32 bytes). Integrity is excluded +* in crypt_ctr_cipher(), which runs after integrity is configured. +*/ +static bool crypt_can_batch_units (struct crypt_config *cc) +{ +return !crypt_integrity_aead (cc) && cc->tfms_count ==1 && + cc->iv_gen_ops && cc->iv_gen_ops->unit_counter && + cc->iv_size &&IS_ALIGNED (cc->iv_size,sizeof (__le64)) && + cc->iv_size <=32 && + (cc->sector_size == (1 <<SECTOR_SHIFT ) || +test_bit (CRYPT_IV_LARGE_SECTORS , &cc->cipher_flags)); +} +static int crypt_alloc_tfms (struct crypt_config *cc,char *ciphermode) {if (crypt_integrity_aead (cc))return crypt_alloc_tfms_aead (cc, ciphermode); -else -return crypt_alloc_tfms_skcipher (cc, ciphermode); +return crypt_alloc_tfms_skcipher (cc, ciphermode); }static unsigned int crypt_subkey_size (struct crypt_config *cc)
@@ -2999,7 +3077,6 @@ static int crypt_ctr_cipher_old(struct dm_target *ti, char *cipher_in, char *key goto bad_mem; } -/* Allocate cipher */ ret =crypt_alloc_tfms (cc, cipher_api);if (ret <0 ) { ti->error ="Error allocating crypto tfm" ;
@@ -3063,6 +3140,21 @@ static int crypt_ctr_cipher(struct dm_target *ti, char *cipher_in, char *key) } } +/* +* Enable multi-unit batching for an eligible config with no integrity +* (integrity is set up after cipher alloc, hence the re-check here). +* The API layer's transparent split is synchronous, so an async cipher +* batches only if it handles multi-unit requests natively. +*/ +if (crypt_can_batch_units (cc) && !cc->integrity_tag_size && + !cc->integrity_iv_size && + (crypto_skcipher_alg (any_tfm (cc))->co.base.cra_flags & + (CRYPTO_ALG_ASYNC |CRYPTO_ALG_REQ_SEG )) !=CRYPTO_ALG_ASYNC ) { +set_bit (CRYPT_MULTI_DATA_UNIT , &cc->cipher_flags); +DMINFO ("Using multi-data-unit crypto offload (du=%u)" , + cc->sector_size); + } +/* wipe the kernel key payload copy */ if (cc->key_string)memset (cc->key,0 , cc->key_size *sizeof (u8));
Signature · 1 line
2.47.3
Quoted 4 lines · Leonid Ravich
* 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.
So what is the use case? So far all crypto driver we've seen have shown to be slower than cpu. Which one are you using that isn't? And if you have a genuinely useful one, should we have a proper interface to it that doesn't pay the scatterlist overhead to start with?
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Leonid Ravich
* 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.So what is the use case? So far all crypto driver we've seen have shown to be slower than cpu. Which one are you using that isn't?
The engine we use is out of tree, but it is not a special case. It is an SoC-integrated, DMA-driven, asynchronous xts(aes) engine of the same class as caam, ccree, qce, hisilicon sec2, inside-secure and the Marvell CPT drivers already in the tree. Like those, it pays a fixed cost per request (descriptor setup, doorbell, completion) that dm-crypt's one-request-per-sector pattern multiplies by the number of sectors. So the problem is common to the whole class, not specific to our hardware.
You are right that such engines are generally not faster than the CPU in raw throughput, and I am not claiming ours is. What they give is
offload: when dm-crypt batches a bio into a single request, the engine does the work that the CPU would otherwise do. In our measurements that cuts CPU utilisation by roughly 20-40% for the same I/O. On systems with few or small cores, those cycles are worth more than peak throughput. Without batching the per-sector request overhead eats that gain, which is why the series targets the request pattern rather than the cipher.
Quoted 3 lines
And if you have a genuinely useful one, should we have a proper interface to it that doesn't pay the scatterlist overhead to start with?
For a DMA engine the scatterlist is not overhead: it is what the hardware consumes. The per-sector cost it pays is the per-request cost, and unit_size is what removes it.
For the software path, v6 follows what Herbert suggested in the v4 thread [1]: the per-unit loop moves from the caller into the Crypto API, and it can then move further down into each algorithm, so the per-unit calls become direct calls and only the single call into the API stays indirect. That can improve on the status quo rather than just match it. v6 is the first step (the mid-layer split); per-algorithm native splitting via CRYPTO_ALG_REQ_SEG is the next.
The interface is also deliberately the one being introduced for acomp in the batching series [2]: the same unit_size field and setter semantics, and the same CRYPTO_ALG_REQ_SEG bit (patch 2 here is Herbert's patch from that series). skcipher and acomp would share one model for multi-unit requests, with a native path for hardware and a mid-layer fallback for everything else, rather than skcipher growing a separate interface.
If you had a different shape in mind for the hardware path, I would rather build that than guess.
[1] https://lore.kernel.org/linux-block/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
Thanks,
Leonid
Quoted 11 lines · Leonid Ravich
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Leonid Ravich
* 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.So what is the use case? So far all crypto driver we've seen have shown to be slower than cpu. Which one are you using that isn't?
The engine we use is out of tree, but it is not a special case. It
It is. Without you having an upstream stack this is a complete no-go to start with.
Quoted 3 lines
is an SoC-integrated, DMA-driven, asynchronous xts(aes) engine of the same class as caam, ccree, qce, hisilicon sec2, inside-secure and the Marvell CPT drivers already in the tree.
Pretty much all of them are broken as f***k everytime someone tried to use them.
Quoted 3 lines
For a DMA engine the scatterlist is not overhead: it is what the hardware consumes. The per-sector cost it pays is the per-request cost, and
unit_sizeis what removes it.
Yes, it is. Take a look how the scatterlist duplicates information already handled by the upper layers. We've been through is a lot and are moving to the dma_iova_ API because of that.
Herbert Xu
maintainerQuoted 7 lines · Leonid Ravich
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:
Quoted 8 lines
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).
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.
We really should only do it once, either in the DM layer or in the Crypto API. It should be transmitted via memory to the other side.
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.
Thanks,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Quoted 4 lines · Herbert Xu
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.
Quoted 11 lines
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