Patchwork [v2,4/4] crypto: add CRYPTO_TFM_REQ_PARALLEL flag

login
register
mail settings
Submitter Stephan Mueller
Date Feb. 7, 2018, 7:44 a.m.
Message ID <3375426.msL1YKqpVq@positron.chronox.de>
Download mbox | patch
Permalink /patch/10204723/
State Superseded
Delegated to: Herbert Xu
Headers show

Comments

Stephan Mueller - Feb. 7, 2018, 7:44 a.m.
Crypto drivers may implement a streamlined serialization support for AIO
requests that is reported by the CRYPTO_TFM_REQ_PARALLEL flag to the
crypto user. When the user decides that he wants to send multiple AIO
requests concurrently and wants the crypto driver to handle the
serialization, the caller has to set CRYPTO_TFM_REQ_PARALLEL to notify
the crypto driver.

Only when this flag is enabled, the crypto driver shall apply its
serialization logic for handling IV updates between requests. If this
flag is not provided, the serialization logic shall not be applied by
the driver as the caller decides that it does not need it (because no
parallel AIO requests are sent) or that it performs its own
serialization.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/algif_aead.c     | 15 ++++++++++++---
 crypto/algif_skcipher.c | 15 ++++++++++++---
 include/linux/crypto.h  |  1 +
 3 files changed, 25 insertions(+), 6 deletions(-)
Stephan Mueller - Feb. 7, 2018, 12:48 p.m.
Am Mittwoch, 7. Februar 2018, 08:44:04 CET schrieb Stephan Müller:

Hi,


> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index 3970ad7f6fd0..da010405eea0 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -66,13 +66,22 @@ static int aead_sendmsg(struct socket *sock, struct
> msghdr *msg, size_t size) {
>  	struct sock *sk = sock->sk;
>  	struct alg_sock *ask = alg_sk(sk);
> +	struct af_alg_ctx *ctx = ask->private;
>  	struct sock *psk = ask->parent;
>  	struct alg_sock *pask = alg_sk(psk);
>  	struct aead_tfm *aeadc = pask->private;
> -	struct crypto_aead *tfm = aeadc->aead;
> -	unsigned int ivsize = crypto_aead_ivsize(tfm);
> +	struct crypto_aead *aead = aeadc->aead;
> +	struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> +	unsigned int ivsize = crypto_aead_ivsize(aead);
> +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> +
> +	if (ret < 0)
> +		return ret;
> 
> -	return af_alg_sendmsg(sock, msg, size, ivsize);
> +	if (ctx->iiv == ALG_IIV_USE)

This should be ALG_IIV_DISABLE of course.

> +		tfm->crt_flags |= CRYPTO_TFM_REQ_PARALLEL;
> +
> +	return ret;
>  }
> 
>  static int crypto_aead_copy_sgl(struct crypto_skcipher *null_tfm,
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index aee602a3ec24..225546666cf7 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -43,12 +43,21 @@ static int skcipher_sendmsg(struct socket *sock, struct
> msghdr *msg, {
>  	struct sock *sk = sock->sk;
>  	struct alg_sock *ask = alg_sk(sk);
> +	struct af_alg_ctx *ctx = ask->private;
>  	struct sock *psk = ask->parent;
>  	struct alg_sock *pask = alg_sk(psk);
> -	struct crypto_skcipher *tfm = pask->private;
> -	unsigned ivsize = crypto_skcipher_ivsize(tfm);
> +	struct crypto_skcipher *skc = pask->private;
> +	struct crypto_tfm *tfm = crypto_skcipher_tfm(skc);
> +	unsigned int ivsize = crypto_skcipher_ivsize(skc);
> +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> +
> +	if (ret < 0)
> +		return ret;
> 
> -	return af_alg_sendmsg(sock, msg, size, ivsize);
> +	if (ctx->iiv == ALG_IIV_USE)

Dto.

I will wait for some more comments before sending a new patch.

Ciao
Stephan
Jonathan Cameron - Feb. 7, 2018, 3:39 p.m.
On Wed, 7 Feb 2018 13:48:32 +0100
Stephan Mueller <smueller@chronox.de> wrote:

> Am Mittwoch, 7. Februar 2018, 08:44:04 CET schrieb Stephan Müller:
> 
> Hi,
> 
> 
> > diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> > index 3970ad7f6fd0..da010405eea0 100644
> > --- a/crypto/algif_aead.c
> > +++ b/crypto/algif_aead.c
> > @@ -66,13 +66,22 @@ static int aead_sendmsg(struct socket *sock, struct
> > msghdr *msg, size_t size) {
> >  	struct sock *sk = sock->sk;
> >  	struct alg_sock *ask = alg_sk(sk);
> > +	struct af_alg_ctx *ctx = ask->private;
> >  	struct sock *psk = ask->parent;
> >  	struct alg_sock *pask = alg_sk(psk);
> >  	struct aead_tfm *aeadc = pask->private;
> > -	struct crypto_aead *tfm = aeadc->aead;
> > -	unsigned int ivsize = crypto_aead_ivsize(tfm);
> > +	struct crypto_aead *aead = aeadc->aead;
> > +	struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> > +	unsigned int ivsize = crypto_aead_ivsize(aead);
> > +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > 
> > -	return af_alg_sendmsg(sock, msg, size, ivsize);
> > +	if (ctx->iiv == ALG_IIV_USE)  
> 
> This should be ALG_IIV_DISABLE of course.

You say that, but my initial reading was that the core
was requesting that the driver do things in parallel
irrespective of whether the driver thought it was safe.
So I would think this was correct.

Definitely needs some documentation or a clearer name.

> 
> > +		tfm->crt_flags |= CRYPTO_TFM_REQ_PARALLEL;
> > +
> > +	return ret;
> >  }
> > 
> >  static int crypto_aead_copy_sgl(struct crypto_skcipher *null_tfm,
> > diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> > index aee602a3ec24..225546666cf7 100644
> > --- a/crypto/algif_skcipher.c
> > +++ b/crypto/algif_skcipher.c
> > @@ -43,12 +43,21 @@ static int skcipher_sendmsg(struct socket *sock, struct
> > msghdr *msg, {
> >  	struct sock *sk = sock->sk;
> >  	struct alg_sock *ask = alg_sk(sk);
> > +	struct af_alg_ctx *ctx = ask->private;
> >  	struct sock *psk = ask->parent;
> >  	struct alg_sock *pask = alg_sk(psk);
> > -	struct crypto_skcipher *tfm = pask->private;
> > -	unsigned ivsize = crypto_skcipher_ivsize(tfm);
> > +	struct crypto_skcipher *skc = pask->private;
> > +	struct crypto_tfm *tfm = crypto_skcipher_tfm(skc);
> > +	unsigned int ivsize = crypto_skcipher_ivsize(skc);
> > +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> > +
> > +	if (ret < 0)
> > +		return ret;
> > 
> > -	return af_alg_sendmsg(sock, msg, size, ivsize);
> > +	if (ctx->iiv == ALG_IIV_USE)  
> 
> Dto.
> 
> I will wait for some more comments before sending a new patch.
> 
> Ciao
> Stephan
> 
>
Stephan Mueller - Feb. 7, 2018, 3:43 p.m.
Am Mittwoch, 7. Februar 2018, 16:39:11 CET schrieb Jonathan Cameron:

Hi Jonathan,

> On Wed, 7 Feb 2018 13:48:32 +0100
> 
> Stephan Mueller <smueller@chronox.de> wrote:
> > Am Mittwoch, 7. Februar 2018, 08:44:04 CET schrieb Stephan Müller:
> > 
> > Hi,
> > 
> > > diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> > > index 3970ad7f6fd0..da010405eea0 100644
> > > --- a/crypto/algif_aead.c
> > > +++ b/crypto/algif_aead.c
> > > @@ -66,13 +66,22 @@ static int aead_sendmsg(struct socket *sock, struct
> > > msghdr *msg, size_t size) {
> > > 
> > >  	struct sock *sk = sock->sk;
> > >  	struct alg_sock *ask = alg_sk(sk);
> > > 
> > > +	struct af_alg_ctx *ctx = ask->private;
> > > 
> > >  	struct sock *psk = ask->parent;
> > >  	struct alg_sock *pask = alg_sk(psk);
> > >  	struct aead_tfm *aeadc = pask->private;
> > > 
> > > -	struct crypto_aead *tfm = aeadc->aead;
> > > -	unsigned int ivsize = crypto_aead_ivsize(tfm);
> > > +	struct crypto_aead *aead = aeadc->aead;
> > > +	struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> > > +	unsigned int ivsize = crypto_aead_ivsize(aead);
> > > +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> > > +
> > > +	if (ret < 0)
> > > +		return ret;
> > > 
> > > -	return af_alg_sendmsg(sock, msg, size, ivsize);
> > > +	if (ctx->iiv == ALG_IIV_USE)
> > 
> > This should be ALG_IIV_DISABLE of course.
> 
> You say that, but my initial reading was that the core
> was requesting that the driver do things in parallel
> irrespective of whether the driver thought it was safe.
> So I would think this was correct.
> 
> Definitely needs some documentation or a clearer name.

How about:

ALG_IV_SERIAL_PROCESSING (was ALG_IIV_DISABLE)
ALG_IV_PARALLEL_PROCESSING (was ALG_IIV_USE)

Ciao
Stephan
Jonathan Cameron - Feb. 7, 2018, 4:14 p.m.
On Wed, 7 Feb 2018 16:43:10 +0100
Stephan Mueller <smueller@chronox.de> wrote:

> Am Mittwoch, 7. Februar 2018, 16:39:11 CET schrieb Jonathan Cameron:
> 
> Hi Jonathan,
> 
> > On Wed, 7 Feb 2018 13:48:32 +0100
> > 
> > Stephan Mueller <smueller@chronox.de> wrote:  
> > > Am Mittwoch, 7. Februar 2018, 08:44:04 CET schrieb Stephan Müller:
> > > 
> > > Hi,
> > >   
> > > > diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> > > > index 3970ad7f6fd0..da010405eea0 100644
> > > > --- a/crypto/algif_aead.c
> > > > +++ b/crypto/algif_aead.c
> > > > @@ -66,13 +66,22 @@ static int aead_sendmsg(struct socket *sock, struct
> > > > msghdr *msg, size_t size) {
> > > > 
> > > >  	struct sock *sk = sock->sk;
> > > >  	struct alg_sock *ask = alg_sk(sk);
> > > > 
> > > > +	struct af_alg_ctx *ctx = ask->private;
> > > > 
> > > >  	struct sock *psk = ask->parent;
> > > >  	struct alg_sock *pask = alg_sk(psk);
> > > >  	struct aead_tfm *aeadc = pask->private;
> > > > 
> > > > -	struct crypto_aead *tfm = aeadc->aead;
> > > > -	unsigned int ivsize = crypto_aead_ivsize(tfm);
> > > > +	struct crypto_aead *aead = aeadc->aead;
> > > > +	struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> > > > +	unsigned int ivsize = crypto_aead_ivsize(aead);
> > > > +	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
> > > > +
> > > > +	if (ret < 0)
> > > > +		return ret;
> > > > 
> > > > -	return af_alg_sendmsg(sock, msg, size, ivsize);
> > > > +	if (ctx->iiv == ALG_IIV_USE)  
> > > 
> > > This should be ALG_IIV_DISABLE of course.  
> > 
> > You say that, but my initial reading was that the core
> > was requesting that the driver do things in parallel
> > irrespective of whether the driver thought it was safe.
> > So I would think this was correct.
> > 
> > Definitely needs some documentation or a clearer name.  
> 
> How about:
> 
> ALG_IV_SERIAL_PROCESSING (was ALG_IIV_DISABLE)
> ALG_IV_PARALLEL_PROCESSING (was ALG_IIV_USE)
> 
Actually those were fine on the basis that inline iv is
obvious enough, it was CRYPTO_TFM_REQ_PARALLEL that
was causing me confusion.

Sorry, wasn't terribly clear on that!

Jonathan

> Ciao
> Stephan
> 
>
Stephan Mueller - Feb. 7, 2018, 4:25 p.m.
Am Mittwoch, 7. Februar 2018, 17:14:12 CET schrieb Jonathan Cameron:

Hi Jonathan,

> Actually those were fine on the basis that inline iv is
> obvious enough, it was CRYPTO_TFM_REQ_PARALLEL that
> was causing me confusion.
> 
> Sorry, wasn't terribly clear on that!

Ok, I will add some comments around that new REQ flag.

Ciao
Stephan

Patch

diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 3970ad7f6fd0..da010405eea0 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -66,13 +66,22 @@  static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
+	struct af_alg_ctx *ctx = ask->private;
 	struct sock *psk = ask->parent;
 	struct alg_sock *pask = alg_sk(psk);
 	struct aead_tfm *aeadc = pask->private;
-	struct crypto_aead *tfm = aeadc->aead;
-	unsigned int ivsize = crypto_aead_ivsize(tfm);
+	struct crypto_aead *aead = aeadc->aead;
+	struct crypto_tfm *tfm = crypto_aead_tfm(aead);
+	unsigned int ivsize = crypto_aead_ivsize(aead);
+	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
+
+	if (ret < 0)
+		return ret;
 
-	return af_alg_sendmsg(sock, msg, size, ivsize);
+	if (ctx->iiv == ALG_IIV_USE)
+		tfm->crt_flags |= CRYPTO_TFM_REQ_PARALLEL;
+
+	return ret;
 }
 
 static int crypto_aead_copy_sgl(struct crypto_skcipher *null_tfm,
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index aee602a3ec24..225546666cf7 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -43,12 +43,21 @@  static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
 {
 	struct sock *sk = sock->sk;
 	struct alg_sock *ask = alg_sk(sk);
+	struct af_alg_ctx *ctx = ask->private;
 	struct sock *psk = ask->parent;
 	struct alg_sock *pask = alg_sk(psk);
-	struct crypto_skcipher *tfm = pask->private;
-	unsigned ivsize = crypto_skcipher_ivsize(tfm);
+	struct crypto_skcipher *skc = pask->private;
+	struct crypto_tfm *tfm = crypto_skcipher_tfm(skc);
+	unsigned int ivsize = crypto_skcipher_ivsize(skc);
+	int ret = af_alg_sendmsg(sock, msg, size, ivsize);
+
+	if (ret < 0)
+		return ret;
 
-	return af_alg_sendmsg(sock, msg, size, ivsize);
+	if (ctx->iiv == ALG_IIV_USE)
+		tfm->crt_flags |= CRYPTO_TFM_REQ_PARALLEL;
+
+	return ret;
 }
 
 static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 4860aa2c9be4..95c9d6e8fb70 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -133,6 +133,7 @@ 
 #define CRYPTO_TFM_REQ_WEAK_KEY		0x00000100
 #define CRYPTO_TFM_REQ_MAY_SLEEP	0x00000200
 #define CRYPTO_TFM_REQ_MAY_BACKLOG	0x00000400
+#define CRYPTO_TFM_REQ_PARALLEL		0x00000800
 #define CRYPTO_TFM_RES_WEAK_KEY		0x00100000
 #define CRYPTO_TFM_RES_BAD_KEY_LEN   	0x00200000
 #define CRYPTO_TFM_RES_BAD_KEY_SCHED 	0x00400000