Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1271167 > unrolled thread

Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-17 14:10 +0100
    Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Tim Chen <tim.c.chen@linux.intel.com> - 2015-11-18 00:10 +0100
      Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-18 01:10 +0100
        Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Tim Chen <tim.c.chen@linux.intel.com> - 2015-11-18 01:40 +0100
          Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-18 06:10 +0100
            Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Tim Chen <tim.c.chen@linux.intel.com> - 2015-11-18 17:00 +0100
              Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-19 01:20 +0100
                Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Tim Chen <tim.c.chen@linux.intel.com> - 2015-11-19 03:40 +0100
                  Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure  support Herbert Xu <herbert@gondor.apana.org.au> - 2015-11-19 10:50 +0100

#1271167 — Re: [PATCH v2 1/5] crypto: Multi-buffer encryptioin infrastructure support

On Thu, Oct 29, 2015 at 03:21:03PM -0700, Tim Chen wrote:
> 
> c) Add support to crypto scatterwalk support that can sleep during
> encryption operation, as we may have buffers for jobs in data lanes
> that are half-finished, waiting for additional jobs to come to fill
> empty lanes before we start the encryption again.  Therefore, we need to
> enhance crypto walk with the option to map data buffers non-atomically.
> This is done by algorithms run from crypto daemon who knows it is safe
> to do so as it can save and restore FPU state in correct context.

What about the existing ablkcipher scatterwalk helpers?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1271725

On Tue, 2015-11-17 at 21:06 +0800, Herbert Xu wrote:
> On Thu, Oct 29, 2015 at 03:21:03PM -0700, Tim Chen wrote:
> > 
> > c) Add support to crypto scatterwalk support that can sleep during
> > encryption operation, as we may have buffers for jobs in data lanes
> > that are half-finished, waiting for additional jobs to come to fill
> > empty lanes before we start the encryption again.  Therefore, we need to
> > enhance crypto walk with the option to map data buffers non-atomically.
> > This is done by algorithms run from crypto daemon who knows it is safe
> > to do so as it can save and restore FPU state in correct context.
> 
> What about the existing ablkcipher scatterwalk helpers?
> 
> Cheers,

I suppose blkcipher was originally used because we 
were under the impression that there are less buffer copying
and less allocation of intermediate buffers
with blkcipher walk. 

But looking at the blkcipher walk and ablkcipher
walk code more carefully now, I am not sure that's really true
as it seems like ablkcipher keep all intermediate buffers till the
end and copy them to destination in one shot while blkcipher does
that at walk of every chunk.  The advantage of blkcipher is
you don't have as many outstanding buffers in a list.  If there's
really not much speed difference, I can try to use ablkcipher.

Herbert, would you prefer me to use ablkcipher scatter walk instead,
assuming the overhead of both walk are about the same?

Thanks.

Tim



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1271756

On Tue, Nov 17, 2015 at 02:59:29PM -0800, Tim Chen wrote:
>
> Herbert, would you prefer me to use ablkcipher scatter walk instead,
> assuming the overhead of both walk are about the same?

Well since you are going to potentially sleep in the middle of
an operation I'd think ablkcipher is required, no?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1271771

On Wed, 2015-11-18 at 08:07 +0800, Herbert Xu wrote:
> On Tue, Nov 17, 2015 at 02:59:29PM -0800, Tim Chen wrote:
> >
> > Herbert, would you prefer me to use ablkcipher scatter walk instead,
> > assuming the overhead of both walk are about the same?
> 
> Well since you are going to potentially sleep in the middle of
> an operation I'd think ablkcipher is required, no?

We're using blkcipher walk in the implementation. 
As long as we use kmap and instead of kmap_atomic,
it allows us to sleep in the middle of the walk.

Tim

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1271848

On Tue, Nov 17, 2015 at 04:30:14PM -0800, Tim Chen wrote:
> On Wed, 2015-11-18 at 08:07 +0800, Herbert Xu wrote:
> > On Tue, Nov 17, 2015 at 02:59:29PM -0800, Tim Chen wrote:
> > >
> > > Herbert, would you prefer me to use ablkcipher scatter walk instead,
> > > assuming the overhead of both walk are about the same?
> > 
> > Well since you are going to potentially sleep in the middle of
> > an operation I'd think ablkcipher is required, no?
> 
> We're using blkcipher walk in the implementation. 
> As long as we use kmap and instead of kmap_atomic,
> it allows us to sleep in the middle of the walk.

What if you were called from an atomic context, such as IPsec?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272320

