Re: [dm-crypt] Encrypting with larger packet size (+some experimental patch)

[Milan Broz <gmazyland@gmail.com>]

On 01/24/2013 08:44 PM, Dinesh Garg wrote:

> >>Please can you be more specific
> I was offering my help in term of code change and testing and upstreaming the patch.

I am more than friendly to add any dmcrypt extension but you can imagine what happens
if reason is

"add larger sector size because some closed source crypto accelerator performs better
with large sector size and as a bonus make device incompatible for others" ...

I need some real reason before submitting such patch.

That said... here is the patch which can use larger block size option. It is not complete
(mainly IV generators need changes and I am afraid there should be morech changes than
basic io hints) but it should work for basic performance testing
if you want to play with it (you have to use dmsetup).

So show your crypto performance numbers now :-)

Milan

-
dm-crypt: optionally support encryption block (sector) size (EXPERIMENTAL)

Add "block_size" optional parameter which specifies encryption
sector size (atomic unit of block device encryption).

Parameter can be in range 512 - 65536 bytes and must be power of two.

NOTE: this device cannot be handled with cryptsetup directly
if this parameter is set.

FIXME: missing fixes for IV generators which have 512 bytes sector hardcoded.
IOW IV is calculated always from 512 bytes offset (which is wrong but
patch still can be used for experiments).

Test script using dmsetup:

  DEV="/dev/sdb"
  DEV_SIZE=$(blockdev --getsz $DEV)
  KEY="9c1185a5c5e9fc54612808977ee8f548b2258d31ddadef707ba62c166051b9e3"
  BLOCK_SIZE=4096

  dmsetup create test_crypt --table "0 $DEV_SIZE crypt aes-xts-plain64 $KEY 0 $DEV 0 2 block_size $BLOCK_SIZE"
  #dmsetup table --showkeys test_crypt

