RE: [PATCH 2/2] crypto : async implementation for sha1-mb

2016-06-02 Thread Dey, Megha


-Original Message-
From: Herbert Xu [mailto:herb...@gondor.apana.org.au] 
Sent: Thursday, June 2, 2016 5:33 PM
To: Dey, Megha <megha@intel.com>
Cc: tim.c.c...@linux.intel.com; da...@davemloft.net; 
linux-crypto@vger.kernel.org; linux-ker...@vger.kernel.org; Yu, Fenghua 
<fenghua...@intel.com>
Subject: Re: [PATCH 2/2] crypto : async implementation for sha1-mb

On Thu, Jun 02, 2016 at 10:20:20AM -0700, Megha Dey wrote:
>
> > > @@ -439,17 +444,18 @@ static int mcryptd_hash_finup_enqueue(struct 
> > > ahash_request *req)  static void mcryptd_hash_digest(struct 
> > > crypto_async_request *req_async, int err)  {
> > >   struct mcryptd_hash_ctx *ctx = crypto_tfm_ctx(req_async->tfm);
> > > - struct crypto_shash *child = ctx->child;
> > > + struct crypto_ahash *child = ctx->child;
> > >   struct ahash_request *req = ahash_request_cast(req_async);
> > >   struct mcryptd_hash_request_ctx *rctx = ahash_request_ctx(req);
> > > - struct shash_desc *desc = >desc;
> > > + struct ahash_request *desc = >areq;
> > > + struct crypto_async_request *base = >base;
> > >  
> > >   if (unlikely(err == -EINPROGRESS))
> > >   goto out;
> > > + base->tfm = >base;
> > > + base->flags = CRYPTO_TFM_REQ_MAY_SLEEP;  /* check this again */
> > 
> > You should not be touching crypto_async_request directly.  Use the 
> > proper ahash interface to set the child request.
> > 
> Herbert, Could you please clarify?
> In the earlier code we had a async_request which is now replaced by 
> crypto_async_request. Do you want a new async_request to be used?
> Do you think we shouldn't be setting the members of the 
> crypto_ahash_request directly, but use some other interface to do the 
> same for us?

You already have an ahash_request here.  So you should be doing

ahash_request_set_tfm(...)
ahash_request_set_callback(...)

>ok,done!
Thanks,
--
Email: Herbert Xu <herb...@gondor.apana.org.au> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V2 2/2] crypto : async implementation for sha1-mb

2016-06-07 Thread Dey, Megha


-Original Message-
From: Herbert Xu [mailto:herb...@gondor.apana.org.au] 
Sent: Tuesday, June 7, 2016 3:35 AM
To: Dey, Megha <megha@intel.com>
Cc: tim.c.c...@linux.intel.com; da...@davemloft.net; 
linux-crypto@vger.kernel.org; linux-ker...@vger.kernel.org; Yu, Fenghua 
<fenghua...@intel.com>; Megha Dey <megha@linux.intel.com>
Subject: Re: [PATCH V2 2/2] crypto : async implementation for sha1-mb

On Thu, Jun 02, 2016 at 07:53:50PM -0700, Megha Dey wrote:
>
> + struct ahash_alg *shash = crypto_ahash_alg(tfm);
>  
>   /* alignment is to be done by multi-buffer crypto algorithm if 
> needed */
>  
> - return shash->finup(desc, NULL, 0, req->result);
> + return shash->finup(desc);

You're still poking in the guts of the API.  Now that it's a real ahash you 
don't need to do that.

Just do crypto_ahash_finup.  That way you don't need to export crypto_ahsh_alg 
either.

> I have made these changes and re-sent the patch.

Thanks,
--
Email: Herbert Xu <herb...@gondor.apana.org.au> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] crypto : async implementation for sha1-mb

2016-06-13 Thread Dey, Megha


-Original Message-
From: Herbert Xu [mailto:herb...@gondor.apana.org.au] 
Sent: Monday, June 13, 2016 1:22 AM
To: Dey, Megha <megha@intel.com>
Cc: tim.c.c...@linux.intel.com; da...@davemloft.net; 
linux-crypto@vger.kernel.org; linux-ker...@vger.kernel.org; Yu, Fenghua 
<fenghua...@intel.com>; Megha Dey <megha@linux.intel.com>
Subject: Re: [PATCH] crypto : async implementation for sha1-mb

On Tue, Jun 07, 2016 at 11:14:42AM -0700, Megha Dey wrote:
>
> - desc->tfm = child;
> - desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> + ahash_request_set_tfm(desc, child);
> + ahash_request_set_callback(desc, CRYPTO_TFM_REQ_MAY_SLEEP, NULL, 
> +NULL);

Why are the callbacks set to NULL/NULL? The child is async so you should have a 
valid callback function here.

Instead of continuing to do the broken callback handling outside of the API 
(i.e., rctx->complete) please use the API mechanism that is provided for this 
purpose.

> In the current implementation, the inner algorithm is called directly, and we 
> use the outer algorithm's callback. We do not use the callback in inner 
> algorithm. We are actually calling the child functions directly and the child 
> is using the parent's call back function. Probably I can add a comment before 
> the set callback function.. will this be ok?

Thanks,
--
Email: Herbert Xu <herb...@gondor.apana.org.au> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: sha1_mb broken

2016-09-28 Thread Dey, Megha


-Original Message-
From: Stephan Mueller [mailto:smuel...@chronox.de] 
Sent: Wednesday, September 28, 2016 11:46 AM
To: Dey, Megha <megha@intel.com>
Cc: linux-crypto@vger.kernel.org; tim.c.c...@linux.intel.com
Subject: Re: sha1_mb broken

Am Mittwoch, 28. September 2016, 11:25:47 CEST schrieb Megha Dey:

Hi Megha,

> Hi Stephan,
> 
> There was a bug fix: Commit ID : 0851561d (introduced in 4.6-rc5).

I use the current cryptodev-2.6 tree.
> 
> Assuming that you are using an older kernel than this one, maybe we 
> are issuing the complete with the wrong pointer, so the original 
> issuer of the request never gets the complete back.
> 
> If you are using an older kernel, can you please use the lastest and 
> let me know if you still see this issue?
> 
> Also can you give more info on the test case? Does it issue single 
> request or multiple requests?

It is a single test with the message "8c899bba" where I expect 
"ac6d8c4851beacf61c175aed0699053d8f632df8"

>> For the message 8c899bba, the expected hash is  not 
>> ac6d8c4851beacf61c175aed0699053d8f632df8 but 
2e2816f6241fef89d78dd7afc926adc03ca94ace. I used this hash value in the 
existing tcrypt test and the test passes.

I would like to duplicate the test case you are using so that I can have a 
better look.
Can you provide the complete code for the test case? The code you sent earlier 
are missing some structure definitions. I will try to incorporate this test in 
the tcrypt test suite and try to reproduce this issue.

The code is the following which works with any other hash:

struct kccavs_ahash_def {
struct crypto_ahash *tfm;
struct ahash_request *req;
struct kccavs_tcrypt_res result;
};

/* Callback function */
static void kccavs_ahash_cb(struct crypto_async_request *req, int error) {
struct kccavs_tcrypt_res *result = req->data;

if (error == -EINPROGRESS)
return;
result->err = error;
complete(>completion);
dbg("Encryption finished successfully\n"); }

/* Perform hash */
static unsigned int kccavs_ahash_op(struct kccavs_ahash_def *ahash) {
int rc = 0;

rc = crypto_ahash_digest(ahash->req);

switch (rc) {
case 0:
break;
case -EINPROGRESS:
case -EBUSY:
rc = 
wait_for_completion_interruptible(>result.completion);
if (!rc && !ahash->result.err) {
#ifdef OLDASYNC
INIT_COMPLETION(aead->result.completion);
#else
reinit_completion(>result.completion);
#endif
break;
}
default:
dbg("ahash cipher operation returned with %d result"
" %d\n",rc, ahash->result.err);
break;
}
init_completion(>result.completion);

return rc;
}

static int kccavs_test_ahash(size_t nbytes) {
int ret;
struct crypto_ahash *tfm;
struct ahash_request *req = NULL;
struct kccavs_ahash_def ahash;
struct kccavs_data *data = _test->data;
struct kccavs_data *key = _test->key;
unsigned char *digest = NULL;
struct scatterlist sg;

/*
 * We explicitly do not check the input buffer as we allow
 * an empty string.
 */

/* allocate synchronous hash */
tfm = crypto_alloc_ahash(kccavs_test->name, 0, 0);
if (IS_ERR(tfm)) {
pr_info("could not allocate digest TFM handle for %s\n", 
kccavs_test-
>name);
return PTR_ERR(tfm);
}

digest = kzalloc(crypto_ahash_digestsize(tfm), GFP_KERNEL);
if (!digest) {
ret = -ENOMEM;
goto out;
}

req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
pr_info("could not allocate request queue\n");
ret = -ENOMEM;
goto out;
}
ahash.tfm = tfm;
ahash.req = req;

if (key->len) {
dbg("set key for HMAC\n");
ret = crypto_ahash_setkey(tfm, key->data, key->len);
if (ret < 0)
goto out;
}

sg_init_one(, data->data, data->len);
ahash_request_set_crypt(req, , digest, data->len);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
   kccavs_ahash_cb, );

ret = kccavs_ahash_op();
if (!ret) {
data->len = crypto_ahash_digestsize(tfm);
memcpy(data->data, digest, data->len);
}

out:
kzfree(digest);
if (req)
ahash_request_free(req);
crypto_free_ahash(tfm);
return ret;
}


Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: sha1_mb broken

2016-10-03 Thread Dey, Megha


-Original Message-
From: Stephan Mueller [mailto:smuel...@chronox.de] 
Sent: Wednesday, September 28, 2016 10:31 PM
To: Dey, Megha <megha@intel.com>
Cc: linux-crypto@vger.kernel.org; tim.c.c...@linux.intel.com
Subject: Re: sha1_mb broken

Am Mittwoch, 28. September 2016, 22:52:46 CEST schrieb Dey, Megha:

Hi Megha,

see a self contained example code attached.
 
> Hi Stephan,
>
> Your test code initialized the completion structure incorrectly, that led to 
> the missing completion from being received. The init_completion call should 
> be made before the crypto_ahash_digest call. The following change to your 
> test code fixes things. ( I have also fixed what I believe is a typo 
> aead->ahash)

> @@ -74,6 +74,8 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def 
> *ahash)
> {
>int rc = 0;
>
> +   init_completion(>result.completion);
> +
>rc = crypto_ahash_digest(ahash->req);
>
>switch (rc) {
>@@ -84,7 +86,7 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def 
>*ahash)
>rc = 
> wait_for_completion_interruptible(>result.completion);
>if (!rc && !ahash->result.err) {
> #ifdef OLDASYNC
> -   INIT_COMPLETION(aead->result.completion);
> +   INIT_COMPLETION(>result.completion);
> #else
>   reinit_completion(>result.completion);
> #endif
> @@ -95,7 +97,6 @@ static unsigned int kccavs_ahash_op(struct kccavs_ahash_def 
> *ahash)
>" %d\n",rc, ahash->result.err);
>break;
>   }
> -   init_completion(>result.completion);
>
> return rc;
>}
> This initialization of the completion structure happens correctly in the 
> tcrypt test module used by the kernel, hence I did not come across this issue 
> earlier.
> Thanks,
> Megha
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-05-11 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Thursday, May 10, 2018 9:46 PM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Fri, May 11, 2018 at 01:24:42AM +, Dey, Megha wrote:
>>
>> Are you suggesting that the SIMD wrapper, will do what is currently being
>done by the ' mcryptd_queue_worker ' function (assuming FPU is not disabled)
>i.e dispatching the job to the inner algorithm?
>>
>> I have got rid of the mcryptd layer( have an inner layer, outer SIMD layer,
>handled the pointers and completions accordingly), but still facing some issues
>after removing the per cpu mcryptd_cpu_queue.
>
>Why don't you post what you've got and we can work it out together?

Hi Herbert,

Sure, I will post an RFC patch. (crypto: Remove mcryptd). 

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


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-05-10 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Monday, May 7, 2018 2:35 AM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Tue, May 01, 2018 at 10:39:15PM +, Dey, Megha wrote:
>>
>> crypto/simd.c provides a simd_skcipher_create_compat. I have used the
>> same template to introduce simd_ahash_create_compat which would wrap
>around the inner hash algorithm.
>>
>> Hence we would still register 2 algs, outer and inner.
>
>Right.
>
>> Currently we have outer_alg -> mcryptd alg -> inner_alg
>>
>> Mcryptd is mainly providing the following:
>> 1. Ensuring the lanes(8 in case of AVX2) are full before dispatching to the
>lower inner algorithm. This is obviously why we would expect better
>performance for multi-buffer as opposed to the present single-buffer
>algorithms.
>> 2. If there no new incoming jobs, issue a flush.
>> 3. A glue layer which sends the correct pointers and completions.
>>
>> If we get rid of mcryptd, these functions needs to be done by someone. Since
>all multi-buffer algorithms would require this tasks, where do you suggest 
>these
>helpers live, if not the current mcryptd.c?
>
>That's the issue.  I don't think mcryptd is doing any of these claimed 
>functions
>except for hosting the flush helper which could really live anywhere.
>
>All these functions are actually being carried out in the inner algorithm 
>already.
>
>> I am not sure if you are suggesting that we need to get rid of the mcryptd
>work queue itself. In that case, we would need to execute in the context of the
>job requesting the crypto transformation.
>
>Which is fine as long as you can disable the FPU.  If not the simd wrapper will
>defer the job to kthread context as required.

Hi Herbert,

Are you suggesting that the SIMD wrapper, will do what is currently being done 
by the ' mcryptd_queue_worker ' function (assuming FPU is not disabled) i.e 
dispatching the job to the inner algorithm?

I have got rid of the mcryptd layer( have an inner layer, outer SIMD layer, 
handled the pointers and completions accordingly), but still facing some issues 
after removing the per cpu mcryptd_cpu_queue.
 
>
>Cheers,
>--
>Email: Herbert Xu <herb...@gondor.apana.org.au> Home Page:
>http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-05-01 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Thursday, April 26, 2018 2:45 AM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Wed, Apr 25, 2018 at 01:14:26AM +, Dey, Megha wrote:
>>
>> Is there any existing implementation of async crypto algorithm that uses the
>above approach? The ones I could find are either sync, have an outer and
>inner algorithm or use cryptd.
>>
>> I tried removing the mcryptd layer and the outer algorithm and some
>> plumbing to pass the correct structures, but see crashes.(obviously
>> some errors in the plumbing)
>
>OK, you can't just remove it because the inner algorithm requires
>kernel_fpu_begin/kernel_fpu_end.  So we do need two layers but I don't think
>we need cryptd or mcryptd.
>
>The existing simd wrapper should work just fine on the inner algorithm,
>provided that we add hash support to it.

Hi Herbert,

crypto/simd.c provides a simd_skcipher_create_compat. I have used the same 
template to introduce simd_ahash_create_compat
which would wrap around the inner hash algorithm.

Hence we would still register 2 algs, outer and inner.
>
>> I am not sure if we remove mcryptd, how would we queue work, flush
>partially completed jobs or call completions (currently done by mcryptd) if we
>simply call the inner algorithm.
>
>I don't think mcryptd is providing any real facility to the flushing apart 
>from a
>helper.  That same helper can live anywhere.

Currently we have outer_alg -> mcryptd alg -> inner_alg

Mcryptd is mainly providing the following:
1. Ensuring the lanes(8 in case of AVX2) are full before dispatching to the 
lower inner algorithm. This is obviously why we would expect better performance 
for multi-buffer as opposed to the present single-buffer algorithms.
2. If there no new incoming jobs, issue a flush.
3. A glue layer which sends the correct pointers and completions.

If we get rid of mcryptd, these functions needs to be done by someone. Since 
all multi-buffer algorithms would require this tasks, where do you suggest 
these helpers live, if not the current mcryptd.c?

I am not sure if you are suggesting that we need to get rid of the mcryptd work 
queue itself. In that case, we would need to execute in the context of the job 
requesting the crypto transformation.
>
>Cheers,
>--
>Email: Herbert Xu <herb...@gondor.apana.org.au> Home Page:
>http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-04-24 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Wednesday, April 18, 2018 8:25 PM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Thu, Apr 19, 2018 at 12:54:16AM +, Dey, Megha wrote:
>>
>> Yeah I think I misunderstood. I think what you mean is to remove mcryptd.c
>completely and avoid the extra layer of indirection to call the underlying
>algorithm, instead call it directly, correct?
>>
>> So currently we have 3 algorithms registered for every multibuffer algorithm:
>> name : __sha1-mb
>> driver   : mcryptd(__intel_sha1-mb)
>>
>> name : sha1
>> driver   : sha1_mb
>>
>> name : __sha1-mb
>> driver   : __intel_sha1-mb
>>
>> If we remove mcryptd, then we will have just the 2?
>
>It should be down to just one, i.e., the current inner algorithm.
>It's doing all the scheduling work already so I don't really see why it needs 
>the
>wrappers around it.

Hi Herbert,

Is there any existing implementation of async crypto algorithm that uses the 
above approach? The ones I could find are either sync, have an outer and inner 
algorithm or use cryptd.

I tried removing the mcryptd layer and the outer algorithm and some plumbing to 
pass the correct structures, but see crashes.(obviously some errors in the 
plumbing)

I am not sure if we remove mcryptd, how would we queue work, flush partially 
completed jobs or call completions (currently done by mcryptd) if we simply 
call the inner algorithm.

Are you suggesting these are not required at all? I am not sure how to move 
forward.

