Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Milan Broz]

On 04/18/2016 03:36 PM, Mike Snitzer wrote:
> On Mon, Apr 18 2016 at  1:31am -0400,
> Baolin Wang  wrote:
> 
>> Hi Herbert,
>>
>> On 15 April 2016 at 21:48, Herbert Xu  wrote:
>>> On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
 Now some cipher hardware engines prefer to handle bulk block by merging 
 requests
 to increase the block size and thus increase the hardware engine 
 processing speed.

 This patchset introduces request bulk mode to help the crypto hardware 
 drivers
 improve in efficiency.
>>>
>>> Could you please explain why this merging can't be done in dm-crypt
>>> instead?
>>
>> We've tried to do this in dm-crypt, but it failed.
>> The dm-crypt maintainer explained to me that I should optimize the
>> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
>> not the first crypto accelerator that is just not suited for this kind
>> of use.
> 
> As a DM mainatiner my only contribution to this line of discussion was
> relative to your proposal to train the dm-crypt target (which is
> bio-based) to also provide request-based support, see:
> https://www.redhat.com/archives/dm-devel/2015-November/msg00112.html
> 
> But follow-up discussion occured, primarily with Milan Broz, which led
> to this bulk mode support in the crypto layer.  Pretty strange Milan
> wasn't cc'd on your patchset posting (I've now cc'd him).

My complaint was mainly that the proposed dmcrypt based version just did
not work properly.
https://lkml.org/lkml/2016/1/2/109

(I did not test the new version we are replying here. I wonder how the problem
I mentioned is fixed though.
Also see Mikulas' comments 
https://www.redhat.com/archives/dm-devel/2016-January/msg00145.html)
 
Anyway, all this seems to optimize case for specific crypto hw, that
is not able to perform optimally with pattern dmcrypt produces.

I do not think dmcrypt should do optimizations for specific hw.

But if we decide that it is needed there, it should not cause any performance
or compatibility problems elsewhere...

Milan

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Milan Broz]

On 04/18/2016 03:36 PM, Mike Snitzer wrote:
> On Mon, Apr 18 2016 at  1:31am -0400,
> Baolin Wang  wrote:
> 
>> Hi Herbert,
>>
>> On 15 April 2016 at 21:48, Herbert Xu  wrote:
>>> On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
 Now some cipher hardware engines prefer to handle bulk block by merging 
 requests
 to increase the block size and thus increase the hardware engine 
 processing speed.

 This patchset introduces request bulk mode to help the crypto hardware 
 drivers
 improve in efficiency.
>>>
>>> Could you please explain why this merging can't be done in dm-crypt
>>> instead?
>>
>> We've tried to do this in dm-crypt, but it failed.
>> The dm-crypt maintainer explained to me that I should optimize the
>> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
>> not the first crypto accelerator that is just not suited for this kind
>> of use.
> 
> As a DM mainatiner my only contribution to this line of discussion was
> relative to your proposal to train the dm-crypt target (which is
> bio-based) to also provide request-based support, see:
> https://www.redhat.com/archives/dm-devel/2015-November/msg00112.html
> 
> But follow-up discussion occured, primarily with Milan Broz, which led
> to this bulk mode support in the crypto layer.  Pretty strange Milan
> wasn't cc'd on your patchset posting (I've now cc'd him).

My complaint was mainly that the proposed dmcrypt based version just did
not work properly.
https://lkml.org/lkml/2016/1/2/109

(I did not test the new version we are replying here. I wonder how the problem
I mentioned is fixed though.
Also see Mikulas' comments 
https://www.redhat.com/archives/dm-devel/2016-January/msg00145.html)
 
Anyway, all this seems to optimize case for specific crypto hw, that
is not able to perform optimally with pattern dmcrypt produces.

I do not think dmcrypt should do optimizations for specific hw.

But if we decide that it is needed there, it should not cause any performance
or compatibility problems elsewhere...

Milan

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Mike Snitzer]

On Mon, Apr 18 2016 at  1:31am -0400,
Baolin Wang  wrote:

