All of lore.kernel.org
 help / color / mirror / Atom feed
* hash finup() issue
@ 2011-01-25 13:44 Dmitry Kasatkin
  2011-01-25 23:29 ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2011-01-25 13:44 UTC (permalink / raw)
  To: linux-crypto, ext Herbert Xu

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: hash finup() issue
  2011-01-25 13:44 hash finup() issue Dmitry Kasatkin
@ 2011-01-25 23:29 ` Herbert Xu
  2011-01-26  7:47   ` Dmitry Kasatkin
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2011-01-25 23:29 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: linux-crypto

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: hash finup() issue
  2011-01-25 23:29 ` Herbert Xu
@ 2011-01-26  7:47   ` Dmitry Kasatkin
  2011-01-26 23:32     ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2011-01-26  7:47 UTC (permalink / raw)
  To: ext Herbert Xu; +Cc: linux-crypto

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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: hash finup() issue
  2011-01-26  7:47   ` Dmitry Kasatkin
@ 2011-01-26 23:32     ` Herbert Xu
  2011-01-27  6:47       ` Dmitry Kasatkin
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2011-01-26 23:32 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: linux-crypto

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: hash finup() issue
  2011-01-26 23:32     ` Herbert Xu
@ 2011-01-27  6:47       ` Dmitry Kasatkin
  2011-01-27  7:18         ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2011-01-27  6:47 UTC (permalink / raw)
  To: ext Herbert Xu; +Cc: linux-crypto

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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: hash finup() issue
  2011-01-27  6:47       ` Dmitry Kasatkin
@ 2011-01-27  7:18         ` Herbert Xu
  0 siblings, 0 replies; 6+ messages in thread
From: Herbert Xu @ 2011-01-27  7:18 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: linux-crypto

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 <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-27  7:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-25 13:44 hash finup() issue Dmitry Kasatkin
2011-01-25 23:29 ` Herbert Xu
2011-01-26  7:47   ` Dmitry Kasatkin
2011-01-26 23:32     ` Herbert Xu
2011-01-27  6:47       ` Dmitry Kasatkin
2011-01-27  7:18         ` Herbert Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.