Re: [dm-devel] dm-integrity

[Milan Broz]

On 07/03/2017 06:44 AM, Renesanso wrote:
> Hi for all.
> 
> Dmitry Kasatkin's fork of linux.git write dm-integrity patch for linux 
...

yes, unfortunately we named the target the same (and I realized it too late).

It is doing something similar but definitely it is not the same.

> I try to use dmsetup to setup  dm-integrity in ecc mode (but if change 
> block on backend device dm-integrity gives not reaction and give another 
> md5sum to upper level. but non error), for dm-crypt I cannot understand 
> how to use AEAD mode.

You probably configured it in mode when it only provide tag space,
but does not calculate and verify internal hash.

(ECC means error correction, this target do not provide error correction,
only detection of error (such a tool could be written on top of dm-integrity 
though).

> Please, give full instrustion to use dm-integrity in ecc mode and with 
> dm-crypt  (with kernel keychain creation)..

dm-integrity can work in standalone mode or together with dm-crypt.

For the standalone mode, it is the best to use integritysetup tool
(for now in master branch of cryptsetup project).
https://gitlab.com/cryptsetup/cryptsetup

There is some simple documentation in man page and on this page
https://gitlab.com/cryptsetup/cryptsetup/wikis/DMIntegrity

(You can setup HMAC integrity protection in standalone mode as well.)
I will update it soon with some more info and prepare some better examples
(the whole userspace is still not finished though but should work.)

For the combination with dm-crypt and AEAD - this is part of LUKS2 branch
in the same repository but it is really only for experiments.
Once we will have some testing build, I'll write more here, sorry, it takes
longer than I expected.

Milan

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v3 01/28] crypto: change backlog return code to -EIOCBQUEUED

[Gilad Ben-Yossef]

On Mon, Jul 3, 2017 at 3:35 PM, Herbert Xu  wrote:
> On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
>> The crypto API was using the -EBUSY return value to indicate
>> both a hard failure to submit a crypto operation into a
>> transformation provider when the latter was busy and the backlog
>> mechanism was not enabled as well as a notification that the operation
>> was queued into the backlog when the backlog mechanism was enabled.
>>
>> Having the same return code indicate two very different conditions
>> depending on a flag is both error prone and requires extra runtime
>> check like the following to discern between the cases:
>>
>>   if (err == -EINPROGRESS ||
>>   (err == -EBUSY && (ahash_request_flags(req) &
>>  CRYPTO_TFM_REQ_MAY_BACKLOG)))
>>
>> This patch changes the return code used to indicate a crypto op
>> was queued in the backlog to -EIOCBQUEUED, thus resolving both
>> issues.
>>
>> Signed-off-by: Gilad Ben-Yossef 
>
> So you're changing the success case from EBUSY to EIOCBQUEUED.
> This results in a lot of churn as you have to change every single
> driver and caller.
>
> How about changing the error case to use something other than
> EBUSY instead?

That was indeed my first thought - I wanted to change EBUSY to EAGAIN.
It might even be a more descriptive error message since the failure is
transient.

What stopped me was that I saw the algif interface passes this EBUSY value
to user space. I don't know of any software that depends on this value but
I'm really averse to changing user space API.

Of course, we can just plant a (ret != AGAIN : ? EBUSY) in the algif interface
return to user space code path but that felt like a kludge to me.

And it doesn't really save any churn, I think - I'd still have to
visit all the places that
use the flags value to distinguish between the two EBUSY use cases and  I need
to visit most places that check for EBUSY as backlog indication because I'm
replacing the check with the generic replacement, so it doesn't look
like in the end
it will be a smaller change.

Having said that, if you prefer me to replace the error case I'd
happily do that.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v3 01/28] crypto: change backlog return code to -EIOCBQUEUED

[Herbert Xu]