> Hi Herbert,
> 
> On 15 April 2016 at 21:48, Herbert Xu  wrote:
> > On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
> >> Now some cipher hardware engines prefer to handle bulk block by merging 
> >> requests
> >> to increase the block size and thus increase the hardware engine 
> >> processing speed.
> >>
> >> This patchset introduces request bulk mode to help the crypto hardware 
> >> drivers
> >> improve in efficiency.
> >
> > Could you please explain why this merging can't be done in dm-crypt
> > instead?
> 
> We've tried to do this in dm-crypt, but it failed.
> The dm-crypt maintainer explained to me that I should optimize the
> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
> not the first crypto accelerator that is just not suited for this kind
> of use.

As a DM mainatiner my only contribution to this line of discussion was
relative to your proposal to train the dm-crypt target (which is
bio-based) to also provide request-based support, see:
https://www.redhat.com/archives/dm-devel/2015-November/msg00112.html

But follow-up discussion occured, primarily with Milan Broz, which led
to this bulk mode support in the crypto layer.  Pretty strange Milan
wasn't cc'd on your patchset posting (I've now cc'd him).

Mike

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Mike Snitzer]

On Mon, Apr 18 2016 at  1:31am -0400,
Baolin Wang  wrote:

> Hi Herbert,
> 
> On 15 April 2016 at 21:48, Herbert Xu  wrote:
> > On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
> >> Now some cipher hardware engines prefer to handle bulk block by merging 
> >> requests
> >> to increase the block size and thus increase the hardware engine 
> >> processing speed.
> >>
> >> This patchset introduces request bulk mode to help the crypto hardware 
> >> drivers
> >> improve in efficiency.
> >
> > Could you please explain why this merging can't be done in dm-crypt
> > instead?
> 
> We've tried to do this in dm-crypt, but it failed.
> The dm-crypt maintainer explained to me that I should optimize the
> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
> not the first crypto accelerator that is just not suited for this kind
> of use.

As a DM mainatiner my only contribution to this line of discussion was
relative to your proposal to train the dm-crypt target (which is
bio-based) to also provide request-based support, see:
https://www.redhat.com/archives/dm-devel/2015-November/msg00112.html

But follow-up discussion occured, primarily with Milan Broz, which led
to this bulk mode support in the crypto layer.  Pretty strange Milan
wasn't cc'd on your patchset posting (I've now cc'd him).

Mike

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:41, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:40:36PM +0800, Baolin Wang wrote:
>>
>> Simply to say, now there are many different hardware engines for
>> different vendors, some engines can support bulk block but some can
>> not (or no cipher hardware engine), then the dm-crypt can not know
>> your hardware engine features. If the dm-crypt send one bulk block to
>> low level, but the engine driver can not support bulk block, then it
>> will crash. So we did the merging action in driver level not dm-crypt
>> level.
>
> Surely we can handle it in the crypto API layer, just as we do GSO
> in the network stack for drivers that can't handle TSO?

Yes, it is similar. Thanks.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:41, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:40:36PM +0800, Baolin Wang wrote:
>>
>> Simply to say, now there are many different hardware engines for
>> different vendors, some engines can support bulk block but some can
>> not (or no cipher hardware engine), then the dm-crypt can not know
>> your hardware engine features. If the dm-crypt send one bulk block to
>> low level, but the engine driver can not support bulk block, then it
>> will crash. So we did the merging action in driver level not dm-crypt
>> level.
>
> Surely we can handle it in the crypto API layer, just as we do GSO
> in the network stack for drivers that can't handle TSO?

Yes, it is similar. Thanks.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:40:36PM +0800, Baolin Wang wrote:
>
> Simply to say, now there are many different hardware engines for
> different vendors, some engines can support bulk block but some can
> not (or no cipher hardware engine), then the dm-crypt can not know
> your hardware engine features. If the dm-crypt send one bulk block to
> low level, but the engine driver can not support bulk block, then it
> will crash. So we did the merging action in driver level not dm-crypt
> level.