On Wed, 2015-11-18 at 13:06 +0800, Herbert Xu wrote:
> On Tue, Nov 17, 2015 at 04:30:14PM -0800, Tim Chen wrote:
> > On Wed, 2015-11-18 at 08:07 +0800, Herbert Xu wrote:
> > > On Tue, Nov 17, 2015 at 02:59:29PM -0800, Tim Chen wrote:
> > > >
> > > > Herbert, would you prefer me to use ablkcipher scatter walk instead,
> > > > assuming the overhead of both walk are about the same?
> > > 
> > > Well since you are going to potentially sleep in the middle of
> > > an operation I'd think ablkcipher is required, no?
> > 
> > We're using blkcipher walk in the implementation. 
> > As long as we use kmap and instead of kmap_atomic,
> > it allows us to sleep in the middle of the walk.
> 
> What if you were called from an atomic context, such as IPsec?
> 

IPSec will invoke this multi-buffer encrypt with async request.
The work is done in crypto daemon, so it wouldn't be in atomic
context.  But anyway, I'm okay with switching to ablkcipher walk,
as long as it doesn't incur too much more overhead than blkcipher
walk.

Tim


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272702

On Wed, Nov 18, 2015 at 07:58:56AM -0800, Tim Chen wrote:
>
> IPSec will invoke this multi-buffer encrypt with async request.
> The work is done in crypto daemon, so it wouldn't be in atomic
> context.  But anyway, I'm okay with switching to ablkcipher walk,
> as long as it doesn't incur too much more overhead than blkcipher
> walk.

What if some other user called the blkcipher interface in an atomic
context? You can't guarantee that your algorithm is only picked up
through the ablkcipher interface, unless of course you do something
like __driver-ctr-aes-aesni.

Hmm I was just looking at the sha_mb code and something doesn't
look right.  For instance, can sha1_mb_update ever return
-EINPROGRESS? This would be wrong as it's registered as an shash
algorithm.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272755

On Thu, 2015-11-19 at 08:12 +0800, Herbert Xu wrote:
> On Wed, Nov 18, 2015 at 07:58:56AM -0800, Tim Chen wrote:
> >
> > IPSec will invoke this multi-buffer encrypt with async request.
> > The work is done in crypto daemon, so it wouldn't be in atomic
> > context.  But anyway, I'm okay with switching to ablkcipher walk,
> > as long as it doesn't incur too much more overhead than blkcipher
> > walk.
> 
> What if some other user called the blkcipher interface in an atomic
> context? You can't guarantee that your algorithm is only picked up
> through the ablkcipher interface, unless of course you do something
> like __driver-ctr-aes-aesni.

The __cbc-aes-aesni-mb algorithm is marked as internal algorithm 
with flag CRYPTO_ALG_INTERNAL, so it should not be picked up by other
algorithms and should only be invoked from mcryptd.

Anyway, I've udpated the aes_cbc_mb code with ablkcipher helper.
So I will be posting the new series with ablkcipher walk
after testing is done.

> 
> Hmm I was just looking at the sha_mb code and something doesn't
> look right.  For instance, can sha1_mb_update ever return
> -EINPROGRESS? This would be wrong as it's registered as an shash
> algorithm.

The __sha1-mb works in tandem with the outer layer of mcryptd 
aysnc algorithm. It does the completion for the outer 
async algorithm.  So as far as mcryptd is concerned, the
inner algorithm is synchronous in the sense that it is done
once it dispatch the job to __sha1-mb and don't have to worry about it.
I don't think mcryptd check for the return value from __sha1-mb
so it should be okay to return 0 instead of -EINPROGRESS.  
I'll double check that.

Tim

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1272973

On Wed, Nov 18, 2015 at 06:39:30PM -0800, Tim Chen wrote:
>
> The __cbc-aes-aesni-mb algorithm is marked as internal algorithm 
> with flag CRYPTO_ALG_INTERNAL, so it should not be picked up by other
> algorithms and should only be invoked from mcryptd.

OK I guess that's fine then.

> Anyway, I've udpated the aes_cbc_mb code with ablkcipher helper.
> So I will be posting the new series with ablkcipher walk
> after testing is done.

Yes I think what you have is a very special case.  As you said
the ablkcipher interface should be fairly low in overhead so I
think it makes sense to use that instead of blkcipher.

> The __sha1-mb works in tandem with the outer layer of mcryptd 
> aysnc algorithm. It does the completion for the outer 
> async algorithm.  So as far as mcryptd is concerned, the
> inner algorithm is synchronous in the sense that it is done
> once it dispatch the job to __sha1-mb and don't have to worry about it.
> I don't think mcryptd check for the return value from __sha1-mb
> so it should be okay to return 0 instead of -EINPROGRESS.  
> I'll double check that.

If it can never return EINPROGRESS then we should probably remove
the code in it that says "return -EINPROGRESS".

Thanks,
-- 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web