crypto_shash_update & CRYPTO_TFM_REQ_MAY_SLEEP

DISCUSSION Dmitry Kasatkin 2 participants 11 messages Feb 3, 2010 – Feb 16, 2010 Crypto API

lore

Share & tools

lore

Dmitry Kasatkin

author

2010-02-03 17:21 UTC #0 lore raw

Hi,

One interesting issue

Many clients in the kernel set CRYPTO_TFM_REQ_MAY_SLEEP to desc.flags. It is used by crypto_yeld().

But the flags also available in the driver.
One can assume that it is possible to sleep, but is not, because crypto walk will

    walk->data = crypto_kmap(walk->pg, 0);

------------------

    for (nbytes = crypto_hash_walk_first_compat(hdesc, &walk, sg, len);
         nbytes > 0; nbytes = crypto_hash_walk_done(&walk, nbytes))
        nbytes = crypto_shash_update(desc, walk.data, nbytes);

------------------

That is quite confusing...
I would expect that driver could sleep while hw is doing calculation.

Seems most of the clients uses sync API (linux/net has only sync).

Any comments? Ideas?

Thanks.

- Dmitry

Herbert Xu

maintainer

2010-02-09 07:41 UTC #1 lore raw

Quoted 5 lines · Dmitry Kasatkin

One interesting issue
Many clients in the kernel set CRYPTO_TFM_REQ_MAY_SLEEP to desc.flags. It is used by crypto_yeld().

That flag is really only meaningful for synchronous implementations.

For hardware crypto that is asynchronous, you can simply ignore it in most cases (exceptions include invoking a syncrhonous backup, for example).

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Dmitry Kasatkin

author

2010-02-09 08:46 UTC #2 lore raw

Hi,

Ok...

But normally kernel client code uses sync API as shash.

And I provide async hw driver via async ahash API. That is simply will not be used at all.

Client must be changed to use async ahash API in order to benefit from HW crypto.

Right?

Thanks,
Dmitry

Quoted 16 lines · ext Herbert Xu

Quoted 5 lines · Dmitry Kasatkin
One interesting issue
Many clients in the kernel set CRYPTO_TFM_REQ_MAY_SLEEP to desc.flags. It is used by crypto_yeld().
That flag is really only meaningful for synchronous implementations.
For hardware crypto that is asynchronous, you can simply ignore it in most cases (exceptions include invoking a syncrhonous backup, for example).
Cheers,

Herbert Xu

maintainer

2010-02-09 09:02 UTC #3 lore raw

Quoted 6 lines · Dmitry Kasatkin

Hi,
Ok...
But normally kernel client code uses sync API as shash.

Quoted 7 lines

And I provide async hw driver via async ahash API. That is simply will not be used at all.
Client must be changed to use async ahash API in order to benefit from HW crypto.
Right?

Yes we will convert hash users over to ahash where appropriate, starting with authenc which covers IPsec.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Dmitry Kasatkin

author

2010-02-10 12:17 UTC #4 lore raw

In reply to Herbert Xu

Hi,

Anyway, crypto driver is called with CRYPTO_TFM_REQ_MAY_SLEEP flag set. It basically mean it can sleep.
But it is not because of kmap_atomic()

So no possibility to know if to use GFP_KERNEL or GFP_ATOMIC.

I guess that is not correct way.

If driver cannot sleep then CRYPTO_TFM_REQ_MAY_SLEEP should not be set when calling it.

Even sync implementation might use HW and sleep a bit while HW is doing calculation...

- Dmitry

Quoted 16 lines · ext Herbert Xu

Quoted 5 lines · Dmitry Kasatkin
One interesting issue
Many clients in the kernel set CRYPTO_TFM_REQ_MAY_SLEEP to desc.flags. It is used by crypto_yeld().
That flag is really only meaningful for synchronous implementations.
For hardware crypto that is asynchronous, you can simply ignore it in most cases (exceptions include invoking a syncrhonous backup, for example).
Cheers,

Dmitry Kasatkin

author

2010-02-15 19:20 UTC #5 lore raw

Hi,

Alg code and drivers uses req/desc->flags to use GFP_KERNEL or GFP_ATOMIC.

crypto_kmap() -> Does not it make a sense to also use kmap/kmap_atomic instead of just kmap_atomic()?

-----------------------------------
static inline void *crypto_kmap(struct page *page, int out)

{
    return kmap_atomic(page, crypto_kmap_type(out));
}

-------------------------------------------------