Surely we can handle it in the crypto API layer, just as we do GSO
in the network stack for drivers that can't handle TSO?

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:40:36PM +0800, Baolin Wang wrote:
>
> Simply to say, now there are many different hardware engines for
> different vendors, some engines can support bulk block but some can
> not (or no cipher hardware engine), then the dm-crypt can not know
> your hardware engine features. If the dm-crypt send one bulk block to
> low level, but the engine driver can not support bulk block, then it
> will crash. So we did the merging action in driver level not dm-crypt
> level.

Surely we can handle it in the crypto API layer, just as we do GSO
in the network stack for drivers that can't handle TSO?

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:31, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:28:46PM +0800, Baolin Wang wrote:
>>
>> What I meaning is if the xts engine can support bulk block, then the
>> engine driver can select bulk mode to do encryption, but if their xts
>> engine can not support bulk mode, which depends on hardware design,
>> the engine driver can not select bulk mode. So the dm-crypt can not
>> know what will be selected by the engine driver, it can not send one
>> bulk block each time.
>
> Why can't the xts code just break it up if it can't handle it?

Simply to say, now there are many different hardware engines for
different vendors, some engines can support bulk block but some can
not (or no cipher hardware engine), then the dm-crypt can not know
your hardware engine features. If the dm-crypt send one bulk block to
low level, but the engine driver can not support bulk block, then it
will crash. So we did the merging action in driver level not dm-crypt
level.

>
> You want to postpone splitting as much as possible.  Even if the
> underlying xts code couldn't handle it, it would still make sense
> for the crypto API to see the request in one piece.
>
> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:31, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:28:46PM +0800, Baolin Wang wrote:
>>
>> What I meaning is if the xts engine can support bulk block, then the
>> engine driver can select bulk mode to do encryption, but if their xts
>> engine can not support bulk mode, which depends on hardware design,
>> the engine driver can not select bulk mode. So the dm-crypt can not
>> know what will be selected by the engine driver, it can not send one
>> bulk block each time.
>
> Why can't the xts code just break it up if it can't handle it?

Simply to say, now there are many different hardware engines for
different vendors, some engines can support bulk block but some can
not (or no cipher hardware engine), then the dm-crypt can not know
your hardware engine features. If the dm-crypt send one bulk block to
low level, but the engine driver can not support bulk block, then it
will crash. So we did the merging action in driver level not dm-crypt
level.

>
> You want to postpone splitting as much as possible.  Even if the
> underlying xts code couldn't handle it, it would still make sense
> for the crypto API to see the request in one piece.
>
> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:28:46PM +0800, Baolin Wang wrote:
> 
> What I meaning is if the xts engine can support bulk block, then the
> engine driver can select bulk mode to do encryption, but if their xts
> engine can not support bulk mode, which depends on hardware design,
> the engine driver can not select bulk mode. So the dm-crypt can not
> know what will be selected by the engine driver, it can not send one
> bulk block each time.

Why can't the xts code just break it up if it can't handle it?

You want to postpone splitting as much as possible.  Even if the
underlying xts code couldn't handle it, it would still make sense
for the crypto API to see the request in one piece.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:28:46PM +0800, Baolin Wang wrote:
> 
> What I meaning is if the xts engine can support bulk block, then the
> engine driver can select bulk mode to do encryption, but if their xts
> engine can not support bulk mode, which depends on hardware design,
> the engine driver can not select bulk mode. So the dm-crypt can not
> know what will be selected by the engine driver, it can not send one
> bulk block each time.

Why can't the xts code just break it up if it can't handle it?

You want to postpone splitting as much as possible.  Even if the
underlying xts code couldn't handle it, it would still make sense
for the crypto API to see the request in one piece.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:17, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:14:48PM +0800, Baolin Wang wrote:
>> On 18 April 2016 at 16:04, Herbert Xu  wrote:
>> > On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>> >>
>> >> That depends on the hardware engine. Some cipher hardware engines
>> >> (like xts(aes) engine) can handle the intermediate values (IV) by
>> >> themselves in one bulk block, which means we can increase the size of
>> >> the request by merging request rather than always 512 bytes and thus
>> >> increase the hardware engine processing speed. But for some other
>> >> hardware engines (like cbc(aes) engine), they can not support bulk
>> >> block, must sector by sector. So the engine drivers can select the
>> >> suitable mode to do encryption/decryption.
>> >
>> > So what is this supposed to handle, xts or cbc?
>>
>> As I know, now cbc engine also need to handle requests sector by
>> sector, but for xts/ecb engine can support bulk block, which means can
>> merge requests.
>
> If it's just xts then why can't dm-crypt merge it and send a single
> request?