On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
> The crypto API was using the -EBUSY return value to indicate
> both a hard failure to submit a crypto operation into a
> transformation provider when the latter was busy and the backlog
> mechanism was not enabled as well as a notification that the operation
> was queued into the backlog when the backlog mechanism was enabled.
> 
> Having the same return code indicate two very different conditions
> depending on a flag is both error prone and requires extra runtime
> check like the following to discern between the cases:
> 
>   if (err == -EINPROGRESS ||
>   (err == -EBUSY && (ahash_request_flags(req) &
>  CRYPTO_TFM_REQ_MAY_BACKLOG)))
> 
> This patch changes the return code used to indicate a crypto op
> was queued in the backlog to -EIOCBQUEUED, thus resolving both
> issues.
> 
> Signed-off-by: Gilad Ben-Yossef 

So you're changing the success case from EBUSY to EIOCBQUEUED.
This results in a lot of churn as you have to change every single
driver and caller.

How about changing the error case to use something other than
EBUSY instead?

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] block: Convert hd_struct in_flight from atomic to percpu

[Ming Lei]

Hi Bian,

On Sat, Jul 1, 2017 at 2:33 AM, Brian King  wrote:
> On 06/30/2017 09:08 AM, Jens Axboe wrote:
> Compared with the totally percpu approach, this way might help 1:M or
> N:M mapping, but won't help 1:1 map(NVMe), when hctx is mapped to
> each CPU(especially there are huge hw queues on a big system), :-(

 Not disagreeing with that, without having some mechanism to only
 loop queues that have pending requests. That would be similar to the
 ctx_map for sw to hw queues. But I don't think that would be worthwhile
 doing, I like your pnode approach better. However, I'm still not fully
 convinced that one per node is enough to get the scalability we need.

 Would be great if Brian could re-test with your updated patch, so we
 know how it works for him at least.
>>>
>>> I'll try running with both approaches today and see how they compare.
>>
>> Focus on Ming's, a variant of that is the most likely path forward,
>> imho. It'd be great to do a quick run on mine as well, just to establish
>> how it compares to mainline, though.
>
> On my initial runs, the one from you Jens, appears to perform a bit better, 
> although
> both are a huge improvement from what I was seeing before.
>
> I ran 4k random reads using fio to nullblk in two configurations on my 20 core
> system with 4 NUMA nodes and 4-way SMT, so 80 logical CPUs. I ran both 80 
> threads
> to a single null_blk as well as 80 threads to 80 null_block devices, so one 
> thread

Could you share what the '80 null_block devices' is?  It means you
create 80 null_blk
devices? Or you create one null_blk and make its hw queue number as 80
via module
parameter of ''submit_queues"?

I guess we should focus on multi-queue case since it is the normal way of NVMe.

> per null_blk. This is what I saw on this machine:
>
> Using the Per node atomic change from Ming Lei
> 1 null_blk, 80 threads
> iops=9376.5K
>
> 80 null_blk, 1 thread
> iops=9523.5K
>
>
> Using the alternate patch from Jens using the tags
> 1 null_blk, 80 threads
> iops=9725.8K
>
> 80 null_blk, 1 thread
> iops=9569.4K

If 1 thread means single fio job, looks the number is too too high, that means
one random IO can complete in about 0.1us(100ns) on single CPU, not sure if it
is possible, :-)


Thanks,
Ming Lei

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v3 28/28] crypto: adapt api sample to use async. op wait

[Gilad Ben-Yossef]

The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef 
---
 Documentation/crypto/api-samples.rst | 52 +++-
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst 
b/Documentation/crypto/api-samples.rst
index c6aa4ba..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-struct tcrypt_result {
-struct completion completion;
-int err;
-};
-
 /* tie all data structures together */
 struct skcipher_def {
 struct scatterlist sg;
 struct crypto_skcipher *tfm;
 struct skcipher_request *req;
-struct tcrypt_result result;
+struct crypto_wait wait;
 };
 
