Skip to contentSkip to conversation

crypto: skcipher - multi-data-unit dispatch as a template

PATCH Superseded Leonid Ravich 4 participants 15 messages Jun 30 – Sep 19

Subsystems (5)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v5 cover crypto: skcipher - multi-data-unit dispatch as a template

This is v5. It reworks the multi-data-unit support from the in-core auto-splitter of v4 into a crypto template, dun(...), addressing the v4
review: there is now no added cost on the core skcipher path, no per-algorithm capability flag, and the per-data-unit split lives in an algorithm rather than in crypto_skcipher_encrypt/decrypt the shape Herbert suggested, which removes the "overhead for everyone" Eric objected to.

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

Model
---

A skcipher_request gains a data_unit_size field (patch 1). When set, the request covers cryptlen / data_unit_size data units sharing one starting IV; per-unit IVs are derived from the IV as a wide data-unit- number (DUN) counter the convention blk-crypto already uses for inline encryption.

dun(...) (patch 2) is a template that wraps an inner skcipher whose IV is that counter (e.g. dun(xts(aes),le)). Its ->encrypt/->decrypt split the request into one inner call per data unit, walking the IV +1 each unit; each inner call is direct, so only the outer dispatch into the API is indirect. A plain skcipher is unchanged and ignores data_unit_size, so existing callers pay nothing the field is inert and the core en/decrypt path is untouched.

The second template parameter selects how the per-unit IV advances. A neighbour relates by a +1 step in exactly one of two ways, little- or big-endian, so dun(...,le) / dun(...,be) is a closed parameter space, not an open-ended set of "IV types". Internally each is one row of a small struct dun_mode op table (an iv_next walk plus an ivsize predicate); adding a future convention e.g. a width-bounded counter, or an affine sector<<shift+k step is one row, with the dispatch loop unchanged. IV constructions that are not such a counter are simply not wrapped (the consumer keeps its per-unit path); an IV that is encrypted (essiv) composes as the inner algorithm, dun(essiv(...),le), since the encryption already lives in that inner template.

Why a template
--------------

- No core cost for anyone. crypto_skcipher_encrypt/decrypt are stock; only a dun() tfm reads data_unit_size. (addresses Eric's "adds checks/overhead for everyone")

- No capability flag. A hardware engine that handles a whole multi-DU request in one pass registers its own dun(xts(aes),le) at a higher cra_priority and is picked automatically exactly how xts-aes-aesni already beats generic xts. No CRYPTO_ALG_* bit, no core branch choosing native-vs-split. Such a native driver may also be async (it owns its dispatch); only the generic template is sync-only.

- The split is in the algorithm. (the direction Herbert described)

- It is the same kind of wrapper crypto/ already has. Like cryptd() (async dispatch) and pcrypt() (parallel dispatch), dun() wraps an inner skcipher and changes only how the request is dispatched here, split across data units performing no cipher transform of its own.

- It is a reusable primitive, not a dm-crypt feature. Two in-tree consumers are included: dm-crypt (patch 4) and blk-crypto-fallback (patch 5), which both open-code the per-DUN loop today; fscrypt's direct (non-inline) path open-codes the same loop and could follow. A HW engine is a provider via cra_priority. Consumers and providers are decoupled through one named algorithm.

What it does and does not buy
-----------------------------

On a software cipher this is not a throughput win: the generic template still issues one inner encrypt per data unit, so the AES compute is unchanged. It removes per-request overhead and the consumer's open-coded per-unit loop, and is byte-for-byte identical to the per-sector path (Verification). The win is for a one-pass provider; no software throughput is claimed.

dm-crypt consumer (patch 4)
---------------------------

dm-crypt submits one request per contiguous bio segment with data_unit_size = cc->sector_size (e.g. the default 512-byte sector with a 4 KiB bio_vec -> one request of 8 data units), using only its existing inline single-entry scatterlist no per-bio allocation, no regression. It allocates dun(<cipher>,<endian>) instead of the bare cipher when the config can form the DUN counter: a counter IV mode (plain64 -> le, plain64be -> be; essiv/lmk/tcw etc. are not plain counters and stay per-sector), single-tfm, non-aead, sector_size 512 or iv_large_sectors. DM_CRYPT selects CRYPTO_DUN and the template resolves against a sync inner, so there is no acceptable wrap failure the bare cipher would survive; an integrity config keeps an inert dun() wrapper but never batches (one inner call per request == the per-sector path).

blk-crypto-fallback consumer (patch 5)
--------------------------------------

Every blk-crypto inline-encryption mode feeds the DUN as a little-endian counter, so the fallback wraps its cipher as dun(<cipher>,le) unconditionally (BLK_INLINE_ENCRYPTION_FALLBACK selects CRYPTO_DUN). Because the template handles any counter width up to 32 bytes, this covers all four modes AES-256-XTS, AES-128-CBC-ESSIV, Adiantum (32-byte IV) and SM4-XTS and the open-coded per-unit loop is removed from both the encrypt and decrypt paths.

Verification
------------

Regression protocol in the tree, on x86 + arm64 under qemu: build clean and checkpatch strict clean (the lone warning is the new-file MAINTAINERS reminder; crypto/ is an F: catch-all); testmgr dun() cross-check (batched == N x single-DU reference over a fragmented scatterlist, plus a boundary-seeded IV that forces a carry across a 64-bit limb / byte run) for every accepted ivsize including 32 (Adiantum) in BOTH dun(...,le) and dun(...,be), so the big-endian counter path is exercised independently of any consumer; an AF_ALG probe forces the dun() cross-check to run for each blk-crypto inner cipher (dun(essiv(cbc(aes),sha256),le), dun(adiantum(xchacha12,aes),le), ...); dm-crypt plain64/plain64be activate dun() (le/be), essiv / plain fall back; negative gates (multikey and integrity not batched); plain64 and plain64be round-trips and a 4096-byte iv_large_sectors round-trip; low-memory; arm64 functional; an end-to-end blk-crypto-fallback test (ext4 + fscrypt -o inlinecrypt with no inline HW, driving dun(xts,le) and verifying a post-cache-drop round-trip); and byte-equivalence:
ciphertext is bit-identical to an unpatched axboe/for-next baseline (sha256 4913910b...43efc0 le, da0869a9...63004 be).

Changes since v4
----------------

- The in-core auto-splitter and validator are gone; multi-DU dispatch is the dun(...) template. crypto_skcipher_encrypt/decrypt revert to stock, so there is no added cost on the core path.
- The CRYPTO_ALG_SKCIPHER_NATIVE_MULTI_DU capability flag is dropped; a native one-pass driver is selected by cra_priority instead.
- The template is dun(<inner>,<endian>) in the cryptd()/pcrypt() family of dispatch-only wrappers; the counter endianness (le/be) is its second parameter, backed by a struct dun_mode op table so a future counter convention is one table row. It handles any counter width up to 32 bytes (covering Adiantum) and rejects a data_unit_size 0 / cryptlen 0 request.
- dm-crypt allocates dun(<cipher>,le|be) when eligible (selecting the IV mode before tfm allocation); plain64 -> le, plain64be -> be. An integrity config keeps an inert dun() wrapper but never batches. DM_CRYPT selects CRYPTO_DUN.
- blk-crypto-fallback is a second consumer (patch 5), demonstrating the template is a shared primitive, not dm-crypt-only; it wraps every mode as dun(<cipher>,le) and BLK_INLINE_ENCRYPTION_FALLBACK selects CRYPTO_DUN.
- testmgr exercises the template via dun(<inner>,le) and dun(<inner>,be), including ivsize 32 and a carry-boundary IV; an end-to-end fscrypt -o inlinecrypt test drives the blk-crypto-fallback consumer.

Leonid Ravich (5):
crypto: skcipher - add per-request data_unit_size
crypto: dun - data-unit-number dispatch template
crypto: testmgr - test dun() dispatch
dm crypt: batch a bio segment's sectors via dun()
blk-crypto: fallback - batch a segment's data units via dun()

 block/Kconfig               |   1 +
 block/blk-crypto-fallback.c |  74 ++++----
 crypto/Kconfig              |  14 ++
 crypto/Makefile             |   1 +
 crypto/dun.c                | 359 ++++++++++++++++++++++++++++++++++++
 crypto/testmgr.c            | 289 +++++++++++++++++++++++++++++
 drivers/md/Kconfig          |   1 +
 drivers/md/dm-crypt.c       | 208 ++++++++++++++++-----
 include/crypto/skcipher.h   |  34 ++++
 9 files changed, 899 insertions(+), 82 deletions(-)
 create mode 100644 crypto/dun.c

base-commit: a8cafdf8c949f17c92eca0045532e88ac0dac30d

Signature · 1 line
2.47.3
#1 lore raw

PATCH v5 1/5 crypto: skcipher - add per-request data_unit_size

Add a data_unit_size field to struct skcipher_request. When non-zero, the request covers cryptlen / data_unit_size data units that share one starting IV; per-unit IVs are derived from the request IV as a wide data-unit-number counter (the convention also used by blk-crypto for inline encryption). cryptlen must be a positive multiple of data_unit_size.

The field is honoured by an skcipher that understands data units -- an instance of the dun(...) template (added next), or a driver that handles a whole multi-DU request natively. A plain skcipher ignores it, so the field is inert for every existing caller; the core en/decrypt path is unchanged. skcipher_request_set_tfm() and the on-stack request initialiser reset it to 0 so a reused request defaults to single-DU.

---
 include/crypto/skcipher.h | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
include/crypto/skcipher.h +34 -0
index 4efe2ca8c4d1..1121be80cb53 100644
@@ -31,6 +31,13 @@ struct scatterlist;
 /**
  *	struct skcipher_request - Symmetric key cipher request
  *	@cryptlen: Number of bytes to encrypt or decrypt
+ *	@data_unit_size: Size in bytes of each data unit, or 0 for a
+ *		single-data-unit request (the default).  When non-zero,
+ *		must be a multiple of the cipher block size, @cryptlen must
+ *		be a positive multiple of it, and per-DU IVs are derived from
+ *		@iv as a wide counter (the data-unit-number convention); the
+ *		counter width and endianness are chosen by the consumer (e.g.
+ *		the dun() template's second parameter).
  *	@iv: Initialisation Vector
  *	@src: Source SG list
  *	@dst: Destination SG list
@@ -39,6 +46,7 @@ struct scatterlist;
  */
 struct skcipher_request {
 	unsigned int cryptlen;
+	unsigned int data_unit_size;
 
 	u8 *iv;
 
@@ -225,6 +233,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)->data_unit_size = 0, \
 		 (void *)__##name##_desc)
 
 /**
@@ -819,6 +828,8 @@ static inline void skcipher_request_set_tfm(struct skcipher_request *req,
 					    struct crypto_skcipher *tfm)
 {
 	req->base.tfm = crypto_skcipher_tfm(tfm);
+	/* Reused requests default to single-data-unit. */
+	req->data_unit_size = 0;
 }
 
 static inline void skcipher_request_set_sync_tfm(struct skcipher_request *req,
@@ -937,5 +948,28 @@ static inline void skcipher_request_set_crypt(
 	req->iv = iv;
 }
 
+/**
+ * skcipher_request_set_data_unit_size() - submit as multiple data units
+ * @req: request handle
+ * @data_unit_size: data-unit size in bytes (a multiple of the cipher block
+ *		    size), or 0 to disable
+ *
+ * Process @req as @cryptlen / @data_unit_size data units sharing one starting
+ * @iv, with per-DU IVs derived by treating @iv as a wide counter (the data-
+ * unit-number convention).  @cryptlen must be a positive multiple of
+ * @data_unit_size.  This is honoured only by a tfm that understands data
+ * units -- an instance of the dun(...) template (which splits the request
+ * into one inner call per unit, with the counter endianness given as its
+ * second parameter), or a driver that consumes a whole multi-DU request
+ * natively, which rejects a request violating these constraints with -EINVAL.
+ * A plain skcipher ignores the field.
+ */
+static inline void
+skcipher_request_set_data_unit_size(struct skcipher_request *req,
+				    unsigned int data_unit_size)
+{
+	req->data_unit_size = data_unit_size;
+}
+
 #endif	/* _CRYPTO_SKCIPHER_H */
 
Signature · 1 line
2.47.3
maintainer
#2 lore raw

Re: [PATCH v5 1/5] crypto: skcipher - add per-request data_unit_size

include/crypto/skcipher.h struct scatterlist; · line 49

Quoted 6 lines · Leonid Ravich
include/crypto/skcipher.h +1 -0
@@ -39,6 +46,7 @@ struct scatterlist;
  */
 struct skcipher_request {
 	unsigned int cryptlen;
+	unsigned int data_unit_size;

Please rename to unit_size to align with the proposed acomp patches.

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
#3 lore raw

PATCH v5 2/5 crypto: dun - data-unit-number dispatch template

In reply to Leonid Ravich

Add a dun(...) skcipher template that wraps an inner skcipher whose IV is a wide data-unit-number counter (e.g. dun(xts(aes),le)). When the caller sets skcipher_request::data_unit_size, the template splits the request into cryptlen / data_unit_size sub-requests on the inner cipher, walking the IV +1 per unit. Each inner ->encrypt/->decrypt is a direct call, so only the outer dispatch into the crypto API is indirect -- the per-unit work is not.

The second template parameter selects the counter endianness: dun(...,le) for a little-endian counter (dm-crypt plain64, blk-crypto inline encryption) and dun(...,be) for a big-endian one (dm-crypt plain64be). Those are the only two ways a per-unit IV relates to its neighbour by a +1 step; IV modes that are not such a counter are simply not wrapped. Like cryptd() and pcrypt(), dun() wraps an inner skcipher and changes only how the request is dispatched -- here, split across data units -- performing no cipher transform of its own.

A dun() tfm exists solely for multi-DU dispatch, so a request with data_unit_size 0 is rejected with -EINVAL; a caller wanting plain single-DU encryption uses the inner skcipher.

A hardware engine that consumes a whole multi-DU request in one pass registers its own dun(...) at a higher cra_priority and is selected automatically by the existing priority mechanism; no per-algorithm capability flag is needed. The generic template is sync-only (the split loop treats any non-zero inner return as terminal), so it resolves against a sync inner cipher (mask | CRYPTO_ALG_ASYNC); async is left to such native drivers.

The inner IV must be a whole number of 64-bit limbs and no wider than 32
bytes: 16 covers xts(...), 32 covers the widest inline-encryption mode (Adiantum).

---
 crypto/Kconfig  |  14 ++
 crypto/Makefile |   1 +
 crypto/dun.c    | 359 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 374 insertions(+)
 create mode 100644 crypto/dun.c
crypto/Kconfig +14 -0
index 103d1f58cb7c..4f90a780c4fc 100644
@@ -746,6 +746,20 @@ config CRYPTO_XTS
 	  implementation currently can't handle a sectorsize which is not a
 	  multiple of 16 bytes.
 
+config CRYPTO_DUN
+	tristate "Data-unit-number (DUN) dispatch template"
+	select CRYPTO_SKCIPHER
+	select CRYPTO_MANAGER
+	help
+	  dun(...) wraps an skcipher whose IV is a wide data-unit-number
+	  counter (e.g. xts(aes)) and lets a caller submit several data units
+	  sharing one starting IV in a single request, via
+	  skcipher_request::data_unit_size.  The counter endianness is the
+	  second parameter: dun(xts(aes),le) or dun(xts(aes),be).  The template
+	  splits the request into one inner call per data unit; a hardware
+	  driver may register a higher-priority dun(...) that handles the whole
+	  request in one pass.  The first user is dm-crypt.
+
 endmenu
 
 menu "AEAD (authenticated encryption with associated data) ciphers"
crypto/Makefile +1 -0
index 162242593c7c..584d9e8c4347 100644
@@ -93,6 +93,7 @@ obj-$(CONFIG_CRYPTO_PCBC) += pcbc.o
 obj-$(CONFIG_CRYPTO_CTS) += cts.o
 obj-$(CONFIG_CRYPTO_LRW) += lrw.o
 obj-$(CONFIG_CRYPTO_XTS) += xts.o
+obj-$(CONFIG_CRYPTO_DUN) += dun.o
 obj-$(CONFIG_CRYPTO_CTR) += ctr.o
 obj-$(CONFIG_CRYPTO_XCTR) += xctr.o
 obj-$(CONFIG_CRYPTO_HCTR2) += hctr2.o
crypto/dun.c new file +359 -0

359 lines not on this page · raw message

Signature · 1 line
2.47.3
maintainer
#4 lore raw

Re: [PATCH v5 2/5] crypto: dun - data-unit-number dispatch template

Quoted 7 lines · Leonid Ravich

Add a dun(...) skcipher template that wraps an inner skcipher whose IV is a wide data-unit-number counter (e.g. dun(xts(aes),le)). When the caller sets skcipher_request::data_unit_size, the template splits the request into cryptlen / data_unit_size sub-requests on the inner cipher, walking the IV +1 per unit. Each inner ->encrypt/->decrypt is a direct call, so only the outer dispatch into the crypto API is indirect -- the per-unit work is not.

This shouldn't be a template. The default data-unit handling should go into the mid-API layer (so skcipher.c). It should transparently split things up *if* the underlying algorithm does not support multiple units.

The eventual goal of course it to add unit support to all underlying algorithms so that the mid-API code does not trigger. But for your patch-set this would keep the work to a minimum.

Again you should align this with the existing acomp patch-set:

https://lore.kernel.org/all/[email protected]/

In face as the acomp work has not been merged yet, just cherry-pick that patch.

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
#5 lore raw

Re: [PATCH v5 2/5] crypto: dun - data-unit-number dispatch template

Quoted 4 lines · Herbert Xu

This shouldn't be a template. The default data-unit handling should go into the mid-API layer (so skcipher.c). It should transparently split things up *if* the underlying algorithm does not support multiple units.

Done for v6 (mid-API split in skcipher.c, cherry-picking acomp CRYPTO_ALG_REQ_SEG / segmentation-wrapper patch as you suggested). One design fork I'd like your call on before I resend.

I benchmarked the mid-API split vs the legacy per-sector loop on r7i.metal (VAES-AVX512): dm-crypt shows no measurable throughput or latency regression, but a microbench isolates a fixed ~50 ns per 512B unit. It's a fixed per-call cost: the split
copies the counter IV to a per-unit scratch and re-walks the sglist per unit and is paid only by callers setting unit_size != 0. That gives two directions:

1. SW batching layer (current v6): mid-API transparently splits when the alg lacks CRYPTO_ALG_REQ_SEG and unit_size != 0. Works today on every existing skcipher at the ~50 ns/unit cost, and goes quiet as algs gain native support.

2. HW-offload-hint model (as in Inel acomp series): callers set unit_size only for CRYPTO_ALG_REQ_SEG algs and the mid-API never emulates non-native ones. Zero overhead, but the path is dead until an in-tree skcipher advertises native support, unlike acomp, where IAA already does.

(1) is usable now with a small permanent SW cost; (2) mirrors the acomp mid-layer dispatch but, absent a native skcipher, ships an interface with no in-tree user. Which would you prefer?

Thanks,
Leonid

maintainer
#6 lore raw

Re: [PATCH v5 2/5] crypto: dun - data-unit-number dispatch template

Quoted 9 lines · Leonid Ravich

I benchmarked the mid-API split vs the legacy per-sector loop on r7i.metal (VAES-AVX512): dm-crypt shows no measurable throughput or latency regression, but a microbench isolates a fixed ~50 ns per 512B unit. It's a fixed per-call cost: the split
copies the counter IV to a per-unit scratch and re-walks the sglist per unit and is paid only by callers setting unit_size != 0. That gives two directions:

Quoted 4 lines

1. SW batching layer (current v6): mid-API transparently splits when the alg lacks CRYPTO_ALG_REQ_SEG and unit_size != 0. Works today on every existing skcipher at the ~50 ns/unit cost, and goes quiet as algs gain native support.

I think we should be able to eliminate this overhead. Could you send me the patch so I can take a look?

But going back to the dun template, I think we should still go with no templates for plain64. But the other ones should use a template instead of using the cc->iv_gen_ops->generator.

IOW if unit-splitting is in use, dm-crypt should always pass the sector number in little-endian format in as the IV. Then depending on which IV generator is configured (which should be done through templates, just like essiv), they can generate the final IV.

With this scheme, there should be no overall increase in indirect calls in any case (and with one less for plain64).

This still preserves the ability for optimised implementations to eliminate indirect calls for the IV generator by handling it directly rather than through the generic template.

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
#7 lore raw

PATCH v5 3/5 crypto: testmgr - test dun() dispatch

In reply to Leonid Ravich

For every ivsize-16 skcipher, wrap it in both a dun(<inner>,le) and a dun(<inner>,be) instance and cross-check each batched output against an independent N x single-DU reference run directly on the inner tfm (both keyed with one random key, the reference counter walked in the matching endianness), over a deliberately fragmented scatterlist whose entries do not align to the data-unit size. The two must produce byte-identical ciphertext; the batched ciphertext is then round-tripped and the caller IV checked unchanged. Testing both endiannesses exercises the be path independently of any in-tree consumer. Algorithms with no dun wrapper (ivsize != 16) are skipped; a genuine mismatch returns -EBADMSG.

---
 crypto/testmgr.c | 289 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 289 insertions(+)
crypto/testmgr.c +289 -0
index 4d86efae65b2..cd9246f432de 100644
@@ -3211,6 +3211,291 @@ static int test_skcipher(int enc, const struct cipher_test_suite *suite,
 	return 0;
 }
 
+/* Upper bound on the IVs the dun() template accepts (16: xts; 32: Adiantum). */
+#define TEST_MDU_MAX_IVSIZE	32
+
+/*
+ * Increment an @ivsize-byte IV as a wide counter.  Byte-wise with carry --
+ * deliberately independent of crypto/dun.c's per-limb walk, so the two only
+ * agree if the carry is right.  LE: byte 0 least significant; BE: last byte.
+ */
+static void test_mdu_iv_inc(u8 *iv, unsigned int ivsize, bool big_endian)
+{
+	int i;
+
+	if (big_endian) {
+		for (i = ivsize - 1; i >= 0; i--)
+			if (++iv[i])
+				break;
+	} else {
+		for (i = 0; i < (int)ivsize; i++)
+			if (++iv[i])
+				break;
+	}
+}
+
+/*
+ * Seed @iv so the low 64-bit limb is all-ones but its least-significant byte:
+ * the 2nd increment wraps the limb and carries into the next.  LE limb is
+ * bytes [0,8); BE limb is the last 8 bytes.  Bytes outside keep their value.
+ */
+static void test_mdu_iv_boundary(u8 *iv, unsigned int ivsize, bool big_endian)
+{
+	unsigned int i;
+
+	if (big_endian) {
+		for (i = ivsize - 8; i < ivsize; i++)
+			iv[i] = 0xff;
+		iv[ivsize - 1] = 0xfe;
+	} else {
+		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 batched dispatch on @mdu (a dun(<inner>,<endian>) tfm) is byte-equal
+ * to an independent N x single-DU reference on @inner with @big_endian-walked
+ * IVs, over a fragmented SG, then round-trips.  Both tfms must share a key.
+ * @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 *mdu,
+				      struct crypto_skcipher *inner,
+				      unsigned int du_size, bool big_endian,
+				      const u8 *iv_orig)
+{
+	const char *driver = crypto_skcipher_driver_name(mdu);
+	const unsigned int total = du_size * TEST_MDU_NR_UNITS;
+	const unsigned int ivsize = crypto_skcipher_ivsize(mdu);
+	struct skcipher_request *req = NULL;
+	struct scatterlist sg[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;
+	unsigned int u;
+	int err;
+
+	plain = kmalloc(total, GFP_KERNEL);
+	buf = kmalloc(total, GFP_KERNEL);
+	ref = kmalloc(total, GFP_KERNEL);
+	req = skcipher_request_alloc(mdu, GFP_KERNEL);
+	if (!plain || !buf || !ref || !req) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	get_random_bytes(plain, total);
+
+	/* Reference: per-DU single requests on the inner tfm, counter-walked IVs. */
+	memcpy(iv_ref, iv_orig, ivsize);
+	for (u = 0; u < TEST_MDU_NR_UNITS; u++) {
+		err = test_mdu_ref_encrypt(inner, 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, ivsize, big_endian);
+	}
+
+	/* 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, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				      CRYPTO_TFM_REQ_MAY_SLEEP,
+				      crypto_req_done, &wait);
+	skcipher_request_set_crypt(req, sg, sg, total, iv_work);
+	skcipher_request_set_data_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;
+	}
+
+	/* Round-trip the batched ciphertext back to plaintext. */
+	test_mdu_sg_fragment(sg, TEST_MDU_NR_FRAGS, buf, total);
+	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, sg, total, iv_work);
+	skcipher_request_set_data_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(ref);
+	kfree(buf);
+	kfree(plain);
+	return err;
+}
+
+/*
+ * Cross-check the dun(<inner>,@endian) wrapper against @tfm over all du sizes.
+ * Returns 0 on success or skip (no wrapper / rejected key); -EBADMSG on a real
+ * mismatch.
+ */
+static int test_skcipher_multi_du_endian(struct crypto_skcipher *tfm,
+					 const char *alg_name,
+					 const char *endian, bool big_endian,
+					 const u8 *keybuf, unsigned int keylen)
+{
+	static const unsigned int du_sizes[] = { 512, 1024, 2048, 4096 };
+	char mdu_name[CRYPTO_MAX_ALG_NAME];
+	struct crypto_skcipher *mdu;
+	unsigned int ivsize;
+	u8 iv[TEST_MDU_MAX_IVSIZE];
+	unsigned int j;
+	int err;
+
+	if (snprintf(mdu_name, sizeof(mdu_name), "dun(%s,%s)", alg_name,
+		     endian) >= (int)sizeof(mdu_name))
+		return 0;
+
+	mdu = crypto_alloc_skcipher(mdu_name, 0, 0);
+	if (IS_ERR(mdu)) {
+		/* No dun wrapper (ivsize not a multiple of 8, or too wide): skip. */
+		if (PTR_ERR(mdu) == -ENOENT || PTR_ERR(mdu) == -EINVAL)
+			return 0;
+		return PTR_ERR(mdu);
+	}
+
+	ivsize = crypto_skcipher_ivsize(mdu);
+	if (ivsize > TEST_MDU_MAX_IVSIZE) {
+		err = 0;	/* wider than we have buffers for: skip */
+		goto out;
+	}
+
+	err = crypto_skcipher_setkey(mdu, keybuf, keylen);
+	if (err) {
+		err = 0;	/* weak/rejected key (e.g. XTS equal halves): skip */
+		goto out;
+	}
+
+	for (j = 0; j < ARRAY_SIZE(du_sizes); j++) {
+		/* A random starting IV. */
+		get_random_bytes(iv, ivsize);
+		err = test_skcipher_multi_du_one(mdu, tfm, du_sizes[j],
+						 big_endian, iv);
+		if (err)
+			break;
+		/* And one seeded to carry across a 64-bit limb / byte run. */
+		get_random_bytes(iv, ivsize);
+		test_mdu_iv_boundary(iv, ivsize, big_endian);
+		err = test_skcipher_multi_du_one(mdu, tfm, du_sizes[j],
+						 big_endian, iv);
+		if (err)
+			break;
+		cond_resched();
+	}
+out:
+	crypto_free_skcipher(mdu);
+	return err;
+}
+
+/*
+ * Cross-check dun() dispatch against a single-DU reference, in both le and be,
+ * for every ivsize the template accepts (16: xts; 32: Adiantum).
+ */
+static int test_skcipher_multi_du(struct crypto_skcipher *tfm)
+{
+	const char *alg_name = crypto_skcipher_alg(tfm)->base.cra_name;
+	u8 keybuf[128];
+	unsigned int keylen;
+	int err;
+
+	/* Key the inner tfm; each dun() wrapper is keyed identically below. */
+	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 */
+
+	err = test_skcipher_multi_du_endian(tfm, alg_name, "le", false,
+					    keybuf, keylen);
+	if (err)
+		return err;
+	return test_skcipher_multi_du_endian(tfm, alg_name, "be", true,
+					     keybuf, keylen);
+}
+
 static int alg_test_skcipher(const struct alg_test_desc *desc,
 			     const char *driver, u32 type, u32 mask)
 {
@@ -3259,6 +3544,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
#8 lore raw

PATCH v5 4/5 dm crypt: batch a bio segment's sectors via dun()

In reply to Leonid Ravich

Submit one skcipher request per contiguous bio segment (a single bio_vec) with data_unit_size = cc->sector_size, instead of one request per sector. E.g. the default 512-byte sector with a 4 KiB bio_vec becomes one request of 8 data units; the crypto layer (the dun() template, or a native driver) walks the per-sector IV as a data-unit counter. Because a bio_vec is one contiguous segment, the request uses only the existing inline dmreq->sg_in[0]/sg_out[0] entry -- no per-bio scatterlist allocation, and no regression on small random I/O.

crypt_alloc_tfms() wraps the skcipher in dun(<cipher>,<endian>) when crypt_can_batch_dun() holds: an IV mode that is a data-unit counter (its crypt_iv_operations sets dun_endian to the counter endianness -- "le" for plain64, "be" for plain64be; non-counter modes such as lmk/tcw/eboiv leave it NULL and are excluded), single-tfm, non-aead, and sector_size 512 or iv_large_sectors so the per-unit IV step is exactly one. This is the same kind of name rewrite as essiv(), done in the one alloc helper so callers are unchanged.

DM_CRYPT selects CRYPTO_DUN and dun() resolves against a sync inner cipher, so wrapping has no acceptable failure that the bare cipher would survive -- there is no fallback; any error propagates. (A config whose only xts provider is async with no generic CRYPTO_XTS would now fail to activate rather than silently run per-sector; generic xts is selected by the dependency chain, so this does not arise in practice.)

crypt_convert_block_skcipher() handles both cases in one function: the length is crypt_skcipher_len() -- a whole contiguous segment when batching, else a single sector -- and data_unit_size is set unconditionally (a dun() tfm reads it; a plain skcipher ignores it). It advances the bio iterators itself (as the aead path already does) and reports the bytes processed, so crypt_convert() advances cc_sector / tag_offset uniformly via one helper, no per-case duplication.

Verified byte-equivalent to the per-sector path: plain64 and plain64be dm-crypt with dun() produce ciphertext bit-identical to an unpatched kernel over a 256 MB device (xts-aes driving the split).

---
 drivers/md/Kconfig    |   1 +
 drivers/md/dm-crypt.c | 208 +++++++++++++++++++++++++++++++++---------
 2 files changed, 166 insertions(+), 43 deletions(-)
drivers/md/Kconfig +1 -0
index a3fcdca7e6db..e8e299566374 100644
@@ -299,6 +299,7 @@ config DM_CRYPT
 	select CRC32
 	select CRYPTO
 	select CRYPTO_CBC
+	select CRYPTO_DUN # multi-data-unit batching of contiguous sectors
 	select CRYPTO_ESSIV
 	select CRYPTO_LIB_AES
 	select CRYPTO_LIB_MD5 # needed by lmk IV mode
drivers/md/dm-crypt.c +165 -43

361 lines not on this page · raw message

Signature · 1 line
2.47.3
#9 lore raw

Re: [PATCH v5 4/5] dm crypt: batch a bio segment's sectors via dun()

kernel test robot noticed "RIP:alg_test.cold" on:

Hello,

kernel test robot noticed "RIP:alg_test.cold" on:

commit: 89bd605959ed9145ac2906d1cc4357b71eee9d3b ("[PATCH v5 4/5] dm crypt: batch a bio segment's sectors via dun()")
url: https://github.com/intel-lab-lkp/linux/commits/Leonid-Ravich/crypto-skcipher-add-per-request-data_unit_size/20260701-185738
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v5 4/5] dm crypt: batch a bio segment's sectors via dun()

in testcase: kunit
version:
with following parameters:

	group: group-00

config: x86_64-rhel-9.4-kunit
compiler: gcc-14
test machine: 10 threads 1 sockets Intel(R) Core Ultra Series 3 @ 3.20GHz (Panther Lake) with 16G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

kern  :warn  : [  136.442332] [   T2675] ------------[ cut here ]------------
kern  :warn  : [  136.442345] [   T2675] alg: self-tests for cbc(aes) using cbc-aes-aesni failed (rc=-110)
kern  :warn  : [  136.442348] [   T2675] WARNING: crypto/testmgr.c:6087 at alg_test.cold+0x9c/0xb1, CPU#5: cryptomgr_test/2675
kern  :warn  : [  136.442364] [   T2675] Modules linked in: dun
kern  :warn  : [  136.442373] [   T2675] CPU: 5 UID: 0 PID: 2675 Comm: cryptomgr_test Not tainted 7.1.0-rc3+ #1 PREEMPT(lazy)
kern  :warn  : [  136.442382] [   T2675] Hardware name: Intel Corporation Panther Lake Client Platform/PTL-UH LP5 T3 RVP1, BIOS PTLPFWI1.R00.3332.D01.2508190102 08/19/2025
kern  :warn  : [  136.442389] [   T2675] RIP: 0010:alg_test.cold (testmgr.c:6087 (discriminator 1))
kern  :warn  : [  136.442395] [   T2675] Code: 89 de 48 c7 c7 e0 44 93 84 e8 9b c2 fb ff 41 83 fc fe 0f 84 b1 e8 43 01 48 8d 3d 7a 80 85 05 48 8b 54 24 08 44 89 e1 48 89 de <67> 48 0f b9 3a e9 95 e8 43 01 48 8b 14 24 b8 ff ff 37 00 48 c1 e0
All code
========
   0:	89 de                	mov    %ebx,%esi
   2:	48 c7 c7 e0 44 93 84 	mov    $0xffffffff849344e0,%rdi
   9:	e8 9b c2 fb ff       	call   0xfffffffffffbc2a9
   e:	41 83 fc fe          	cmp    $0xfffffffe,%r12d
  12:	0f 84 b1 e8 43 01    	je     0x143e8c9
  18:	48 8d 3d 7a 80 85 05 	lea    0x585807a(%rip),%rdi        # 0x5858099
  1f:	48 8b 54 24 08       	mov    0x8(%rsp),%rdx
  24:	44 89 e1             	mov    %r12d,%ecx
  27:	48 89 de             	mov    %rbx,%rsi
  2a:*	67 48 0f b9 3a       	ud1    (%edx),%rdi		<-- trapping instruction
  2f:	e9 95 e8 43 01       	jmp    0x143e8c9
  34:	48 8b 14 24          	mov    (%rsp),%rdx
  38:	b8 ff ff 37 00       	mov    $0x37ffff,%eax
  3d:	48                   	rex.W
  3e:	c1                   	.byte 0xc1
  3f:	e0                   	.byte 0xe0

Code starting with the faulting instruction
===========================================

   0:	67 48 0f b9 3a       	ud1    (%edx),%rdi
   5:	e9 95 e8 43 01       	jmp    0x143e89f
   a:	48 8b 14 24          	mov    (%rsp),%rdx
   e:	b8 ff ff 37 00       	mov    $0x37ffff,%eax
  13:	48                   	rex.W
  14:	c1                   	.byte 0xc1
  15:	e0                   	.byte 0xe0
kern  :warn  : [  136.442404] [   T2675] RSP: 0018:ffffc9000219fce0 EFLAGS: 00010293
kern  :warn  : [  136.442411] [   T2675] RAX: 0000000000000041 RBX: ffff888474e11480 RCX: 00000000ffffff92
kern  :warn  : [  136.442416] [   T2675] RDX: ffff888474e11400 RSI: ffff888474e11480 RDI: ffffffff86abc5a0
kern  :warn  : [  136.442421] [   T2675] RBP: 1ffff92000433fa5 R08: 0000000000000000 R09: fffffbfff0b2b848
kern  :warn  : [  136.442426] [   T2675] R10: 0000000000000003 R11: fefefefefefefeff R12: 00000000ffffff92
kern  :warn  : [  136.442431] [   T2675] R13: dffffc0000000000 R14: 0000000000000069 R15: 0000000000000027
kern  :warn  : [  136.442436] [   T2675] FS:  0000000000000000(0000) GS:ffff88848572b000(0000) knlGS:0000000000000000
kern  :warn  : [  136.442442] [   T2675] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
kern  :warn  : [  136.442446] [   T2675] CR2: 00007fc9e48120ac CR3: 000000047c074001 CR4: 0000000108770ef0
kern  :warn  : [  136.442451] [   T2675] PKRU: 55555554
kern  :warn  : [  136.442455] [   T2675] Call Trace:
kern  :warn  : [  136.442458] [   T2675]  <TASK>
kern  :warn  : [  136.442464] [   T2675]  ? __pfx_alg_test (linux/crypto.h:449)
kern  :warn  : [  136.442470] [   T2675]  ? kasan_save_track (kasan/common.c:78)
kern  :warn  : [  136.442477] [   T2675]  ? _raw_spin_lock_irqsave (linux/instrumented.h:55 linux/atomic/atomic-instrumented.h:1301 asm-generic/qspinlock.h:111 linux/spinlock.h:187 linux/spinlock_api_smp.h:133 locking/spinlock.c:166)
kern  :warn  : [  136.442483] [   T2675]  ? __pfx__raw_spin_lock_irqsave (locking/spinlock.c:273)
kern  :warn  : [  136.442489] [   T2675]  ? __pfx_cryptomgr_test (usercopy_64.c:?)
kern  :warn  : [  136.442496] [   T2675]  ? kthread_affine_node (kthread.c:377)
kern  :warn  : [  136.442501] [   T2675]  ? __kthread_parkme (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 kthread.c:272)
kern  :warn  : [  136.442508] [   T2675]  ? __pfx_cryptomgr_test (usercopy_64.c:?)
kern  :warn  : [  136.442513] [   T2675]  cryptomgr_test (algboss.c:179)
kern  :warn  : [  136.442519] [   T2675]  kthread (kthread.c:436)
kern  :warn  : [  136.442523] [   T2675]  ? recalc_sigpending (linux/instrumented.h:97 asm-generic/bitops/instrumented-atomic.h:41 linux/thread_info.h:109 signal.c:181)
kern  :warn  : [  136.442530] [   T2675]  ? __pfx_kthread (linux/list.h:381 (discriminator 2))
kern  :warn  : [  136.442534] [   T2675]  ret_from_fork (x86/kernel/process.c:158)
kern  :warn  : [  136.442541] [   T2675]  ? __pfx_ret_from_fork (x86/include/asm/entry-common.h:54)
kern  :warn  : [  136.442547] [   T2675]  ? switch_fpu (linux/instrumented.h:82 asm-generic/bitops/instrumented-non-atomic.h:141 linux/thread_info.h:133 linux/sched.h:2069 x86/include/asm/fpu/sched.h:34)
kern  :warn  : [  136.442553] [   T2675]  ? __switch_to (x86/kernel/process_64.c:403 x86/kernel/process_64.c:663)
kern  :warn  : [  136.442558] [   T2675]  ? __pfx_kthread (linux/list.h:381 (discriminator 2))
kern  :warn  : [  136.442563] [   T2675]  ret_from_fork_asm (x86/entry/entry_64.S:245)
kern  :warn  : [  136.442570] [   T2675]  </TASK>
kern  :warn  : [  136.442573] [   T2675] ---[ end trace 0000000000000000 ]---
kern  :err   : [  136.442580] [   T2675] alg: Unexpected test result for cbc-aes-aesni: -110

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260716/[email protected]

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#10 lore raw

Re: [PATCH v5 4/5] dm crypt: batch a bio segment's sectors via dun()

In reply to Leonid Ravich

drivers/md/dm-crypt.c static int crypt_convert_block_skcipher(struct crypt_config *cc, · line 1457

Quoted 9 lines · Leonid Ravich
drivers/md/dm-crypt.c +3 -1
@@ -1410,7 +1452,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);
+	/* A dun() tfm reads this; a plain skcipher ignores it (len is one sector). */
+	skcipher_request_set_data_unit_size(req, sector_size);

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?

Cheers,

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
#11 lore raw

PATCH v5 5/5 blk-crypto: fallback - batch a segment's data units via dun()

In reply to Leonid Ravich

blk-crypto-fallback open-codes a per-data-unit loop, issuing one skcipher request per data unit with the IV walked as a DUN counter. Allocate dun(<cipher>,le) instead of the bare cipher so a contiguous bio segment is encrypted/decrypted as one multi-data-unit request, the crypto layer walking the per-unit IV. Every blk-crypto mode feeds the DUN as a little-endian counter, and dun() handles any counter width up to 32 bytes, so all modes -- including Adiantum (32-byte IV) -- are wrapped and the open-coded inner per-unit loop is removed from both the encrypt and decrypt paths. This makes blk-crypto-fallback a second consumer of the template (after dm-crypt) and lets a higher-priority hardware dun(...) driver, if present, handle the request in one pass.

Output is unchanged: the template's little-endian per-unit counter is exactly blk_crypto_dun_to_iv()/bio_crypt_dun_increment().

---
 block/Kconfig               |  1 +
 block/blk-crypto-fallback.c | 74 ++++++++++++++++++-------------------
 2 files changed, 36 insertions(+), 39 deletions(-)
block/Kconfig +1 -0
index 15027963472d..0c9025f9b0f6 100644
@@ -204,6 +204,7 @@ config BLK_INLINE_ENCRYPTION_FALLBACK
 	depends on BLK_INLINE_ENCRYPTION
 	select CRYPTO
 	select CRYPTO_SKCIPHER
+	select CRYPTO_DUN # batches a segment's data units per crypto request
 	help
 	  Enabling this lets the block layer handle inline encryption
 	  by falling back to the kernel crypto API when inline
block/blk-crypto-fallback.c +35 -39

134 lines not on this page · raw message

Signature · 1 line
2.47.3
#12 lore raw
In reply to Leonid Ravich
Quoted 7 lines · Leonid Ravich

This is v5. It reworks the multi-data-unit support from the in-core auto-splitter of v4 into a crypto template, dun(...), addressing the v4
review: there is now no added cost on the core skcipher path, no per-algorithm capability flag, and the per-data-unit split lives in an algorithm rather than in crypto_skcipher_encrypt/decrypt the shape Herbert suggested, which removes the "overhead for everyone" Eric objected to.

No, this didn't address my feedback. It moved things around but still adds additional overhead for everyone to support an out-of-tree driver, which also hasn't been shown to be any better than just using the CPU.

- Eric

#13 lore raw
Quoted 3 lines · Eric Biggers

No, this didn't address my feedback. It moved things around but still adds additional overhead for everyone to support an out-of-tree driver, which also hasn't been shown to be any better than just using the CPU.

Eric, thanks for the fast reply.

Overhead: for a non-user the only cost is the data_unit_size field plus one zeroing store in set_tfm()/ON_STACK; the en/decrypt paths are untouched. A dun() user pays one indirect dispatch into the template per request plus a scatterwalk step and IV copy per unit -- the same per-DU bookkeeping the consumer already open-codes today.

On the driver: I agree pushing code optimized for an out-of-tree driver is wrong, but I don't think that's the case here -- this helps any async crypto engine, and there are in-tree async xts(aes) ones dm-crypt is eligible to use today: HiSilicon SEC2, TI DTHEv2, Atmel (I don't have any to test on). To bound the win, I used cryptd as a pure async carrier and moved the per-DU split inside it, then ran dm-crypt + fio: batching cut CPU ~30% on 128k I/O (large batch) and had zero impact on 4k -- so the saving is dispatch, not crypto. A real engine that submits a whole multi-DU request in one descriptor avoids that per-DU dispatch entirely, so it saves at least that.

So the question for me is what the bar is: does landing the API and dun() template now (with the in-tree consolidation it already buys dm-crypt and blk-crypto-fallback), with a throughput demonstration deferred to a real async provider, work for you ?

Thanks,
Leonid

#14 lore raw
Quoted 10 lines · Leonid Ravich
Quoted 3 lines · Eric Biggers

No, this didn't address my feedback. It moved things around but still adds additional overhead for everyone to support an out-of-tree driver, which also hasn't been shown to be any better than just using the CPU.

Eric, thanks for the fast reply.

Overhead: for a non-user the only cost is the data_unit_size field plus one zeroing store in set_tfm()/ON_STACK; the en/decrypt paths are untouched.

Sure, which is still a cost for everyone.

Quoted 3 lines

A dun() user pays one indirect dispatch into the template per request plus a scatterwalk step and IV copy per unit -- the same per-DU bookkeeping the consumer already open-codes today.

It's not the same at all. There's now an extra indirect call, more per-request memory used, additional overhead to create a scatterlist and then break it up into multiple ones using the fully generalized scatterlist walker instead of just creating the correct ones in the first place from the bio_vec, etc. We need to be simplifying the crypto APIs, not making them even more complex and adding more overhead.

Quoted 2 lines

On the driver: I agree pushing code optimized for an out-of-tree driver is wrong

So don't do it.

Quoted 4 lines

but I don't think that's the case here -- this helps any async crypto engine, and there are in-tree async xts(aes) ones dm-crypt is eligible to use today: HiSilicon SEC2, TI DTHEv2, Atmel (I don't have any to test on).

It helps nothing, as there is no patch and no benchmarks.

Quoted 8 lines

To bound the win, I used cryptd as a pure async carrier and moved the per-DU split inside it, then ran dm-crypt + fio: batching cut CPU ~30% on 128k I/O (large batch) and had zero impact on 4k -- so the saving is dispatch, not crypto.

A real engine that submits a whole
multi-DU request in one descriptor avoids that per-DU dispatch entirely, so it saves at least that.

That is not an equivalent benchmark at all.

Quoted 4 lines

So the question for me is what the bar is: does landing the API and dun() template now (with the in-tree consolidation it already buys dm-crypt and blk-crypto-fallback), with a throughput demonstration deferred to a real async provider, work for you ?

I definitely don't want this for blk-crypto-fallback. blk-crypto-fallback already only supports CPU-based crypto acceleration. (As it should, because nothing else is actually worthwhile these days, other than hardware inline encryption which is separately supported.) And I'm also planning to switch it from crypto_skcipher to lib/crypto/ to eliminate the remaining API overhead, which will actually accomplish that, unlike the dun() thing which just makes it worse.

The bar for adding things to the upstream kernel is that it has to actually be used *and* beneficial in the upstream kernel.

- Eric