What I meaning is if the xts engine can support bulk block, then the
engine driver can select bulk mode to do encryption, but if their xts
engine can not support bulk mode, which depends on hardware design,
the engine driver can not select bulk mode. So the dm-crypt can not
know what will be selected by the engine driver, it can not send one
bulk block each time.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:17, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 04:14:48PM +0800, Baolin Wang wrote:
>> On 18 April 2016 at 16:04, Herbert Xu  wrote:
>> > On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>> >>
>> >> That depends on the hardware engine. Some cipher hardware engines
>> >> (like xts(aes) engine) can handle the intermediate values (IV) by
>> >> themselves in one bulk block, which means we can increase the size of
>> >> the request by merging request rather than always 512 bytes and thus
>> >> increase the hardware engine processing speed. But for some other
>> >> hardware engines (like cbc(aes) engine), they can not support bulk
>> >> block, must sector by sector. So the engine drivers can select the
>> >> suitable mode to do encryption/decryption.
>> >
>> > So what is this supposed to handle, xts or cbc?
>>
>> As I know, now cbc engine also need to handle requests sector by
>> sector, but for xts/ecb engine can support bulk block, which means can
>> merge requests.
>
> If it's just xts then why can't dm-crypt merge it and send a single
> request?

What I meaning is if the xts engine can support bulk block, then the
engine driver can select bulk mode to do encryption, but if their xts
engine can not support bulk mode, which depends on hardware design,
the engine driver can not select bulk mode. So the dm-crypt can not
know what will be selected by the engine driver, it can not send one
bulk block each time.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:14:48PM +0800, Baolin Wang wrote:
> On 18 April 2016 at 16:04, Herbert Xu  wrote:
> > On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
> >>
> >> That depends on the hardware engine. Some cipher hardware engines
> >> (like xts(aes) engine) can handle the intermediate values (IV) by
> >> themselves in one bulk block, which means we can increase the size of
> >> the request by merging request rather than always 512 bytes and thus
> >> increase the hardware engine processing speed. But for some other
> >> hardware engines (like cbc(aes) engine), they can not support bulk
> >> block, must sector by sector. So the engine drivers can select the
> >> suitable mode to do encryption/decryption.
> >
> > So what is this supposed to handle, xts or cbc?
> 
> As I know, now cbc engine also need to handle requests sector by
> sector, but for xts/ecb engine can support bulk block, which means can
> merge requests.

If it's just xts then why can't dm-crypt merge it and send a single
request?

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 04:14:48PM +0800, Baolin Wang wrote:
> On 18 April 2016 at 16:04, Herbert Xu  wrote:
> > On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
> >>
> >> That depends on the hardware engine. Some cipher hardware engines
> >> (like xts(aes) engine) can handle the intermediate values (IV) by
> >> themselves in one bulk block, which means we can increase the size of
> >> the request by merging request rather than always 512 bytes and thus
> >> increase the hardware engine processing speed. But for some other
> >> hardware engines (like cbc(aes) engine), they can not support bulk
> >> block, must sector by sector. So the engine drivers can select the
> >> suitable mode to do encryption/decryption.
> >
> > So what is this supposed to handle, xts or cbc?
> 
> As I know, now cbc engine also need to handle requests sector by
> sector, but for xts/ecb engine can support bulk block, which means can
> merge requests.

If it's just xts then why can't dm-crypt merge it and send a single
request?

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:04, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>>
>> That depends on the hardware engine. Some cipher hardware engines
>> (like xts(aes) engine) can handle the intermediate values (IV) by
>> themselves in one bulk block, which means we can increase the size of
>> the request by merging request rather than always 512 bytes and thus
>> increase the hardware engine processing speed. But for some other
>> hardware engines (like cbc(aes) engine), they can not support bulk
>> block, must sector by sector. So the engine drivers can select the
>> suitable mode to do encryption/decryption.
>
> So what is this supposed to handle, xts or cbc?

As I know, now cbc engine also need to handle requests sector by
sector, but for xts/ecb engine can support bulk block, which means can
merge requests.

>
>> > Even with batching we should be involving the user because only the
>> > user knows (if anyone does) whether more data will be forthcoming.
>>
>> If this cipher engine can support bulk block encryption, the crypto
>> engine framework can merge requests if they are eligible
>> automatically. Don't need to worry about how many data will be
>> forthcoming.
>
> Merging is simply wrong when the data is coming in as one piece
> and you've just artifically broken it up, only to merge it later.

It will not broke it up,  and it will check if the requests coming
from dm-crypt can be merged together.

>
> If the data can be merged then it should have stayed as one piece
> rather than being fragmented.

Yes, usually one whole block can be merged into one request as the latency.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 16:04, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>>
>> That depends on the hardware engine. Some cipher hardware engines
>> (like xts(aes) engine) can handle the intermediate values (IV) by
>> themselves in one bulk block, which means we can increase the size of
>> the request by merging request rather than always 512 bytes and thus
>> increase the hardware engine processing speed. But for some other
>> hardware engines (like cbc(aes) engine), they can not support bulk
>> block, must sector by sector. So the engine drivers can select the
>> suitable mode to do encryption/decryption.
>
> So what is this supposed to handle, xts or cbc?

As I know, now cbc engine also need to handle requests sector by
sector, but for xts/ecb engine can support bulk block, which means can
merge requests.

>
>> > Even with batching we should be involving the user because only the
>> > user knows (if anyone does) whether more data will be forthcoming.
>>
>> If this cipher engine can support bulk block encryption, the crypto
>> engine framework can merge requests if they are eligible
>> automatically. Don't need to worry about how many data will be
>> forthcoming.
>
> Merging is simply wrong when the data is coming in as one piece
> and you've just artifically broken it up, only to merge it later.

It will not broke it up,  and it will check if the requests coming
from dm-crypt can be merged together.

>
> If the data can be merged then it should have stayed as one piece
> rather than being fragmented.

Yes, usually one whole block can be merged into one request as the latency.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>
> That depends on the hardware engine. Some cipher hardware engines
> (like xts(aes) engine) can handle the intermediate values (IV) by
> themselves in one bulk block, which means we can increase the size of
> the request by merging request rather than always 512 bytes and thus
> increase the hardware engine processing speed. But for some other
> hardware engines (like cbc(aes) engine), they can not support bulk
> block, must sector by sector. So the engine drivers can select the
> suitable mode to do encryption/decryption.

So what is this supposed to handle, xts or cbc?

> > Even with batching we should be involving the user because only the
> > user knows (if anyone does) whether more data will be forthcoming.
> 
> If this cipher engine can support bulk block encryption, the crypto
> engine framework can merge requests if they are eligible
> automatically. Don't need to worry about how many data will be
> forthcoming.

Merging is simply wrong when the data is coming in as one piece
and you've just artifically broken it up, only to merge it later.

If the data can be merged then it should have stayed as one piece
rather than being fragmented.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 03:58:59PM +0800, Baolin Wang wrote:
>
> That depends on the hardware engine. Some cipher hardware engines
> (like xts(aes) engine) can handle the intermediate values (IV) by
> themselves in one bulk block, which means we can increase the size of
> the request by merging request rather than always 512 bytes and thus
> increase the hardware engine processing speed. But for some other
> hardware engines (like cbc(aes) engine), they can not support bulk
> block, must sector by sector. So the engine drivers can select the
> suitable mode to do encryption/decryption.

So what is this supposed to handle, xts or cbc?

> > Even with batching we should be involving the user because only the
> > user knows (if anyone does) whether more data will be forthcoming.
> 
> If this cipher engine can support bulk block encryption, the crypto
> engine framework can merge requests if they are eligible
> automatically. Don't need to worry about how many data will be
> forthcoming.

Merging is simply wrong when the data is coming in as one piece
and you've just artifically broken it up, only to merge it later.

If the data can be merged then it should have stayed as one piece
rather than being fragmented.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 15:24, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 03:21:16PM +0800, Baolin Wang wrote:
>>
>> I don't think so, the dm-crypt can not send maximal requests at some
>> situations. For example, the 'cbc(aes)' cipher, it must be handled
>> sector by sector (IV is dependency for each sector), so the dm-crypt
>> can not send maximal requests, but must sector by sector in this
>> situation.
>
> If you can't merge them as requests, then how is the hardware going
> to benefit from batching them? Or are you talking about parallel

That depends on the hardware engine. Some cipher hardware engines
(like xts(aes) engine) can handle the intermediate values (IV) by
themselves in one bulk block, which means we can increase the size of
the request by merging request rather than always 512 bytes and thus
increase the hardware engine processing speed. But for some other
hardware engines (like cbc(aes) engine), they can not support bulk
block, must sector by sector. So the engine drivers can select the
suitable mode to do encryption/decryption.

> processing similar to sha-mb?
>
> Even with batching we should be involving the user because only the
> user knows (if anyone does) whether more data will be forthcoming.

If this cipher engine can support bulk block encryption, the crypto
engine framework can merge requests if they are eligible
automatically. Don't need to worry about how many data will be
forthcoming.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 15:24, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 03:21:16PM +0800, Baolin Wang wrote:
>>
>> I don't think so, the dm-crypt can not send maximal requests at some
>> situations. For example, the 'cbc(aes)' cipher, it must be handled
>> sector by sector (IV is dependency for each sector), so the dm-crypt
>> can not send maximal requests, but must sector by sector in this
>> situation.
>
> If you can't merge them as requests, then how is the hardware going
> to benefit from batching them? Or are you talking about parallel

That depends on the hardware engine. Some cipher hardware engines
(like xts(aes) engine) can handle the intermediate values (IV) by
themselves in one bulk block, which means we can increase the size of
the request by merging request rather than always 512 bytes and thus
increase the hardware engine processing speed. But for some other
hardware engines (like cbc(aes) engine), they can not support bulk
block, must sector by sector. So the engine drivers can select the
suitable mode to do encryption/decryption.

> processing similar to sha-mb?
>
> Even with batching we should be involving the user because only the
> user knows (if anyone does) whether more data will be forthcoming.

If this cipher engine can support bulk block encryption, the crypto
engine framework can merge requests if they are eligible
automatically. Don't need to worry about how many data will be
forthcoming.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 03:21:16PM +0800, Baolin Wang wrote:
>
> I don't think so, the dm-crypt can not send maximal requests at some
> situations. For example, the 'cbc(aes)' cipher, it must be handled
> sector by sector (IV is dependency for each sector), so the dm-crypt
> can not send maximal requests, but must sector by sector in this
> situation.

If you can't merge them as requests, then how is the hardware going
to benefit from batching them? Or are you talking about parallel
processing similar to sha-mb?

Even with batching we should be involving the user because only the
user knows (if anyone does) whether more data will be forthcoming.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 03:21:16PM +0800, Baolin Wang wrote:
>
> I don't think so, the dm-crypt can not send maximal requests at some
> situations. For example, the 'cbc(aes)' cipher, it must be handled
> sector by sector (IV is dependency for each sector), so the dm-crypt
> can not send maximal requests, but must sector by sector in this
> situation.

If you can't merge them as requests, then how is the hardware going
to benefit from batching them? Or are you talking about parallel
processing similar to sha-mb?

Even with batching we should be involving the user because only the
user knows (if anyone does) whether more data will be forthcoming.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 15:04, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 02:02:51PM +0800, Baolin Wang wrote:
>>
>> If the crypto hardware engine can support bulk data
>> encryption/decryption, so the engine driver can select bulk mode to
>> handle the requests. I think it is a totally driver things, not in
>> dmcrypt. The dmcrypt can not get the hardware engine's attributes.
>
> It has nothing to do with the hardware attributes.  dm-crypt should
> be sending maximal requests in the first place.

