2011-01-25 13:44:52

by Dmitry Kasatkin

[permalink] [raw]
Subject: hash finup() issue

Hello,

It is often very important to know for implementation or optimization if
we get more data to process or not.

New user space crypto API uses socket MSG_MORE flag to know if more data
is coming.

>From kernel crypto API it is also may be very important to know if more
data is coming.

OMAP SHA1 accelerator is designed in such a way that it requires to
submit last request (data) with special control bit
CLOSE_HASH.

Using OLD crypto API driver cannot know it until it get hash_final() call...

hash_init()
hash_update()
...
hash_update()
hash_final()

So basically on every hash_update() call driver must store a tail =
(data_len % block_len)
So basically that tail would be handled from hash_final() call...
For example if we have update call with size of one page 4096, then we
need to hash only 4032 bytes and leave 64 bytes
to hash in final() call...
It makes DMA impossible or inefficient, because on next update calls we
need to fill the buffer by copying data and then run DMA.
So extra copying is involved.

Similar case is also when we also use new crypto API and hash_finup()
with current implementation....
hash_init()
hash_update()
...
hash_finup()


Basically what is important is that driver on hash_update() call would
know that more data is coming.
If consider that client will use hash_finup() for last update then
driver could consider that hash_update() is not a final update and more
data will come (until hash_finup()).
But that assumption will break old API to work....

For that reason I guess it would be great to have some way to tell the
driver if we use old or new API - I mean
update+final or finup...

What we have done in our system is introduced a new flag which is set to
request.
flags |= CRYPTO_TFM_REQ_USE_FINUP;
ahash_request_set_callback(req, flags, tcrypt_complete, &tresult);

Then the driver has just one extra check

if (!tail && !(dd->req->base.flags & CRYPTO_TFM_REQ_USE_FINUP))
tail = SHA1_MD5_BLOCK_SIZE;

Basically when hashing a page 4096, it is goes via DMA without extra
copying...


What do you think about it?
Some other ways to do the same?

- Dmitry


2011-01-25 23:29:20

by Herbert Xu

[permalink] [raw]
Subject: Re: hash finup() issue

On Tue, Jan 25, 2011 at 03:44:47PM +0200, Dmitry Kasatkin wrote:
>
> What we have done in our system is introduced a new flag which is set to
> request.
> flags |= CRYPTO_TFM_REQ_USE_FINUP;
> ahash_request_set_callback(req, flags, tcrypt_complete, &tresult);

We don't need a flag for this, we just need to optimise the
user-interface code to actually use finup when MSG_MORE is not
set.

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

2011-01-26 07:48:23

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: hash finup() issue

I am not talking about user space API at all.

I talk about kernel crypto API and kernel clients.

Please understand the following: when update() is called driver does not
know if it is last update or not...
That is essential.
If client code would always use update/finup then it is fine..
But original API and clients uses update/final...
That is why some way (flag) needed to tell if finup() will be used or not...

It gave up 20% performance improvement in some case because no extra
memcpy has been done...

If you think about other solution, please share...

Have a nice day,

- Dmitry


On 26/01/11 01:29, ext Herbert Xu wrote:
> On Tue, Jan 25, 2011 at 03:44:47PM +0200, Dmitry Kasatkin wrote:
>> What we have done in our system is introduced a new flag which is set to
>> request.
>> flags |= CRYPTO_TFM_REQ_USE_FINUP;
>> ahash_request_set_callback(req, flags, tcrypt_complete, &tresult);
> We don't need a flag for this, we just need to optimise the
> user-interface code to actually use finup when MSG_MORE is not
> set.
>
> Cheers,

2011-01-26 23:32:22

by Herbert Xu

[permalink] [raw]
Subject: Re: hash finup() issue

On Wed, Jan 26, 2011 at 09:47:27AM +0200, Dmitry Kasatkin wrote:
> I am not talking about user space API at all.
>
> I talk about kernel crypto API and kernel clients.
>
> Please understand the following: when update() is called driver does not
> know if it is last update or not...
> That is essential.
> If client code would always use update/finup then it is fine..
> But original API and clients uses update/final...
> That is why some way (flag) needed to tell if finup() will be used or not...
>
> It gave up 20% performance improvement in some case because no extra
> memcpy has been done...
>
> If you think about other solution, please share...

If the crypto user can set the FINUP flag then it can just call
finup, simple.

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

2011-01-27 06:47:46

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: hash finup() issue

No.no.. That is not a flag for particular request...
The flag is how total hash will be handled...

FINUP flag means that client will call finup().
So it will be

update()
update()
...
update()
finup()


not
update()
...
update()
final()...

In other words...
update() means that it will be more data coming (like MSG_MORE)
finup() is just final update...

Basically driver on update() will not do extra buffering..

Is it clear now?

How would you solve such issue?

- Dmitry

On 27/01/11 01:32, ext Herbert Xu wrote:
> On Wed, Jan 26, 2011 at 09:47:27AM +0200, Dmitry Kasatkin wrote:
>> I am not talking about user space API at all.
>>
>> I talk about kernel crypto API and kernel clients.
>>
>> Please understand the following: when update() is called driver does not
>> know if it is last update or not...
>> That is essential.
>> If client code would always use update/finup then it is fine..
>> But original API and clients uses update/final...
>> That is why some way (flag) needed to tell if finup() will be used or not...
>>
>> It gave up 20% performance improvement in some case because no extra
>> memcpy has been done...
>>
>> If you think about other solution, please share...
> If the crypto user can set the FINUP flag then it can just call
> finup, simple.
>
> Cheers,

2011-01-27 07:19:00

by Herbert Xu

[permalink] [raw]
Subject: Re: hash finup() issue

On Thu, Jan 27, 2011 at 08:47:40AM +0200, Dmitry Kasatkin wrote:
> No.no.. That is not a flag for particular request...
> The flag is how total hash will be handled...
>
> FINUP flag means that client will call finup().
> So it will be
>
> update()
> update()
> ...
> update()
> finup()
>
>
> not
> update()
> ...
> update()
> final()...
>
> In other words...
> update() means that it will be more data coming (like MSG_MORE)
> finup() is just final update...
>
> Basically driver on update() will not do extra buffering..
>
> Is it clear now?

No I have absolutely no idea what you're saying. Can you give
an actual example of what the user is doing and what you want to
do in the driver?

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