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 2/2] crypto : async implementation for sha1-mb

2016-06-02 Thread Herbert Xu 
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(...)

Thanks,
-- 
Email: Herbert Xu 
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 2/2] crypto : async implementation for sha1-mb

2016-06-02 Thread Megha Dey 
On Thu, 2016-06-02 at 18:33 +0800, Herbert Xu wrote:
> On Tue, May 31, 2016 at 02:42:21PM -0700, Megha Dey wrote:
> >
> > @@ -416,8 +421,8 @@ static void mcryptd_hash_finup(struct 
> > crypto_async_request *req_async, int err)
> >  
> > if (unlikely(err == -EINPROGRESS))
> > goto out;
> > -
> > -   err = shash_ahash_mcryptd_finup(req, >desc);
> > +   rctx->out = req->result;
> > +   err = shash_ahash_mcryptd_finup(req, >areq);
> 
> These shash_ahash functions should be renamed.
> 
> Also why are they exported?

taken care of this.
> 
> > @@ -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?
> Thanks,


--
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 2/2] crypto : async implementation for sha1-mb

2016-06-02 Thread Herbert Xu 
On Tue, May 31, 2016 at 02:42:21PM -0700, Megha Dey wrote:
>
> @@ -416,8 +421,8 @@ static void mcryptd_hash_finup(struct 
> crypto_async_request *req_async, int err)
>  
>   if (unlikely(err == -EINPROGRESS))
>   goto out;
> -
> - err = shash_ahash_mcryptd_finup(req, >desc);
> + rctx->out = req->result;
> + err = shash_ahash_mcryptd_finup(req, >areq);

These shash_ahash functions should be renamed.

Also why are they exported?

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

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

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

2016-05-31 Thread Megha Dey 
From: Megha Dey 

Herbert wants the sha1-mb algorithm to have an async implementation:
https://lkml.org/lkml/2016/4/5/286.
Currently, sha1-mb uses an async interface for the outer algorithm
and a sync interface for the inner algorithm. This patch introduces
a async interface for even the inner algorithm.

Signed-off-by: Megha Dey 
Signed-off-by: Tim Chen 
---
 arch/x86/crypto/sha-mb/sha1_mb.c | 190 ++-
 crypto/ahash.c   |   6 --
 crypto/mcryptd.c | 117 +---
 include/crypto/hash.h|   6 ++
 include/crypto/internal/hash.h   |   8 +-
 include/crypto/mcryptd.h |   8 +-
 6 files changed, 184 insertions(+), 151 deletions(-)

diff --git a/arch/x86/crypto/sha-mb/sha1_mb.c b/arch/x86/crypto/sha-mb/sha1_mb.c
index 0a46491..efc19e3 100644
--- a/arch/x86/crypto/sha-mb/sha1_mb.c
+++ b/arch/x86/crypto/sha-mb/sha1_mb.c
@@ -68,6 +68,7 @@
 #include 
 #include 
 #include "sha_mb_ctx.h"
+#include 
 
 #define FLUSH_INTERVAL 1000 /* in usec */
 
@@ -80,10 +81,10 @@ struct sha1_mb_ctx {
 static inline struct mcryptd_hash_request_ctx
*cast_hash_to_mcryptd_ctx(struct sha1_hash_ctx *hash_ctx)
 {
-   struct shash_desc *desc;
+   struct ahash_request *areq;
 
-   desc = container_of((void *) hash_ctx, struct shash_desc, __ctx);
-   return container_of(desc, struct mcryptd_hash_request_ctx, desc);
+   areq = container_of((void *) hash_ctx, struct ahash_request, __ctx);
+   return container_of(areq, struct mcryptd_hash_request_ctx, areq);
 }
 
 static inline struct ahash_request
@@ -93,7 +94,7 @@ static inline struct ahash_request
 }
 
 static void req_ctx_init(struct mcryptd_hash_request_ctx *rctx,
-   struct shash_desc *desc)
+   struct ahash_request *areq)
 {
rctx->flag = HASH_UPDATE;
 }
@@ -375,9 +376,9 @@ static struct sha1_hash_ctx *sha1_ctx_mgr_flush(struct 
sha1_ctx_mgr *mgr)
}
 }
 
-static int sha1_mb_init(struct shash_desc *desc)
+static int sha1_mb_init(struct ahash_request *areq)
 {
-   struct sha1_hash_ctx *sctx = shash_desc_ctx(desc);
+   struct sha1_hash_ctx *sctx = ahash_request_ctx(areq);
 
hash_ctx_init(sctx);
sctx->job.result_digest[0] = SHA1_H0;
@@ -395,7 +396,7 @@ static int sha1_mb_init(struct shash_desc *desc)
 static int sha1_mb_set_results(struct mcryptd_hash_request_ctx *rctx)
 {
int i;
-   struct  sha1_hash_ctx *sctx = shash_desc_ctx(>desc);
+   struct  sha1_hash_ctx *sctx = ahash_request_ctx(>areq);
__be32  *dst = (__be32 *) rctx->out;
 
for (i = 0; i < 5; ++i)
@@ -427,7 +428,7 @@ static int sha_finish_walk(struct mcryptd_hash_request_ctx 
**ret_rctx,
 
}
sha_ctx = (struct sha1_hash_ctx *)
-   shash_desc_ctx(>desc);
+   ahash_request_ctx(>areq);
kernel_fpu_begin();
sha_ctx = sha1_ctx_mgr_submit(cstate->mgr, sha_ctx,
rctx->walk.data, nbytes, flag);
@@ -519,11 +520,10 @@ static void sha1_mb_add_list(struct 
mcryptd_hash_request_ctx *rctx,
mcryptd_arm_flusher(cstate, delay);
 }
 
-static int sha1_mb_update(struct shash_desc *desc, const u8 *data,
- unsigned int len)
+static int sha1_mb_update(struct ahash_request *areq)
 {
struct mcryptd_hash_request_ctx *rctx =
-   container_of(desc, struct mcryptd_hash_request_ctx, desc);
+   container_of(areq, struct mcryptd_hash_request_ctx, areq);
struct mcryptd_alg_cstate *cstate =
this_cpu_ptr(sha1_mb_alg_state.alg_cstate);
 
@@ -539,7 +539,7 @@ static int sha1_mb_update(struct shash_desc *desc, const u8 
*data,
}
 
/* need to init context */
-   req_ctx_init(rctx, desc);
+   req_ctx_init(rctx, areq);
 
nbytes = crypto_ahash_walk_first(req, >walk);
 
@@ -552,7 +552,7 @@ static int sha1_mb_update(struct shash_desc *desc, const u8 
*data,
rctx->flag |= HASH_DONE;
 
/* submit */
-   sha_ctx = (struct sha1_hash_ctx *) shash_desc_ctx(desc);
+   sha_ctx = (struct sha1_hash_ctx *) ahash_request_ctx(areq);
sha1_mb_add_list(rctx, cstate);
kernel_fpu_begin();
sha_ctx = sha1_ctx_mgr_submit(cstate->mgr, sha_ctx, rctx->walk.data,
@@ -579,11 +579,10 @@ done:
return ret;
 }
 
-static int sha1_mb_finup(struct shash_desc *desc, const u8 *data,
-unsigned int len, u8 *out)
+static int sha1_mb_finup(struct ahash_request *areq)
 {
struct mcryptd_hash_request_ctx *rctx =
-   container_of(desc, struct mcryptd_hash_request_ctx, desc);
+   container_of(areq, struct