I don't think so, the dm-crypt can not send maximal requests at some
situations. For example, the 'cbc(aes)' cipher, it must be handled
sector by sector (IV is dependency for each sector), so the dm-crypt
can not send maximal requests, but must sector by sector in this
situation.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 15:04, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 02:02:51PM +0800, Baolin Wang wrote:
>>
>> If the crypto hardware engine can support bulk data
>> encryption/decryption, so the engine driver can select bulk mode to
>> handle the requests. I think it is a totally driver things, not in
>> dmcrypt. The dmcrypt can not get the hardware engine's attributes.
>
> It has nothing to do with the hardware attributes.  dm-crypt should
> be sending maximal requests in the first place.

I don't think so, the dm-crypt can not send maximal requests at some
situations. For example, the 'cbc(aes)' cipher, it must be handled
sector by sector (IV is dependency for each sector), so the dm-crypt
can not send maximal requests, but must sector by sector in this
situation.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 02:02:51PM +0800, Baolin Wang wrote:
> 
> If the crypto hardware engine can support bulk data
> encryption/decryption, so the engine driver can select bulk mode to
> handle the requests. I think it is a totally driver things, not in
> dmcrypt. The dmcrypt can not get the hardware engine's attributes.

It has nothing to do with the hardware attributes.  dm-crypt should
be sending maximal requests in the first place.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 02:02:51PM +0800, Baolin Wang wrote:
> 
> If the crypto hardware engine can support bulk data
> encryption/decryption, so the engine driver can select bulk mode to
> handle the requests. I think it is a totally driver things, not in
> dmcrypt. The dmcrypt can not get the hardware engine's attributes.

It has nothing to do with the hardware attributes.  dm-crypt should
be sending maximal requests in the first place.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 13:45, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 01:31:09PM +0800, Baolin Wang wrote:
>>
>> We've tried to do this in dm-crypt, but it failed.
>> The dm-crypt maintainer explained to me that I should optimize the
>> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
>> not the first crypto accelerator that is just not suited for this kind
>> of use.
>> He thought if it can process batch of chunks of data each with own IV,
>> then it can work with dm-crypt, but he thought such optimized code
>> should be inside crypto API, not in dmcrypt.
>
> That's a completely bogus argument.  The user always has more
> information available than the underlying API.  So it is totally
> stupid to have the API try to extract information that the user
> could have provided in the first place.

If the crypto hardware engine can support bulk data
encryption/decryption, so the engine driver can select bulk mode to
handle the requests. I think it is a totally driver things, not in
dmcrypt. The dmcrypt can not get the hardware engine's attributes.

>
> I'm not taking this patch-set.
>
> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

On 18 April 2016 at 13:45, Herbert Xu  wrote:
> On Mon, Apr 18, 2016 at 01:31:09PM +0800, Baolin Wang wrote:
>>
>> We've tried to do this in dm-crypt, but it failed.
>> The dm-crypt maintainer explained to me that I should optimize the
>> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
>> not the first crypto accelerator that is just not suited for this kind
>> of use.
>> He thought if it can process batch of chunks of data each with own IV,
>> then it can work with dm-crypt, but he thought such optimized code
>> should be inside crypto API, not in dmcrypt.
>
> That's a completely bogus argument.  The user always has more
> information available than the underlying API.  So it is totally
> stupid to have the API try to extract information that the user
> could have provided in the first place.

If the crypto hardware engine can support bulk data
encryption/decryption, so the engine driver can select bulk mode to
handle the requests. I think it is a totally driver things, not in
dmcrypt. The dmcrypt can not get the hardware engine's attributes.

>
> I'm not taking this patch-set.
>
> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 01:31:09PM +0800, Baolin Wang wrote:
> 
> We've tried to do this in dm-crypt, but it failed.
> The dm-crypt maintainer explained to me that I should optimize the
> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
> not the first crypto accelerator that is just not suited for this kind
> of use.
> He thought if it can process batch of chunks of data each with own IV,
> then it can work with dm-crypt, but he thought such optimized code
> should be inside crypto API, not in dmcrypt.