>
>Cheers,
>--
>Email: Herbert Xu <herb...@gondor.apana.org.au> Home Page:
>http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-04-17 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Friday, March 16, 2018 7:54 AM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Thu, Jan 18, 2018 at 04:44:21PM -0800, Megha Dey wrote:
>>
>> > So the mcryptd template is in fact completely superfluous.  You can
>> > remove it and just have all the main encrypt/decrypt functions
>> > invoke the underlying encrypt/decrypt function directly and achieve
>> > the same result.
>> >
>> > Am I missing something?
>>
>> Hi Herbert,
>>
>> After discussing with Tim, it seems like the mcryptd is responsible
>> for queuing up the encrypt requests and dispatching them to the actual
>> multi-buffer raw algorithm.  It also flushes the queue if we wait too
>> long without new requests coming in to force dispatch of the requests
>> in queue.
>>
>> Its function is analogous to cryptd but it has its own multi-lane
>> twists so we haven't reused the cryptd interface.
>
>I have taken a deeper look and I'm even more convinced now that mcryptd is
>simply not needed in your current model.
>
>The only reason you would need mcryptd is if you need to limit the rate of
>requests going into the underlying mb algorithm.
>
>However, it doesn't do that all.  Even though it seems to have a batch size of
>10, but because it immediately reschedules itself after the batch runs out,
>it's essentially just dumping all requests at the underlying algorithm as fast
>as they're coming in.  The underlying algorithm doesn't have need throttling
>anyway because it'll do the work when the queue is full synchronously.
>
>So why not just get rid of mcryptd completely and expose the underlying
>algorithm as a proper async skcipher/hash?

Hi Herbert,

Most part of the cryptd.c and mcryptd.c are similar, except the logic used to 
flush out partially completed jobs
in the case of multibuffer algorithms.

I think I will try to merge the cryptd and mcryptd adding necessary quirks for 
multibuffer where needed.

Also, in cryptd.c, I see shash interface being used for hash digests, update, 
finup, setkey etc. whereas we have shifted
to ahash interface for mcryptd. Is this correct?

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


RE: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support

2018-04-18 Thread Dey, Megha


>-Original Message-
>From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>Sent: Wednesday, April 18, 2018 4:01 AM
>To: Dey, Megha <megha@intel.com>
>Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>da...@davemloft.net
>Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure
>support
>
>On Tue, Apr 17, 2018 at 06:40:17PM +, Dey, Megha wrote:
>>
>>
>> >-Original Message-
>> >From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
>> >Sent: Friday, March 16, 2018 7:54 AM
>> >To: Dey, Megha <megha@intel.com>
>> >Cc: linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org;
>> >da...@davemloft.net
>> >Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption
>> >infrastructure support
>> >
>> >I have taken a deeper look and I'm even more convinced now that
>> >mcryptd is simply not needed in your current model.
>> >
>> >The only reason you would need mcryptd is if you need to limit the
>> >rate of requests going into the underlying mb algorithm.
>> >
>> >However, it doesn't do that all.  Even though it seems to have a
>> >batch size of 10, but because it immediately reschedules itself after
>> >the batch runs out, it's essentially just dumping all requests at the
>> >underlying algorithm as fast as they're coming in.  The underlying
>> >algorithm doesn't have need throttling anyway because it'll do the work
>when the queue is full synchronously.
>> >
>> >So why not just get rid of mcryptd completely and expose the
>> >underlying algorithm as a proper async skcipher/hash?
>>
>> Hi Herbert,
>>
>> Most part of the cryptd.c and mcryptd.c are similar, except the logic
>> used to flush out partially completed jobs in the case of multibuffer
>algorithms.
>>
>> I think I will try to merge the cryptd and mcryptd adding necessary quirks 
>> for
>multibuffer where needed.
>
>I think you didn't quite get my point.  From what I'm seeing you don't need
>either cryptd or mcryptd.  You just need to expose the underlying mb
>algorithm directly.

Hi Herbert,

Yeah I think I misunderstood. I think what you mean is to remove mcryptd.c 
completely and avoid the extra layer of indirection to call the underlying 
algorithm, instead call it directly, correct?

So currently we have 3 algorithms registered for every multibuffer algorithm:
name : __sha1-mb
driver   : mcryptd(__intel_sha1-mb)

name : sha1
driver   : sha1_mb

name : __sha1-mb
driver   : __intel_sha1-mb

If we remove mcryptd, then we will have just the 2?

The outer algorithm:sha1-mb, will 
>
>So I'm not sure what we would gain from merging cryptd and mcryptd.
>
>Cheers,
>--
>Email: Herbert Xu <herb...@gondor.apana.org.au> Home Page:
>http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt