diff mbox

crypto: tcrypt speed: fix filter for aead algorithms

Message ID 1422971988-21517-1-git-send-email-cristian.stoica@freescale.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show

Commit Message

Cristian Stoica Feb. 3, 2015, 1:59 p.m. UTC
test_aead_speed is written for sync algorithms without specifically
requiring them. The effect is that an async algorithm may be used without
setting up the request callback, this leading to a kernel panic.

Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com>
---
 crypto/tcrypt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Herbert Xu Feb. 27, 2015, 9:25 a.m. UTC | #1
On Tue, Feb 03, 2015 at 03:59:48PM +0200, Cristian Stoica wrote:
> test_aead_speed is written for sync algorithms without specifically
> requiring them. The effect is that an async algorithm may be used without
> setting up the request callback, this leading to a kernel panic.
> 
> Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com>

Please fix it to test asynchronously instead.

Thanks,
Cristian Stoica Feb. 27, 2015, 11:14 a.m. UTC | #2
Hi Herbert,

On 02/27/2015 11:25 AM, Herbert Xu wrote:
> On Tue, Feb 03, 2015 at 03:59:48PM +0200, Cristian Stoica wrote:
>> test_aead_speed is written for sync algorithms without specifically
[...]
> Please fix it to test asynchronously instead.

Thanks for review.

I think that a fix for async aead requires a different test function as
is the case with test_cipher_speed/test_acipher_speed and also with
test_hash_speed/test_ahash_speed.

But I see a issue here with the current async tests:

int test_acipher_cycles(...)
{
  for (i = 0; i < 8; i++) {
    [...]
    ret = do_one_acipher_op(req, crypto_ablkcipher_encrypt(req));
    [...]
  }
}

and in do_one_acipher_op we wait for completion of
crypto_ablkcipher_encrypt:

if (ret == -EINPROGRESS || ret == -EBUSY) {
	struct tcrypt_result *tr = req->base.data;
	ret = wait_for_completion_interruptible(&tr->completion);
}

Doesn't this defeat the purpose of async execution?

Cristian S.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu Feb. 28, 2015, 9:13 a.m. UTC | #3
On Fri, Feb 27, 2015 at 01:14:20PM +0200, Cristian Stoica wrote:
> Hi Herbert,
> 
> On 02/27/2015 11:25 AM, Herbert Xu wrote:
> > On Tue, Feb 03, 2015 at 03:59:48PM +0200, Cristian Stoica wrote:
> >> test_aead_speed is written for sync algorithms without specifically
> [...]
> > Please fix it to test asynchronously instead.
> 
> Thanks for review.
> 
> I think that a fix for async aead requires a different test function as
> is the case with test_cipher_speed/test_acipher_speed and also with
> test_hash_speed/test_ahash_speed.
> 
> But I see a issue here with the current async tests:
> 
> int test_acipher_cycles(...)
> {
>   for (i = 0; i < 8; i++) {
>     [...]
>     ret = do_one_acipher_op(req, crypto_ablkcipher_encrypt(req));
>     [...]
>   }
> }
> 
> and in do_one_acipher_op we wait for completion of
> crypto_ablkcipher_encrypt:
> 
> if (ret == -EINPROGRESS || ret == -EBUSY) {
> 	struct tcrypt_result *tr = req->base.data;
> 	ret = wait_for_completion_interruptible(&tr->completion);
> }
> 
> Doesn't this defeat the purpose of async execution?

We're trying to measure the minimum amount of time it takes
for the implementation to handle one request so waiting for
completion makes sense.

Cheers,
diff mbox

Patch

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 4b9e23f..5dc5a25 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -314,7 +314,8 @@  static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 	asg = &sg[8];
 	sgout = &asg[8];
 
-	tfm = crypto_alloc_aead(algo, 0, 0);
+	/* This test is not for ASYNC algorithms */
+	tfm = crypto_alloc_aead(algo, 0, CRYPTO_ALG_ASYNC);
 
 	if (IS_ERR(tfm)) {
 		pr_err("alg: aead: Failed to load transform for %s: %ld\n", algo,