Signed-off-by: Milan Broz <gmazyland@gmail.com>

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index f7369f9..0f4ecf2 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -143,6 +143,7 @@ struct crypt_config {
 	sector_t iv_offset;
 	unsigned int iv_size;
 
+	unsigned int block_size;
 	/*
 	 * Duplicated per cpu state. Access through
 	 * per_cpu_ptr() only.
@@ -693,20 +694,20 @@ static int crypt_convert_block(struct crypt_config *cc,
 	dmreq->iv_sector = ctx->cc_sector;
 	dmreq->ctx = ctx;
 	sg_init_table(&dmreq->sg_in, 1);
-	sg_set_page(&dmreq->sg_in, bv_in->bv_page, 1 << SECTOR_SHIFT,
+	sg_set_page(&dmreq->sg_in, bv_in->bv_page, cc->block_size,
 		    bv_in->bv_offset + ctx->offset_in);
 
 	sg_init_table(&dmreq->sg_out, 1);
-	sg_set_page(&dmreq->sg_out, bv_out->bv_page, 1 << SECTOR_SHIFT,
+	sg_set_page(&dmreq->sg_out, bv_out->bv_page, cc->block_size,
 		    bv_out->bv_offset + ctx->offset_out);
 
-	ctx->offset_in += 1 << SECTOR_SHIFT;
+	ctx->offset_in += cc->block_size;
 	if (ctx->offset_in >= bv_in->bv_len) {
 		ctx->offset_in = 0;
 		ctx->idx_in++;
 	}
 
-	ctx->offset_out += 1 << SECTOR_SHIFT;
+	ctx->offset_out += cc->block_size;
 	if (ctx->offset_out >= bv_out->bv_len) {
 		ctx->offset_out = 0;
 		ctx->idx_out++;
@@ -719,7 +720,7 @@ static int crypt_convert_block(struct crypt_config *cc,
 	}
 
 	ablkcipher_request_set_crypt(req, &dmreq->sg_in, &dmreq->sg_out,
-				     1 << SECTOR_SHIFT, iv);
+				     cc->block_size, iv);
 
 	if (bio_data_dir(ctx->bio_in) == WRITE)
 		r = crypto_ablkcipher_encrypt(req);
@@ -1563,7 +1564,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	char dummy;
 
 	static struct dm_arg _args[] = {
-		{0, 1, "Invalid number of feature args"},
+		{0, 3, "Invalid number of feature args"},
+		{512, 65536, "Invalid encryption sector size"},
 	};
 
 	if (argc < 5) {
@@ -1638,6 +1640,8 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	argv += 5;
 	argc -= 5;
 
+	cc->block_size = (1 << SECTOR_SHIFT);
+
 	/* Optional parameters */
 	if (argc) {
 		as.argc = argc;
@@ -1647,15 +1651,25 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		if (ret)
 			goto bad;
 
-		opt_string = dm_shift_arg(&as);
-
-		if (opt_params == 1 && opt_string &&
-		    !strcasecmp(opt_string, "allow_discards"))
-			ti->num_discard_requests = 1;
-		else if (opt_params) {
-			ret = -EINVAL;
-			ti->error = "Invalid feature arguments";
-			goto bad;
+		while (opt_params && (opt_string = dm_shift_arg(&as))) {
+			opt_params--;
+
+			if (!strcasecmp(opt_string, "allow_discards")) {
+				ti->num_discard_requests = 1;
+			} else if (opt_params && !strcasecmp(opt_string, "block_size")) {
+				opt_params--;
+				ret = dm_read_arg(_args + 1, &as, &cc->block_size, &ti->error);
+				/* value must be power of 2 */
+				if (ret || (1 << ilog2(cc->block_size) != cc->block_size)) {
+					ret = -EINVAL;
+					ti->error = "Invalid feature value for block_size";
+					goto bad;
+				}
+			} else {
+				ret = -EINVAL;
+				ti->error = "Invalid feature arguments";
+				goto bad;
+			}
 		}
 	}
 
@@ -1721,7 +1735,8 @@ static int crypt_status(struct dm_target *ti, status_type_t type,
 			unsigned status_flags, char *result, unsigned maxlen)
 {
 	struct crypt_config *cc = ti->private;
-	unsigned int sz = 0;
+	unsigned int sz = 0, opt_num = 0;
+	bool opt_discards = false, opt_sector = false;
 
 	switch (type) {
 	case STATUSTYPE_INFO:
@@ -1746,8 +1761,24 @@ static int crypt_status(struct dm_target *ti, status_type_t type,
 		DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset,
 				cc->dev->name, (unsigned long long)cc->start);
 
-		if (ti->num_discard_requests)
-			DMEMIT(" 1 allow_discards");
+		if (ti->num_discard_requests) {
+			opt_discards = true;
+			opt_num++;
+		}
+
+		if (cc->block_size != (1 << SECTOR_SHIFT)) {
+			opt_sector = true;
+			opt_num += 2;
+		}
+
+		if (opt_num)
+			DMEMIT(" %d", opt_num);
+
+		if (opt_discards)
+			DMEMIT(" allow_discards");
+
+		if (opt_sector)
+			DMEMIT(" block_size %d", cc->block_size);
 
 		break;
 	}
@@ -1843,9 +1874,21 @@ static int crypt_iterate_devices(struct dm_target *ti,
 	return fn(ti, cc->dev, cc->start, ti->len, data);
 }
 
+static void crypt_io_hints(struct dm_target *ti,
+			    struct queue_limits *limits)
+{
+	struct crypt_config *cc = ti->private;
+
+	if (cc->block_size != (1 << SECTOR_SHIFT)) {
+		limits->logical_block_size = cc->block_size;
+		limits->physical_block_size = cc->block_size;
+		blk_limits_io_min(limits, cc->block_size);
+	}
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 12, 0},
+	.version = {1, 13, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
@@ -1857,6 +1900,7 @@ static struct target_type crypt_target = {
 	.message = crypt_message,
 	.merge  = crypt_merge,
 	.iterate_devices = crypt_iterate_devices,
+	.io_hints = crypt_io_hints,
 };
 
 static int __init dm_crypt_init(void)