It is now not only about sync but also async code. Would be nice to "wait" for hash completion in some cases...

What do you think?
What is the reason to have only kmap_atomic()?

- Dmitry

Quoted 44 lines · Kasatkin Dmitry (Nokia-D/Helsinki)

Hi,
Anyway, crypto driver is called with CRYPTO_TFM_REQ_MAY_SLEEP flag set. It basically mean it can sleep.
But it is not because of kmap_atomic()
So no possibility to know if to use GFP_KERNEL or GFP_ATOMIC.
I guess that is not correct way.
If driver cannot sleep then CRYPTO_TFM_REQ_MAY_SLEEP should not be set when calling it.
Even sync implementation might use HW and sleep a bit while HW is doing calculation...
- Dmitry
Quoted 9 lines · ext Herbert Xu
Quoted 6 lines · Dmitry Kasatkin
One interesting issue
Many clients in the kernel set CRYPTO_TFM_REQ_MAY_SLEEP to desc.flags. It is used by crypto_yeld().
Quoted 9 lines
That flag is really only meaningful for synchronous implementations.
For hardware crypto that is asynchronous, you can simply ignore it in most cases (exceptions include invoking a syncrhonous backup, for example).
Cheers,
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Herbert Xu

maintainer

2010-02-16 00:44 UTC #6 lore raw

Quoted 6 lines · Dmitry Kasatkin

Hi,
Alg code and drivers uses req/desc->flags to use GFP_KERNEL or GFP_ATOMIC.
crypto_kmap() -> Does not it make a sense to also use kmap/kmap_atomic instead of just kmap_atomic()?

crypto_kmap is not meant for async drivers. Why do you need it?

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Dmitry Kasatkin

author

2010-02-16 06:41 UTC #7 lore raw

Hi,

we have HW accelerator which can be accessed without DMA. CPU just sequentially writes data to the port and then reads result.

But before it can read result Interrupt will happen to notify processing completion.

Instead of looping over "ready" bit the process would just go to sleep:
wait_for_completion().

Can you please shortly tell why crypto_kmap() cannot be also like GFP_KERNEL or GFP_ATOMIC?

Is it really important to have kmap_atomic()?

Thanks,
Dmitry

Quoted 14 lines · ext Herbert Xu

Quoted 7 lines · Dmitry Kasatkin
Hi,
Alg code and drivers uses req/desc->flags to use GFP_KERNEL or GFP_ATOMIC.
crypto_kmap() -> Does not it make a sense to also use kmap/kmap_atomic instead of just kmap_atomic()?
crypto_kmap is not meant for async drivers. Why do you need it?
Cheers,

Herbert Xu

maintainer

2010-02-16 06:58 UTC #8 lore raw

Quoted 4 lines · Dmitry Kasatkin

Hi,
we have HW accelerator which can be accessed without DMA. CPU just sequentially writes data to the port and then reads result.

Well if that's the case then you definitely don't need anything other than kmap_atomic. You should be able to map it atomically, write the data out, unmap it, and then wait for the completion.

Non-atomic kmaps should be avoided if at all possible.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Dmitry Kasatkin

author

2010-02-16 07:10 UTC #9 lore raw

Hi,

How is it possible to wait for completion here?: shash.c

shash_compat_digest()
{
....

        data = crypto_kmap(sg_page(sg), 0);
        err = crypto_shash_digest(desc, data + offset, nbytes, out);
        crypto_kunmap(data, 0);

....
}

It happens as single operation/call.

Thanks,
Dmitry

Quoted 16 lines · ext Herbert Xu

Quoted 5 lines · Dmitry Kasatkin
Hi,
we have HW accelerator which can be accessed without DMA. CPU just sequentially writes data to the port and then reads result.
Well if that's the case then you definitely don't need anything other than kmap_atomic. You should be able to map it atomically, write the data out, unmap it, and then wait for the completion.
Non-atomic kmaps should be avoided if at all possible.
Cheers,

Herbert Xu

maintainer

2010-02-16 07:27 UTC #10 lore raw

Quoted 7 lines · Dmitry Kasatkin

Hi,
How is it possible to wait for completion here?: shash.c
shash_compat_digest()
{
....

Quoted 7 lines

        data = crypto_kmap(sg_page(sg), 0);
        err = crypto_shash_digest(desc, data + offset, nbytes, out);
        crypto_kunmap(data, 0);

....
}

It happens as single operation/call.

Well you'd do it as an ahash algorithm, not shash.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt