LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Megha Dey <megha.dey@intel.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	davem@davemloft.net
Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support
Date: Thu, 18 Jan 2018 16:44:21 -0800	[thread overview]
Message-ID: <1516322661.2526.3.camel@megha-Z97X-UD7-TH> (raw)
In-Reply-To: <20180118113905.GA19904@gondor.apana.org.au>

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(&subreq, child);
> > +	skcipher_request_set_callback(&subreq,
> > +					CRYPTO_TFM_REQ_MAY_SLEEP, 0, 0);
> > +	skcipher_request_set_crypt(&subreq, 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(&rctx->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,

  reply	other threads:[~2018-01-19  0:28 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10  0:09 [PATCH V8 0/5] crypto: AES CBC multibuffer implementation Megha Dey
2018-01-10  0:09 ` [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support Megha Dey
2018-01-18 11:39   ` Herbert Xu
2018-01-19  0:44     ` Megha Dey [this message]
2018-03-16 14:53       ` Herbert Xu
2018-04-17 18:40         ` Dey, Megha
2018-04-18 11:01           ` Herbert Xu
2018-04-19  0:54             ` Dey, Megha
2018-04-19  3:25               ` Herbert Xu
2018-04-25  1:14                 ` Dey, Megha
2018-04-26  9:44                   ` Herbert Xu
2018-05-01 22:39                     ` Dey, Megha
2018-05-07  9:35                       ` Herbert Xu
2018-05-11  1:24                         ` Dey, Megha
2018-05-11  4:45                           ` Herbert Xu
2018-05-12  1:21                             ` Dey, Megha
2018-06-21  1:05                             ` Dey, Megha
2018-01-10  0:09 ` [PATCH V8 2/5] crypto: AES CBC multi-buffer data structures Megha Dey
2018-01-10  0:09 ` [PATCH V8 3/5] crypto: AES CBC multi-buffer scheduler Megha Dey
2018-01-10  0:09 ` [PATCH V8 4/5] crypto: AES CBC by8 encryption Megha Dey
2018-01-10  0:09 ` [PATCH V8 5/5] crypto: AES CBC multi-buffer glue code Megha Dey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1516322661.2526.3.camel@megha-Z97X-UD7-TH \
    --to=megha.dey@intel.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).