LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Megha Dey <megha.dey@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	davem@davemloft.net, megha.dey@intel.com
Subject: Re: [PATCH V8 1/5] crypto: Multi-buffer encryption infrastructure support
Date: Thu, 18 Jan 2018 22:39:06 +1100	[thread overview]
Message-ID: <20180118113905.GA19904@gondor.apana.org.au> (raw)
In-Reply-To: <1515542948-24041-2-git-send-email-megha.dey@linux.intel.com>

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?

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

  reply	other threads:[~2018-01-18 11:39 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 [this message]
2018-01-19  0:44     ` Megha Dey
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=20180118113905.GA19904@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=davem@davemloft.net \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=megha.dey@intel.com \
    --cc=megha.dey@linux.intel.com \
    --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).