That's a completely bogus argument.  The user always has more
information available than the underlying API.  So it is totally
stupid to have the API try to extract information that the user
could have provided in the first place.

I'm not taking this patch-set.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Mon, Apr 18, 2016 at 01:31:09PM +0800, Baolin Wang wrote:
> 
> We've tried to do this in dm-crypt, but it failed.
> The dm-crypt maintainer explained to me that I should optimize the
> driver, not add strange hw-dependent crypto modes to dm-crypt, this is
> not the first crypto accelerator that is just not suited for this kind
> of use.
> He thought if it can process batch of chunks of data each with own IV,
> then it can work with dm-crypt, but he thought such optimized code
> should be inside crypto API, not in dmcrypt.

That's a completely bogus argument.  The user always has more
information available than the underlying API.  So it is totally
stupid to have the API try to extract information that the user
could have provided in the first place.

I'm not taking this patch-set.

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

Hi Herbert,

On 15 April 2016 at 21:48, Herbert Xu  wrote:
> On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
>> Now some cipher hardware engines prefer to handle bulk block by merging 
>> requests
>> to increase the block size and thus increase the hardware engine processing 
>> speed.
>>
>> This patchset introduces request bulk mode to help the crypto hardware 
>> drivers
>> improve in efficiency.
>
> Could you please explain why this merging can't be done in dm-crypt
> instead?

We've tried to do this in dm-crypt, but it failed.
The dm-crypt maintainer explained to me that I should optimize the
driver, not add strange hw-dependent crypto modes to dm-crypt, this is
not the first crypto accelerator that is just not suited for this kind
of use.
He thought if it can process batch of chunks of data each with own IV,
then it can work with dm-crypt, but he thought such optimized code
should be inside crypto API, not in dmcrypt.

I think his suggestion is reasonable, so we introduce the crypto
engine framework to factor out the common patterns for driving the
queue of operations. Then it will be more reasonable to do the bulk
mode optimization in crypto engine framework. Thanks.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Baolin Wang]

Hi Herbert,

On 15 April 2016 at 21:48, Herbert Xu  wrote:
> On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
>> Now some cipher hardware engines prefer to handle bulk block by merging 
>> requests
>> to increase the block size and thus increase the hardware engine processing 
>> speed.
>>
>> This patchset introduces request bulk mode to help the crypto hardware 
>> drivers
>> improve in efficiency.
>
> Could you please explain why this merging can't be done in dm-crypt
> instead?

We've tried to do this in dm-crypt, but it failed.
The dm-crypt maintainer explained to me that I should optimize the
driver, not add strange hw-dependent crypto modes to dm-crypt, this is
not the first crypto accelerator that is just not suited for this kind
of use.
He thought if it can process batch of chunks of data each with own IV,
then it can work with dm-crypt, but he thought such optimized code
should be inside crypto API, not in dmcrypt.

I think his suggestion is reasonable, so we introduce the crypto
engine framework to factor out the common patterns for driving the
queue of operations. Then it will be more reasonable to do the bulk
mode optimization in crypto engine framework. Thanks.

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



-- 
Baolin.wang
Best Regards

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
> Now some cipher hardware engines prefer to handle bulk block by merging 
> requests
> to increase the block size and thus increase the hardware engine processing 
> speed.
> 
> This patchset introduces request bulk mode to help the crypto hardware drivers
> improve in efficiency.

Could you please explain why this merging can't be done in dm-crypt
instead?

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

Re: [PATCH v2 0/4] Introduce bulk mode for crypto engine framework

[Herbert Xu]

On Tue, Mar 15, 2016 at 03:47:58PM +0800, Baolin Wang wrote:
> Now some cipher hardware engines prefer to handle bulk block by merging 
> requests
> to increase the block size and thus increase the hardware engine processing 
> speed.
> 
> This patchset introduces request bulk mode to help the crypto hardware drivers
> improve in efficiency.

Could you please explain why this merging can't be done in dm-crypt
instead?

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