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

[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

[Herbert Xu]

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?

Thanks,
-- 
Email: Herbert Xu 
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

[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

[Herbert Xu]

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.

Cheers,
-- 
Email: Herbert Xu 
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

[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

[Herbert Xu]

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.

> 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.

Cheers,
-- 
Email: Herbert Xu 
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

[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

[Herbert Xu]

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.

Cheers,
-- 
Email: Herbert Xu 
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

[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

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

[Herbert Xu]

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.

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

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

[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

[Herbert Xu]

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?

Thanks,
-- 
Email: Herbert Xu 
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

[Megha Dey]

On Thu, 2018-01-18 at 22:39 +1100, Herbert Xu wrote:
> On Tue, Jan 09, 2018 at 04:09:04PM -0800, Megha Dey wrote:
> >
> > +static void mcryptd_skcipher_encrypt(struct crypto_async_request *base,
> > +   int err)
> > +{
> > +   struct skcipher_request *req = skcipher_request_cast(base);
> > +   struct mcryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
> > +   struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> > +   struct mcryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> > +   struct crypto_skcipher *child = ctx->child;
> > +   struct skcipher_request subreq;
> > +
> > +   if (unlikely(err == -EINPROGRESS))
> > +   goto out;
> > +
> > +   /* set up the skcipher request to work on */
> > +   skcipher_request_set_tfm(, child);
> > +   skcipher_request_set_callback(,
> > +   CRYPTO_TFM_REQ_MAY_SLEEP, 0, 0);
> > +   skcipher_request_set_crypt(, req->src, req->dst,
> > +   req->cryptlen, req->iv);
> > +
> > +   /*
> > +* pass addr of descriptor stored in the request context
> > +* so that the callee can get to the request context
> > +*/
> > +   rctx->desc = subreq;
> > +   err = crypto_skcipher_encrypt(>desc);
> > +
> > +   if (err) {
> > +   req->base.complete = rctx->complete;
> > +   goto out;
> > +   }
> > +   return;
> > +
> > +out:
> > +   mcryptd_skcipher_complete(req, err);
> > +}
> 
> OK this looks better but it's still abusing the crypto API interface.
> In particular, you're sharing data with the underlying algorithm
> behind the crypto API's back.  Also, the underlying algorithm does
> callback completion behind the API's back through the shared data
> context.
> 
> It seems to me that the current mcryptd scheme is flawed.  You
> want to batch multiple requests and yet this isn't actually being
> done by mcryptd at all.  The actual batching happens at the very
> lowest level, i.e., in the crypto algorithm below mcryptd.  For
> example, with your patch, the batching appears to happen in
> aes_cbc_job_mgr_submit.
> 
> 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.
> 
> Cheers,

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

[Megha Dey]

In this patch, the infrastructure needed to support multibuffer
encryption implementation is added:

a) Enhance mcryptd daemon to support skcipher requests.

b) Add multi-buffer mcryptd skcipher helper which presents the
   top-level algorithm as an skcipher.

b) Update configuration to include multi-buffer encryption build
support.

For an introduction to the multi-buffer implementation, please see
http://www.intel.com/content/www/us/en/communications/communications-ia-multi-buffer-paper.html

Originally-by: Chandramouli Narayanan 
Signed-off-by: Megha Dey 
Acked-by: Tim Chen 
---
 crypto/Kconfig   |  15 ++
 crypto/mcryptd.c | 475 +++
 include/crypto/mcryptd.h |  56 ++
 3 files changed, 546 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 9327fbf..66bd682 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1021,6 +1021,21 @@ config CRYPTO_AES_NI_INTEL
  ECB, CBC, LRW, PCBC, XTS. The 64 bit version has additional
  acceleration for CTR.
 
+config CRYPTO_AES_CBC_MB
+tristate "AES CBC algorithm (x86_64 Multi-Buffer, Experimental)"
+depends on X86 && 64BIT
+select CRYPTO_SIMD
+select CRYPTO_MCRYPTD
+help
+  AES CBC encryption implemented using multi-buffer technique.
+  This algorithm computes on multiple data lanes concurrently with
+  SIMD instructions for better throughput. It should only be used
+  when we expect many concurrent crypto requests to keep all the
+  data lanes filled to realize the performance benefit. If the data
+  lanes are unfilled, a flush operation will be initiated after some
+  delay to process the exisiting crypto jobs, adding some extra
+  latency to low load case.
+
 config CRYPTO_AES_SPARC64
tristate "AES cipher algorithms (SPARC64)"
depends on SPARC64
diff --git a/crypto/mcryptd.c b/crypto/mcryptd.c
index 2908382..ce502e2 100644
--- a/crypto/mcryptd.c
+++ b/crypto/mcryptd.c
@@ -269,6 +269,443 @@ static inline bool mcryptd_check_internal(struct rtattr 
**tb, u32 *type,
return false;
 }
 
+static int mcryptd_enqueue_skcipher_request(struct mcryptd_queue *queue,
+   struct crypto_async_request *request,
+   struct mcryptd_skcipher_request_ctx *rctx)
+{
+   int cpu, err;
+   struct mcryptd_cpu_queue *cpu_queue;
+
+   cpu = get_cpu();
+   cpu_queue = this_cpu_ptr(queue->cpu_queue);
+   rctx->tag.cpu = cpu;
+
+   err = crypto_enqueue_request(_queue->queue, request);
+   pr_debug("enqueue request: cpu %d cpu_queue %p request %p\n",
+   cpu, cpu_queue, request);
+   queue_work_on(cpu, kcrypto_wq, _queue->work);
+   put_cpu();
+
+   return err;
+}
+
+static int mcryptd_skcipher_setkey(struct crypto_skcipher *parent,
+   const u8 *key, unsigned int keylen)
+{
+   struct mcryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(parent);
+   struct crypto_skcipher *child = ctx->child;
+   int err;
+
+   crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
+   crypto_skcipher_set_flags(child, crypto_skcipher_get_flags(parent) &
+   CRYPTO_TFM_REQ_MASK);
+   err = crypto_skcipher_setkey(child, key, keylen);
+   crypto_skcipher_set_flags(parent, crypto_skcipher_get_flags(child) &
+   CRYPTO_TFM_RES_MASK);
+   return err;
+}
+
+static void mcryptd_skcipher_complete(struct skcipher_request *req, int err)
+{
+   struct mcryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
+
+   local_bh_disable();
+   rctx->complete(>base, err);
+   local_bh_enable();
+}
+
+static void mcryptd_skcipher_encrypt(struct crypto_async_request *base,
+   int err)
+{
+   struct skcipher_request *req = skcipher_request_cast(base);
+   struct mcryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
+   struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+   struct mcryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
+   struct crypto_skcipher *child = ctx->child;
+   struct skcipher_request subreq;
+
+   if (unlikely(err == -EINPROGRESS))
+   goto out;
+
+   /* set up the skcipher request to work on */
+   skcipher_request_set_tfm(, child);
+   skcipher_request_set_callback(,
+   CRYPTO_TFM_REQ_MAY_SLEEP, 0, 0);
+   skcipher_request_set_crypt(, req->src, req->dst,
+   req->cryptlen, req->iv);
+
+   /*
+* pass addr of descriptor stored in the request context
+* so that the callee can get to the request context
+*/
+