date:20050904

RE: [ACRYPTO] dm-crypt ported to acrypto.

2005-09-04 Thread Ronen Shitrit

Hi
 
Nice job, (horoshaya rabota)
Just two issue:
-I saw this patch is using a global variable which counts the number
pending requests: dm_async_pending.
I think that this variable should be allocated per dm_crypt session, i.e
for each crypt_config, since there can be
more then 2 consumers reading through the dm_crypt.
-I think that we might have a problem  if a write operation will be
processed in parallel to a read operation,
the read might wait for the write to complete, and the dm_async_pending
might also get wrong values???

Regards

Ronen Shitrit 
Marvell Semiconductor Israel Ltd

-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Evgeniy
Polyakov
Sent: Friday, September 02, 2005 9:09 AM
To: linux-crypto@vger.kernel.org
Subject: [ACRYPTO] dm-crypt ported to acrypto.

Hello, developers.

dm-crypt from device mapper package has been ported to acrypto.
Patch against 2.6.13 for dm-crypt attached. [1]

I did not run any performance test, since I have no hardware setup
currently which can show any diference between sync/async dm-crypt.

dm-crypt is the second storage management tool working with acrypto.
BD is the first and the fastest one, specially designed for asynchronous
operations, it can be fount at [2].

1. http://tservice.net.ru/~s0mbre/archive/acrypto/drivers
2. http://tservice.net.ru/~s0mbre/?section=projectsitem=bd

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -230,6 +230,173 @@ static struct crypt_iv_operations crypt_
.generator = crypt_iv_essiv_gen
 };
 
+#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
+
+static DECLARE_WAIT_QUEUE_HEAD(dm_async_queue);
+static int dm_async_pending;
+
+#include linux/acrypto.h
+#include linux/crypto_def.h
+
+static inline u16 crypto_tfm_get_type(struct crypto_tfm *tfm) {
+   u16 type;
+   char *name;
+
+   name = (char *)crypto_tfm_alg_name(tfm);
+
+   if (!strncmp(name, aes, 3)) {
+   switch (crypto_tfm_alg_blocksize(tfm)  3) {
+   case 128:
+   type = CRYPTO_TYPE_AES_128;
+   break;
+   case 192:
+   type = CRYPTO_TYPE_AES_192;
+   break;
+   case 256:
+   type = CRYPTO_TYPE_AES_256;
+   break;
+   default:
+   type = 0x;
+   break;
+   }
+   } else if (!strncmp(name, des3_ede, 3))
+   type = CRYPTO_TYPE_3DES;
+   else
+   type = 0x;
+
+   return type;
+}
+
+static inline u16 crypto_tfm_get_mode(struct crypto_tfm *tfm) {
+   u16 mode = tfm-crt_cipher.cit_mode  CRYPTO_TFM_MODE_MASK;
+
+   switch (mode) {
+   case CRYPTO_TFM_MODE_ECB:
+   mode = CRYPTO_MODE_ECB;
+   break;
+   case CRYPTO_TFM_MODE_CBC:
+   mode = CRYPTO_MODE_CBC;
+   break;
+   case CRYPTO_TFM_MODE_CFB:
+   mode = CRYPTO_MODE_CFB;
+   break;
+   case CRYPTO_TFM_MODE_CTR:
+   mode = CRYPTO_MODE_CTR;
+   break;
+   default:
+   mode = 0x;
+   break;
+   }
+
+   return mode;
+}
+
+static void dm_callback(struct crypto_session_initializer *ci, struct 
+crypto_data *data) {
+   dm_async_pending--;
+   
+   kfree((void *)(data-priv));
+   wake_up(dm_async_queue);
+}
+
+static inline int acrypto_process(struct crypt_config *cc, struct
scatterlist *out, 
+   struct scatterlist *in, unsigned int len, u8 *iv, int
iv_size, int 
+write) {
+   struct crypto_session *s;
+   struct crypto_session_initializer ci;
+   struct crypto_data data;
+   struct scatterlist *sg;
+   int sg_size, err = 0;
+   u8 *local_key;
+
+   memset(ci, 0, sizeof(ci));
+   memset(data, 0, sizeof(data));
+
+   ci.operation = (write)?CRYPTO_OP_ENCRYPT:CRYPTO_OP_DECRYPT;
+   ci.priority = 0;
+   ci.callback = dm_callback;
+   ci.type = crypto_tfm_get_type(cc-tfm);
+   ci.mode = crypto_tfm_get_mode(cc-tfm);
+   
+   if (ci.type == 0x || ci.mode == 0x) {
+   err = -EINVAL;
+   goto err_out_exit;
+   }
+
+   data.sg_src_num = data.sg_dst_num = data.sg_key_num = 1;
+   data.sg_iv_num = (iv)?1:0;
+
+   sg_size = sizeof(*sg) * (data.sg_src_num + data.sg_dst_num +
data.sg_iv_num + data.sg_key_num + iv_size + cc-key_size);
+   sg = kmalloc(sg_size, GFP_KERNEL);
+   if (!sg) {
+   err = -ENOMEM;
+   goto err_out_exit;
+   }
+   
+   memset(sg, 0, sg_size);
+#if 0
+   printk(%s:

Re: [ACRYPTO] dm-crypt ported to acrypto.

2005-09-04 Thread Evgeniy Polyakov

On Sun, Sep 04, 2005 at 03:51:57PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) 
wrote:
 Hi

Hello, Ronen.

 Nice job, (horoshaya rabota)

:) thank you.

 Just two issue:
 -I saw this patch is using a global variable which counts the number
 pending requests: dm_async_pending.
 I think that this variable should be allocated per dm_crypt session, i.e
 for each crypt_config, since there can be
 more then 2 consumers reading through the dm_crypt.
 -I think that we might have a problem  if a write operation will be
 processed in parallel to a read operation,
 the read might wait for the write to complete, and the dm_async_pending
 might also get wrong values???

dm-crypt process' it's requests in workqueue, so it can not be
interrupted with the same callback, but you rised another issue - 
dm-crypt uses multithreaded workqueue, so several requests could be 
run in parallel on different CPUs, but block layer may not allow
BIO reordering (until using BIO barriers, but dm core does not use it),
so dm-crypt is already synchronized, and kcryptd_do_work() was changed
to wait until all crypto requests are finished.

Simultaneous readig and writing is not possible - BIOs are pushed to the 
block driver under queue lock.

And to add yet another guard - dm itself process' it's remapping
subclasses (like dm-crypt) under read semaphore.

So dm itself in general and dm-crypt in particular are fully
synchronized.

Huge problem can be striken, if we have several devices under dm-crypt
control, in this case you are absolutely right, and pending counter will
be broken. Solution is to put both dm_async_queue and dm_pending counter 
into crypt_config structure.

 Regards
 
 Ronen Shitrit 
 Marvell Semiconductor Israel Ltd
 
 -Original Message-
 From: [EMAIL PROTECTED]
 [mailto:[EMAIL PROTECTED] On Behalf Of Evgeniy
 Polyakov
 Sent: Friday, September 02, 2005 9:09 AM
 To: linux-crypto@vger.kernel.org
 Subject: [ACRYPTO] dm-crypt ported to acrypto.
 
 Hello, developers.
 
 dm-crypt from device mapper package has been ported to acrypto.
 Patch against 2.6.13 for dm-crypt attached. [1]
 
 I did not run any performance test, since I have no hardware setup
 currently which can show any diference between sync/async dm-crypt.
 
 dm-crypt is the second storage management tool working with acrypto.
 BD is the first and the fastest one, specially designed for asynchronous
 operations, it can be fount at [2].
 
 1. http://tservice.net.ru/~s0mbre/archive/acrypto/drivers
 2. http://tservice.net.ru/~s0mbre/?section=projectsitem=bd
 
 diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
 --- a/drivers/md/dm-crypt.c
 +++ b/drivers/md/dm-crypt.c
 @@ -230,6 +230,173 @@ static struct crypt_iv_operations crypt_
   .generator = crypt_iv_essiv_gen
  };
  
 +#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
 +
 +static DECLARE_WAIT_QUEUE_HEAD(dm_async_queue);
 +static int dm_async_pending;
 +
 +#include linux/acrypto.h
 +#include linux/crypto_def.h
 +
 +static inline u16 crypto_tfm_get_type(struct crypto_tfm *tfm) {
 + u16 type;
 + char *name;
 +
 + name = (char *)crypto_tfm_alg_name(tfm);
 +
 + if (!strncmp(name, aes, 3)) {
 + switch (crypto_tfm_alg_blocksize(tfm)  3) {
 + case 128:
 + type = CRYPTO_TYPE_AES_128;
 + break;
 + case 192:
 + type = CRYPTO_TYPE_AES_192;
 + break;
 + case 256:
 + type = CRYPTO_TYPE_AES_256;
 + break;
 + default:
 + type = 0x;
 + break;
 + }
 + } else if (!strncmp(name, des3_ede, 3))
 + type = CRYPTO_TYPE_3DES;
 + else
 + type = 0x;
 +
 + return type;
 +}
 +
 +static inline u16 crypto_tfm_get_mode(struct crypto_tfm *tfm) {
 + u16 mode = tfm-crt_cipher.cit_mode  CRYPTO_TFM_MODE_MASK;
 +
 + switch (mode) {
 + case CRYPTO_TFM_MODE_ECB:
 + mode = CRYPTO_MODE_ECB;
 + break;
 + case CRYPTO_TFM_MODE_CBC:
 + mode = CRYPTO_MODE_CBC;
 + break;
 + case CRYPTO_TFM_MODE_CFB:
 + mode = CRYPTO_MODE_CFB;
 + break;
 + case CRYPTO_TFM_MODE_CTR:
 + mode = CRYPTO_MODE_CTR;
 + break;
 + default:
 + mode = 0x;
 + break;
 + }
 +
 + return mode;
 +}
 +
 +static void dm_callback(struct crypto_session_initializer *ci, struct 
 +crypto_data *data) {
 + dm_async_pending--;
 + 
 + kfree((void *)(data-priv));
 + wake_up(dm_async_queue);
 +}
 +
 +static inline int acrypto_process(struct crypt_config *cc, struct
 scatterlist *out, 
 + struct

Re: [ACRYPTO] dm-crypt ported to acrypto.

2005-09-04 Thread Evgeniy Polyakov

On Sun, Sep 04, 2005 at 10:41:02PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) 
wrote:
 Hi
  
 What do you think about the second remark??
 
  -I think that we might have a problem  if a write operation will be
  processed in parallel to a read operation, the read might wait for the
 
  write to complete, and the dm_async_pending might also get wrong 
  values???   
 
 I think that the pending counter , should be updated only in read
 operations (cb and convert ).

Hmm, how does read operation differ from write in respect to dm-crypt?
It either decrypts (read) or encrypts (write) cloned BIOs, and all operations
are strongly serialized.
dm_async_pending is a number of crypt operations to be performed by the
stack on given data, most of the time it is number of segments (bvec) in BIO,
so dm_async_pending is always set to number of operatons, so it should
be decremented when one operation is finished, no matter if it is read
(decrypt) or write (encrypt).

 Regards
 
 Ronen Shitrit 
 Marvell Semiconductor Israel Ltd
 
 -Original Message-
 From: [EMAIL PROTECTED]
 [mailto:[EMAIL PROTECTED] On Behalf Of Ronen Shitrit
 Sent: Sunday, September 04, 2005 10:10 PM
 To: [EMAIL PROTECTED]
 Subject: FW: [ACRYPTO] dm-crypt ported to acrypto.
 
  Evgeniy Polyakov wrote:
 
  Hi
   
  Nice job, (horoshaya rabota)
  Just two issue:
  -I saw this patch is using a global variable which counts the number
  pending requests: dm_async_pending.
  I think that this variable should be allocated per dm_crypt session, 
  i.e for each crypt_config, since there can be more then 2 consumers 
  reading through the dm_crypt.
  -I think that we might have a problem  if a write operation will be
  processed in parallel to a read operation, the read might wait for the
 
  write to complete, and the dm_async_pending might also get wrong 
  values???
 
 Updated patch attached.
 Only compile tested though - will run test tomorrow.
 
  Regards
  
  Ronen Shitrit
  Marvell Semiconductor Israel Ltd
 
 diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
 --- a/drivers/md/dm-crypt.c
 +++ b/drivers/md/dm-crypt.c
 @@ -85,6 +85,11 @@ struct crypt_config {
   struct crypto_tfm *tfm;
   unsigned int key_size;
   u8 key[0];
 +
 +#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
 + wait_queue_head_t dm_async_queue;
 + int dm_async_pending;
 +#endif
  };
  
  #define MIN_IOS256
 @@ -230,6 +235,186 @@ static struct crypt_iv_operations crypt_
   .generator = crypt_iv_essiv_gen
  };
  
 +#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
 +
 +#include linux/acrypto.h
 +#include linux/crypto_def.h
 +
 +struct dm_async_private
 +{
 + void*sg;
 + struct crypt_config *cc;
 +};
 +
 +static inline u16 crypto_tfm_get_type(struct crypto_tfm *tfm) {
 + u16 type;
 + char *name;
 +
 + name = (char *)crypto_tfm_alg_name(tfm);
 +
 + if (!strncmp(name, aes, 3)) {
 + switch (crypto_tfm_alg_blocksize(tfm)  3) {
 + case 128:
 + type = CRYPTO_TYPE_AES_128;
 + break;
 + case 192:
 + type = CRYPTO_TYPE_AES_192;
 + break;
 + case 256:
 + type = CRYPTO_TYPE_AES_256;
 + break;
 + default:
 + type = 0x;
 + break;
 + }
 + } else if (!strncmp(name, des3_ede, 3))
 + type = CRYPTO_TYPE_3DES;
 + else
 + type = 0x;
 +
 + return type;
 +}
 +
 +static inline u16 crypto_tfm_get_mode(struct crypto_tfm *tfm) {
 + u16 mode = tfm-crt_cipher.cit_mode  CRYPTO_TFM_MODE_MASK;
 +
 + switch (mode) {
 + case CRYPTO_TFM_MODE_ECB:
 + mode = CRYPTO_MODE_ECB;
 + break;
 + case CRYPTO_TFM_MODE_CBC:
 + mode = CRYPTO_MODE_CBC;
 + break;
 + case CRYPTO_TFM_MODE_CFB:
 + mode = CRYPTO_MODE_CFB;
 + break;
 + case CRYPTO_TFM_MODE_CTR:
 + mode = CRYPTO_MODE_CTR;
 + break;
 + default:
 + mode = 0x;
 + break;
 + }
 +
 + return mode;
 +}
 +
 +static void dm_callback(struct crypto_session_initializer *ci, struct 
 +crypto_data *data) {
 + struct dm_async_private *priv = data-priv;
 + 
 + priv-cc-dm_async_pending--;
 + wake_up(priv-cc-dm_async_queue);
 + 
 + kfree(priv-sg);
 +}
 +
 +static inline int acrypto_process(struct crypt_config *cc, struct
 scatterlist *out, 
 + struct scatterlist *in, unsigned int len, u8 *iv, int
 iv_size, int 
 +write) {
 + struct crypto_session *s;
 + struct crypto_session_initializer ci;
 +

RE: [ACRYPTO] dm-crypt ported to acrypto.

2005-09-04 Thread Ronen Shitrit

 Hi

Did you tried to format a large partition (let say 10G) encrypted by
dm-crypt with AES-CBC?
I'm trying this and I see, that write and read request are mixing
together.
Why? Cause in the current patch the Read is blocked until all its
fragments will be encrypted,
But the Write is not blocked it is queued and we return to the user,
before the encrypt is completed.

Regards

Ronen Shitrit 
Marvell Semiconductor Israel Ltd


-Original Message-
From: Evgeniy Polyakov [mailto:[EMAIL PROTECTED] 
Sent: Sunday, September 04, 2005 10:50 PM
To: Ronen Shitrit
Cc: [EMAIL PROTECTED]; linux-crypto@vger.kernel.org
Subject: Re: [ACRYPTO] dm-crypt ported to acrypto.

On Sun, Sep 04, 2005 at 10:41:02PM +0300, Ronen Shitrit
([EMAIL PROTECTED]) wrote:
 Hi
  
 What do you think about the second remark??
 
  -I think that we might have a problem  if a write operation will be
  processed in parallel to a read operation, the read might wait for 
  the
 
  write to complete, and the dm_async_pending might also get wrong 
  values???   
 
 I think that the pending counter , should be updated only in read 
 operations (cb and convert ).

Hmm, how does read operation differ from write in respect to dm-crypt?
It either decrypts (read) or encrypts (write) cloned BIOs, and all
operations are strongly serialized.
dm_async_pending is a number of crypt operations to be performed by the
stack on given data, most of the time it is number of segments (bvec) in
BIO, so dm_async_pending is always set to number of operatons, so it
should be decremented when one operation is finished, no matter if it is
read
(decrypt) or write (encrypt).

 Regards
 
 Ronen Shitrit
 Marvell Semiconductor Israel Ltd
 
 -Original Message-
 From: [EMAIL PROTECTED] 
 [mailto:[EMAIL PROTECTED] On Behalf Of Ronen Shitrit
 Sent: Sunday, September 04, 2005 10:10 PM
 To: [EMAIL PROTECTED]
 Subject: FW: [ACRYPTO] dm-crypt ported to acrypto.
 
  Evgeniy Polyakov wrote:
 
  Hi
   
  Nice job, (horoshaya rabota)
  Just two issue:
  -I saw this patch is using a global variable which counts the 
  -number
  pending requests: dm_async_pending.
  I think that this variable should be allocated per dm_crypt session,

  i.e for each crypt_config, since there can be more then 2 consumers 
  reading through the dm_crypt.
  -I think that we might have a problem  if a write operation will be
  processed in parallel to a read operation, the read might wait for 
  the
 
  write to complete, and the dm_async_pending might also get wrong 
  values???
 
 Updated patch attached.
 Only compile tested though - will run test tomorrow.
 
  Regards
  
  Ronen Shitrit
  Marvell Semiconductor Israel Ltd
 
 diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
 --- a/drivers/md/dm-crypt.c
 +++ b/drivers/md/dm-crypt.c
 @@ -85,6 +85,11 @@ struct crypt_config {
   struct crypto_tfm *tfm;
   unsigned int key_size;
   u8 key[0];
 +
 +#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
 + wait_queue_head_t dm_async_queue;
 + int dm_async_pending;
 +#endif
  };
  
  #define MIN_IOS256
 @@ -230,6 +235,186 @@ static struct crypt_iv_operations crypt_
   .generator = crypt_iv_essiv_gen
  };
  
 +#if defined CONFIG_ACRYPTO || defined CONFIG_ACRYPTO_MODULE
 +
 +#include linux/acrypto.h
 +#include linux/crypto_def.h
 +
 +struct dm_async_private
 +{
 + void*sg;
 + struct crypt_config *cc;
 +};
 +
 +static inline u16 crypto_tfm_get_type(struct crypto_tfm *tfm) {
 + u16 type;
 + char *name;
 +
 + name = (char *)crypto_tfm_alg_name(tfm);
 +
 + if (!strncmp(name, aes, 3)) {
 + switch (crypto_tfm_alg_blocksize(tfm)  3) {
 + case 128:
 + type = CRYPTO_TYPE_AES_128;
 + break;
 + case 192:
 + type = CRYPTO_TYPE_AES_192;
 + break;
 + case 256:
 + type = CRYPTO_TYPE_AES_256;
 + break;
 + default:
 + type = 0x;
 + break;
 + }
 + } else if (!strncmp(name, des3_ede, 3))
 + type = CRYPTO_TYPE_3DES;
 + else
 + type = 0x;
 +
 + return type;
 +}
 +
 +static inline u16 crypto_tfm_get_mode(struct crypto_tfm *tfm) {
 + u16 mode = tfm-crt_cipher.cit_mode  CRYPTO_TFM_MODE_MASK;
 +
 + switch (mode) {
 + case CRYPTO_TFM_MODE_ECB:
 + mode = CRYPTO_MODE_ECB;
 + break;
 + case CRYPTO_TFM_MODE_CBC:
 + mode = CRYPTO_MODE_CBC;
 + break;
 + case CRYPTO_TFM_MODE_CFB:
 + mode = CRYPTO_MODE_CFB;
 + break;
 + case CRYPTO_TFM_MODE_CTR:
 + mode = CRYPTO_MODE_CTR;
 +

Re: [ACRYPTO] dm-crypt ported to acrypto.

2005-09-04 Thread Evgeniy Polyakov

On Sun, Sep 04, 2005 at 11:27:24PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) 
wrote:
  Hi
 
 I already ported this patch to the OCF, and I'm using it over the OCF.
 I also don't see any issue with small partitions??
 
 The crypt_convert API is called from 2 different places in the code:
 1. For read requests through the kcrypt_do_work working Q, called from
 bi_end_io of the crypt_clone.
 2. For Write requests through the crypt_clone directly.

Ok, I see now.
Such schema was probably created to eliminate problem with buffer cache,
when it is accessed from kernel itself and should not have encrypted
data, but when it's data is wrtten to disk itself, BIOs should be already 
encrypted.

 In my recent version of the patch:
  I implemented a callback for read, which update the pending counter,
 and callback for write
  Which doesn't update it.
  And in the crypt_convert I update the pending counter only if it's a
 read operation.
 
 By doing this I'm making sure that only one read request will be handled
 at a time and the pending counter
 Will be updated correctly for the read requests.
 
 Now, I still see that I'm getting few writing requests inside the
 Acrypto Q at the same time.
 I think it is not OK  since currently the patch is not handling buffers
 correctly for this case: the write task
 Believe that the write was done, and it frees the buffers. (If I
 understood it correctly) ???

I think the best solution is just to put waiting for dm_async_pending to
become zero into crypt_convert().
Patch attached.
I will setup large partition tomorrow and send my results with it.

Thank you for catching this.

 
 I will send a patch, once the dm_crypt will be stable with the OCF.
 
 Ronen Shitrit 
 Marvell Semiconductor Israel Ltd

-- 
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: OCF or Acrypto for IPSec and dm-crypt

2005-09-04 Thread Herbert Xu

Ronen Shitrit [EMAIL PROTECTED] wrote:
 
 I think, Correct me if I'm wrong,
 What you described below, is 1 session with few operations (using the
 routing feature of the acrypto), but once the session/set of operations 
 are done, then the session is closed.

I see.  You want the session to last through the lifetime of an IPsec
SA, right?

Well the good news is that the Async Crypto API that I'm working on 
will do exactly that.  The session is identified with the existing
crypto_tfm object which is tied to each IPsec SA (for IPsec that is).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [ACRYPTO] dm-crypt ported to acrypto.

Re: [ACRYPTO] dm-crypt ported to acrypto.

Re: [ACRYPTO] dm-crypt ported to acrypto.

RE: [ACRYPTO] dm-crypt ported to acrypto.

Re: [ACRYPTO] dm-crypt ported to acrypto.

Re: OCF or Acrypto for IPSec and dm-crypt

6 matches

Site Navigation

Mail list logo

Footer information