-/* Callback function */
-static void test_skcipher_cb(struct crypto_async_request *req, int error)
-{
-struct tcrypt_result *result = req->data;
-
-if (error == -EINPROGRESS)
-return;
-result->err = error;
-complete(>completion);
-pr_info("Encryption finished successfully\n");
-}
-
 /* Perform cipher operation */
 static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
  int enc)
 {
-int rc = 0;
+int rc;
 
 if (enc)
-rc = crypto_skcipher_encrypt(sk->req);
+rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), >wait);
 else
-rc = crypto_skcipher_decrypt(sk->req);
-
-switch (rc) {
-case 0:
-break;
-case -EINPROGRESS:
-case -EIOCBQUEUED:
-rc = wait_for_completion_interruptible(
->result.completion);
-if (!rc && !sk->result.err) {
-reinit_completion(>result.completion);
-break;
-}
-default:
-pr_info("skcipher encrypt returned with %d result %d\n",
-rc, sk->result.err);
-break;
-}
-init_completion(>result.completion);
+rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), >wait);
+
+   if (rc)
+   pr_info("skcipher encrypt returned with result %d\n", rc);
 
 return rc;
 }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
 }
 
 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  test_skcipher_cb,
-  );
+  crypto_req_done,
+  );
 
 /* AES 256 with random key */
 get_random_bytes(, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
 /* We encrypt one block */
 sg_init_one(, scratchpad, 16);
 skcipher_request_set_crypt(req, , , 16, ivdata);
-init_completion();
+crypto_init_wait();
 
 /* encrypt data */
 ret = test_skcipher_encdec(, 1);
-- 
2.1.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v3 13/28] crypto: adapt api sample to -EIOCBQUEUED as backlog indication

[Gilad Ben-Yossef]

Replace -EBUSY with -EIOCBQUEUED for backlog queueing indication
as part of new API.

Signed-off-by: Gilad Ben-Yossef 
---

This patch should be squashed with the first patch in the series
when applied.

 Documentation/crypto/api-samples.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/crypto/api-samples.rst 
b/Documentation/crypto/api-samples.rst
index 2531948..c6aa4ba 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -47,7 +47,7 @@ Code Example For Symmetric Key Cipher Operation
 case 0:
 break;
 case -EINPROGRESS:
-case -EBUSY:
+case -EIOCBQUEUED:
 rc = wait_for_completion_interruptible(
 >result.completion);
 if (!rc && !sk->result.err) {
-- 
2.1.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH] dm-zoned: Fix overflow when converting zone ID to sectors

[Damien Le Moal]

A zone ID is a 32 bits unsigned int which can overflow when doing the
bit shifts calculations in dmz_start_sect(). With a 256 MB zone size
drive, the overflow happens for a zone ID >= 8192.
Fix this by casting the zone ID to a sector_t before doing the bit
shift. While at it, similarly fix dmz_start_block().

Signed-off-by: Damien Le Moal 
---
 drivers/md/dm-zoned-metadata.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
index 4618441c..884ff7c 100644
--- a/drivers/md/dm-zoned-metadata.c
+++ b/drivers/md/dm-zoned-metadata.c
@@ -191,12 +191,12 @@ unsigned int dmz_id(struct dmz_metadata *zmd, struct 
dm_zone *zone)
 
 sector_t dmz_start_sect(struct dmz_metadata *zmd, struct dm_zone *zone)
 {
-   return dmz_id(zmd, zone) << zmd->dev->zone_nr_sectors_shift;
+   return (sector_t)dmz_id(zmd, zone) << zmd->dev->zone_nr_sectors_shift;
 }
 
 sector_t dmz_start_block(struct dmz_metadata *zmd, struct dm_zone *zone)
 {
-   return dmz_id(zmd, zone) << zmd->dev->zone_nr_blocks_shift;
+   return (sector_t)dmz_id(zmd, zone) << zmd->dev->zone_nr_blocks_shift;
 }
 
 unsigned int dmz_nr_chunks(struct dmz_metadata *zmd)
-- 
2.9.4

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel