DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results
@ 2024-01-03  3:56 Suanming Mou
  2024-01-03  3:56 ` [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Suanming Mou @ 2024-01-03  3:56 UTC (permalink / raw)
  To: Ciara Power; +Cc: dev

The function memcmp() returns an integer less than, equal to,
or greater than zero. In current code, if the first memcmp()
returns less than zero and the second memcmp() returns greater
than zero, the sum of results may still be 0 and indicates
verify succussed.

This commit converts the return value to be zero or greater
than zero. That will make sure the sum of results be correct.

Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---
 app/test-crypto-perf/cperf_test_verify.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index a6c0ffe813..8aa714b969 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -186,18 +186,18 @@ cperf_verify_op(struct rte_crypto_op *op,
 
 	if (cipher == 1) {
 		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
-			res += memcmp(data + cipher_offset,
+			res += !!memcmp(data + cipher_offset,
 					vector->ciphertext.data,
 					options->test_buffer_size);
 		else
-			res += memcmp(data + cipher_offset,
+			res += !!memcmp(data + cipher_offset,
 					vector->plaintext.data,
 					options->test_buffer_size);
 	}
 
 	if (auth == 1) {
 		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE)
-			res += memcmp(data + auth_offset,
+			res += !!memcmp(data + auth_offset,
 					vector->digest.data,
 					options->digest_sz);
 	}
-- 
2.34.1


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

* [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify
  2024-01-03  3:56 [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Suanming Mou
@ 2024-01-03  3:56 ` Suanming Mou
  2024-01-04  5:13   ` [EXT] " Anoob Joseph
  2024-01-03 11:33 ` [EXT] [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Anoob Joseph
  2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
  2 siblings, 1 reply; 10+ messages in thread
From: Suanming Mou @ 2024-01-03  3:56 UTC (permalink / raw)
  To: Ciara Power; +Cc: dev

AEAD users RTE_CRYPTO_AEAD_OP_* with aead_op and CIPHER uses
RTE_CRYPTO_CIPHER_OP_* with cipher_op in current code.

This commit aligns aead_op and cipher_op operation to fix
incorrect AEAD verification.

Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---
 app/test-crypto-perf/cperf_test_verify.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index 8aa714b969..525a2b1373 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -113,6 +113,7 @@ cperf_verify_op(struct rte_crypto_op *op,
 	uint8_t *data;
 	uint32_t cipher_offset, auth_offset;
 	uint8_t	cipher, auth;
+	bool is_encrypt = false;
 	int res = 0;
 
 	if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS)
@@ -154,12 +155,14 @@ cperf_verify_op(struct rte_crypto_op *op,
 		cipher_offset = 0;
 		auth = 0;
 		auth_offset = 0;
+		is_encrypt = options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT;
 		break;
 	case CPERF_CIPHER_THEN_AUTH:
 		cipher = 1;
 		cipher_offset = 0;
 		auth = 1;
 		auth_offset = options->test_buffer_size;
+		is_encrypt = options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT;
 		break;
 	case CPERF_AUTH_ONLY:
 		cipher = 0;
@@ -172,12 +175,14 @@ cperf_verify_op(struct rte_crypto_op *op,
 		cipher_offset = 0;
 		auth = 1;
 		auth_offset = options->test_buffer_size;
+		is_encrypt = options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT;
 		break;
 	case CPERF_AEAD:
 		cipher = 1;
 		cipher_offset = 0;
-		auth = 1;
+		auth = options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT;
 		auth_offset = options->test_buffer_size;
+		is_encrypt = !!auth;
 		break;
 	default:
 		res = 1;
@@ -185,7 +190,7 @@ cperf_verify_op(struct rte_crypto_op *op,
 	}
 
 	if (cipher == 1) {
-		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
+		if (is_encrypt)
 			res += !!memcmp(data + cipher_offset,
 					vector->ciphertext.data,
 					options->test_buffer_size);
-- 
2.34.1


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

* RE: [EXT] [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results
  2024-01-03  3:56 [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Suanming Mou
  2024-01-03  3:56 ` [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
@ 2024-01-03 11:33 ` Anoob Joseph
  2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
  2 siblings, 0 replies; 10+ messages in thread
From: Anoob Joseph @ 2024-01-03 11:33 UTC (permalink / raw)
  To: Suanming Mou, Ciara Power; +Cc: dev

> The function memcmp() returns an integer less than, equal to, or greater than
> zero. In current code, if the first memcmp() returns less than zero and the
> second memcmp() returns greater than zero, the sum of results may still be 0
> and indicates verify succussed.
> 
> This commit converts the return value to be zero or greater than zero. That will
> make sure the sum of results be correct.
> 
> Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>

Acked-by: Anoob Joseph <anoobj@marvell.com>


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

* RE: [EXT] [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify
  2024-01-03  3:56 ` [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
@ 2024-01-04  5:13   ` Anoob Joseph
  2024-01-04  8:56     ` Suanming Mou
  0 siblings, 1 reply; 10+ messages in thread
From: Anoob Joseph @ 2024-01-04  5:13 UTC (permalink / raw)
  To: Suanming Mou, Ciara Power; +Cc: dev

Hi Suanming,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Suanming Mou <suanmingm@nvidia.com>
> Sent: Wednesday, January 3, 2024 9:26 AM
> To: Ciara Power <ciara.power@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify
> 
> External Email
> 
> ----------------------------------------------------------------------
> AEAD users RTE_CRYPTO_AEAD_OP_* with aead_op and CIPHER uses
[Anoob] users -> uses

> RTE_CRYPTO_CIPHER_OP_* with cipher_op in current code.
> 
> This commit aligns aead_op and cipher_op operation to fix incorrect AEAD
> verification.
> 
> Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> ---
>  app/test-crypto-perf/cperf_test_verify.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-
> perf/cperf_test_verify.c
> index 8aa714b969..525a2b1373 100644
> --- a/app/test-crypto-perf/cperf_test_verify.c
> +++ b/app/test-crypto-perf/cperf_test_verify.c
> @@ -113,6 +113,7 @@ cperf_verify_op(struct rte_crypto_op *op,
>  	uint8_t *data;
>  	uint32_t cipher_offset, auth_offset;
>  	uint8_t	cipher, auth;
> +	bool is_encrypt = false;
>  	int res = 0;
> 
>  	if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) @@ -154,12
> +155,14 @@ cperf_verify_op(struct rte_crypto_op *op,
>  		cipher_offset = 0;
>  		auth = 0;
>  		auth_offset = 0;
> +		is_encrypt = options->cipher_op ==
> RTE_CRYPTO_CIPHER_OP_ENCRYPT;
>  		break;
>  	case CPERF_CIPHER_THEN_AUTH:
>  		cipher = 1;
>  		cipher_offset = 0;
>  		auth = 1;
>  		auth_offset = options->test_buffer_size;
> +		is_encrypt = options->cipher_op ==
> RTE_CRYPTO_CIPHER_OP_ENCRYPT;
>  		break;
>  	case CPERF_AUTH_ONLY:
>  		cipher = 0;
> @@ -172,12 +175,14 @@ cperf_verify_op(struct rte_crypto_op *op,
>  		cipher_offset = 0;
>  		auth = 1;
>  		auth_offset = options->test_buffer_size;
> +		is_encrypt = options->cipher_op ==
> RTE_CRYPTO_CIPHER_OP_ENCRYPT;
>  		break;
>  	case CPERF_AEAD:
>  		cipher = 1;
>  		cipher_offset = 0;
> -		auth = 1;
> +		auth = options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT;
>  		auth_offset = options->test_buffer_size;
> +		is_encrypt = !!auth;
>  		break;
>  	default:
>  		res = 1;
> @@ -185,7 +190,7 @@ cperf_verify_op(struct rte_crypto_op *op,
>  	}
> 
>  	if (cipher == 1) {
> -		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
> +		if (is_encrypt)

[Anoob] A similar check is there under 'auth == 1' check, right? Won't that also need fixing?

	if (auth == 1) {
		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE)

I think some renaming of the local variables might make code better.
        bool cipher, digest_verify = false, is_encrypt = false;

	case CPERF_CIPHER_THEN_AUTH:
		cipher = true;
		cipher_offset = 0;
		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
			is_encrypt = true;
			digest_verify = true; /* Assumption - options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE is verified elsewhere */
			auth_offset = options->test_buffer_size;
		}
		break;
	<...>
	case CPERF_AEAD:
		cipher = true;
		cipher_offset = 0;
                             if (options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT) {
                             	is_encrypt = true;
			digest_verify = true;
			auth_offset = options->test_buffer_size;
		}

What do you think?

>  			res += !!memcmp(data + cipher_offset,
>  					vector->ciphertext.data,
>  					options->test_buffer_size);
> --
> 2.34.1


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

* RE: [EXT] [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify
  2024-01-04  5:13   ` [EXT] " Anoob Joseph
@ 2024-01-04  8:56     ` Suanming Mou
  0 siblings, 0 replies; 10+ messages in thread
From: Suanming Mou @ 2024-01-04  8:56 UTC (permalink / raw)
  To: Anoob Joseph, Ciara Power; +Cc: dev

Hi,

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Thursday, January 4, 2024 1:13 PM
> To: Suanming Mou <suanmingm@nvidia.com>; Ciara Power
> <ciara.power@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [EXT] [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify
> 
> Hi Suanming,
> 
> Please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Suanming Mou <suanmingm@nvidia.com>
> > Sent: Wednesday, January 3, 2024 9:26 AM
> > To: Ciara Power <ciara.power@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [EXT] [PATCH 2/2] app/test-crypto-perf: fix encrypt operation
> > verify
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > AEAD users RTE_CRYPTO_AEAD_OP_* with aead_op and CIPHER uses
> [Anoob] users -> uses
> 
> > RTE_CRYPTO_CIPHER_OP_* with cipher_op in current code.
> >
> > This commit aligns aead_op and cipher_op operation to fix incorrect
> > AEAD verification.
> >
> > Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test
> > type")
> >
> > Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> > ---
> >  app/test-crypto-perf/cperf_test_verify.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-crypto-perf/cperf_test_verify.c
> > b/app/test-crypto- perf/cperf_test_verify.c index
> > 8aa714b969..525a2b1373 100644
> > --- a/app/test-crypto-perf/cperf_test_verify.c
> > +++ b/app/test-crypto-perf/cperf_test_verify.c
> > @@ -113,6 +113,7 @@ cperf_verify_op(struct rte_crypto_op *op,
> >  	uint8_t *data;
> >  	uint32_t cipher_offset, auth_offset;
> >  	uint8_t	cipher, auth;
> > +	bool is_encrypt = false;
> >  	int res = 0;
> >
> >  	if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS) @@ -154,12
> > +155,14 @@ cperf_verify_op(struct rte_crypto_op *op,
> >  		cipher_offset = 0;
> >  		auth = 0;
> >  		auth_offset = 0;
> > +		is_encrypt = options->cipher_op ==
> > RTE_CRYPTO_CIPHER_OP_ENCRYPT;
> >  		break;
> >  	case CPERF_CIPHER_THEN_AUTH:
> >  		cipher = 1;
> >  		cipher_offset = 0;
> >  		auth = 1;
> >  		auth_offset = options->test_buffer_size;
> > +		is_encrypt = options->cipher_op ==
> > RTE_CRYPTO_CIPHER_OP_ENCRYPT;
> >  		break;
> >  	case CPERF_AUTH_ONLY:
> >  		cipher = 0;
> > @@ -172,12 +175,14 @@ cperf_verify_op(struct rte_crypto_op *op,
> >  		cipher_offset = 0;
> >  		auth = 1;
> >  		auth_offset = options->test_buffer_size;
> > +		is_encrypt = options->cipher_op ==
> > RTE_CRYPTO_CIPHER_OP_ENCRYPT;
> >  		break;
> >  	case CPERF_AEAD:
> >  		cipher = 1;
> >  		cipher_offset = 0;
> > -		auth = 1;
> > +		auth = options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT;
> >  		auth_offset = options->test_buffer_size;
> > +		is_encrypt = !!auth;
> >  		break;
> >  	default:
> >  		res = 1;
> > @@ -185,7 +190,7 @@ cperf_verify_op(struct rte_crypto_op *op,
> >  	}
> >
> >  	if (cipher == 1) {
> > -		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
> > +		if (is_encrypt)
> 
> [Anoob] A similar check is there under 'auth == 1' check, right? Won't that also
> need fixing?
> 
> 	if (auth == 1) {
> 		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE)
> 
> I think some renaming of the local variables might make code better.
>         bool cipher, digest_verify = false, is_encrypt = false;
> 
> 	case CPERF_CIPHER_THEN_AUTH:
> 		cipher = true;
> 		cipher_offset = 0;
> 		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
> 			is_encrypt = true;
> 			digest_verify = true; /* Assumption - options->auth_op
> == RTE_CRYPTO_AUTH_OP_GENERATE is verified elsewhere */
> 			auth_offset = options->test_buffer_size;
> 		}
> 		break;
> 	<...>
> 	case CPERF_AEAD:
> 		cipher = true;
> 		cipher_offset = 0;
>                              if (options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT) {
>                              	is_encrypt = true;
> 			digest_verify = true;
> 			auth_offset = options->test_buffer_size;
> 		}
> 
> What do you think?

Yes, so we can totally remove the auth for now. I will do that. Thanks for the suggestion.

> 
> >  			res += !!memcmp(data + cipher_offset,
> >  					vector->ciphertext.data,
> >  					options->test_buffer_size);
> > --
> > 2.34.1


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

* [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results
  2024-01-03  3:56 [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Suanming Mou
  2024-01-03  3:56 ` [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
  2024-01-03 11:33 ` [EXT] [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Anoob Joseph
@ 2024-01-05  0:03 ` Suanming Mou
  2024-01-05  0:03   ` [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
                     ` (2 more replies)
  2 siblings, 3 replies; 10+ messages in thread
From: Suanming Mou @ 2024-01-05  0:03 UTC (permalink / raw)
  To: anoobj, ciara.power; +Cc: dev

The function memcmp() returns an integer less than, equal to,
or greater than zero. In current code, if the first memcmp()
returns less than zero and the second memcmp() returns greater
than zero, the sum of results may still be 0 and indicates
verify succussed.

This commit converts the return value to be zero or greater
than zero. That will make sure the sum of results be correct.

Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
Acked-by: Anoob Joseph <anoobj@marvell.com>
---
 app/test-crypto-perf/cperf_test_verify.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index a6c0ffe813..8aa714b969 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -186,18 +186,18 @@ cperf_verify_op(struct rte_crypto_op *op,
 
 	if (cipher == 1) {
 		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
-			res += memcmp(data + cipher_offset,
+			res += !!memcmp(data + cipher_offset,
 					vector->ciphertext.data,
 					options->test_buffer_size);
 		else
-			res += memcmp(data + cipher_offset,
+			res += !!memcmp(data + cipher_offset,
 					vector->plaintext.data,
 					options->test_buffer_size);
 	}
 
 	if (auth == 1) {
 		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE)
-			res += memcmp(data + auth_offset,
+			res += !!memcmp(data + auth_offset,
 					vector->digest.data,
 					options->digest_sz);
 	}
-- 
2.34.1


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

* [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify
  2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
@ 2024-01-05  0:03   ` Suanming Mou
  2024-01-05  4:44     ` [EXT] " Anoob Joseph
  2024-01-12 16:21   ` [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results Power, Ciara
  2024-02-01  8:47   ` [EXT] " Akhil Goyal
  2 siblings, 1 reply; 10+ messages in thread
From: Suanming Mou @ 2024-01-05  0:03 UTC (permalink / raw)
  To: anoobj, ciara.power; +Cc: dev

AEAD uses RTE_CRYPTO_AEAD_OP_* with aead_op and CIPHER uses
RTE_CRYPTO_CIPHER_OP_* with cipher_op in current code.

This commit aligns aead_op and cipher_op operation to fix
incorrect AEAD verification.

Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---

v2: align auth/cipher to bool.

---
 app/test-crypto-perf/cperf_test_verify.c | 55 ++++++++++++------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/app/test-crypto-perf/cperf_test_verify.c b/app/test-crypto-perf/cperf_test_verify.c
index 8aa714b969..2b0d3f142b 100644
--- a/app/test-crypto-perf/cperf_test_verify.c
+++ b/app/test-crypto-perf/cperf_test_verify.c
@@ -111,8 +111,10 @@ cperf_verify_op(struct rte_crypto_op *op,
 	uint32_t len;
 	uint16_t nb_segs;
 	uint8_t *data;
-	uint32_t cipher_offset, auth_offset;
-	uint8_t	cipher, auth;
+	uint32_t cipher_offset, auth_offset = 0;
+	bool cipher = false;
+	bool digest_verify = false;
+	bool is_encrypt = false;
 	int res = 0;
 
 	if (op->status != RTE_CRYPTO_OP_STATUS_SUCCESS)
@@ -150,42 +152,43 @@ cperf_verify_op(struct rte_crypto_op *op,
 
 	switch (options->op_type) {
 	case CPERF_CIPHER_ONLY:
-		cipher = 1;
+		cipher = true;
 		cipher_offset = 0;
-		auth = 0;
-		auth_offset = 0;
-		break;
-	case CPERF_CIPHER_THEN_AUTH:
-		cipher = 1;
-		cipher_offset = 0;
-		auth = 1;
-		auth_offset = options->test_buffer_size;
+		is_encrypt = options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT;
 		break;
 	case CPERF_AUTH_ONLY:
-		cipher = 0;
 		cipher_offset = 0;
-		auth = 1;
-		auth_offset = options->test_buffer_size;
+		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE) {
+			auth_offset = options->test_buffer_size;
+			digest_verify = true;
+		}
 		break;
+	case CPERF_CIPHER_THEN_AUTH:
 	case CPERF_AUTH_THEN_CIPHER:
-		cipher = 1;
+		cipher = true;
 		cipher_offset = 0;
-		auth = 1;
-		auth_offset = options->test_buffer_size;
+		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
+			auth_offset = options->test_buffer_size;
+			digest_verify = true;
+			is_encrypt = true;
+		}
 		break;
 	case CPERF_AEAD:
-		cipher = 1;
+		cipher = true;
 		cipher_offset = 0;
-		auth = 1;
-		auth_offset = options->test_buffer_size;
+		if (options->aead_op == RTE_CRYPTO_AEAD_OP_ENCRYPT) {
+			auth_offset = options->test_buffer_size;
+			digest_verify = true;
+			is_encrypt = true;
+		}
 		break;
 	default:
 		res = 1;
 		goto out;
 	}
 
-	if (cipher == 1) {
-		if (options->cipher_op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
+	if (cipher) {
+		if (is_encrypt)
 			res += !!memcmp(data + cipher_offset,
 					vector->ciphertext.data,
 					options->test_buffer_size);
@@ -195,12 +198,8 @@ cperf_verify_op(struct rte_crypto_op *op,
 					options->test_buffer_size);
 	}
 
-	if (auth == 1) {
-		if (options->auth_op == RTE_CRYPTO_AUTH_OP_GENERATE)
-			res += !!memcmp(data + auth_offset,
-					vector->digest.data,
-					options->digest_sz);
-	}
+	if (digest_verify)
+		res += !!memcmp(data + auth_offset, vector->digest.data, options->digest_sz);
 
 out:
 	rte_free(data);
-- 
2.34.1


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

* RE: [EXT] [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify
  2024-01-05  0:03   ` [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
@ 2024-01-05  4:44     ` Anoob Joseph
  0 siblings, 0 replies; 10+ messages in thread
From: Anoob Joseph @ 2024-01-05  4:44 UTC (permalink / raw)
  To: Suanming Mou, ciara.power; +Cc: dev

> AEAD uses RTE_CRYPTO_AEAD_OP_* with aead_op and CIPHER uses
> RTE_CRYPTO_CIPHER_OP_* with cipher_op in current code.
> 
> This commit aligns aead_op and cipher_op operation to fix incorrect AEAD
> verification.
> 
> Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>

Acked-by: Anoob Joseph <anoobj@marvell.com>



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

* RE: [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results
  2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
  2024-01-05  0:03   ` [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
@ 2024-01-12 16:21   ` Power, Ciara
  2024-02-01  8:47   ` [EXT] " Akhil Goyal
  2 siblings, 0 replies; 10+ messages in thread
From: Power, Ciara @ 2024-01-12 16:21 UTC (permalink / raw)
  To: Suanming Mou, anoobj; +Cc: dev



> -----Original Message-----
> From: Suanming Mou <suanmingm@nvidia.com>
> Sent: Friday, January 5, 2024 12:03 AM
> To: anoobj@marvell.com; Power, Ciara <ciara.power@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results
> 
> The function memcmp() returns an integer less than, equal to, or greater than
> zero. In current code, if the first memcmp() returns less than zero and the
> second memcmp() returns greater than zero, the sum of results may still be 0
> and indicates verify succussed.
> 
> This commit converts the return value to be zero or greater than zero. That will
> make sure the sum of results be correct.
> 
> Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
> ---
>  app/test-crypto-perf/cperf_test_verify.c | 6 +++---

Acked-by: Ciara Power <ciara.power@intel.com>

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

* RE: [EXT] [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results
  2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
  2024-01-05  0:03   ` [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
  2024-01-12 16:21   ` [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results Power, Ciara
@ 2024-02-01  8:47   ` Akhil Goyal
  2 siblings, 0 replies; 10+ messages in thread
From: Akhil Goyal @ 2024-02-01  8:47 UTC (permalink / raw)
  To: Suanming Mou, Anoob Joseph, ciara.power; +Cc: dev

> The function memcmp() returns an integer less than, equal to,
> or greater than zero. In current code, if the first memcmp()
> returns less than zero and the second memcmp() returns greater
> than zero, the sum of results may still be 0 and indicates
> verify succussed.
> 
> This commit converts the return value to be zero or greater
> than zero. That will make sure the sum of results be correct.
> 
> Fixes: df52cb3b6e13 ("app/crypto-perf: move verify as single test type")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> Acked-by: Anoob Joseph <anoobj@marvell.com>
Cc: stable@dpdk.org
Series
Applied to dpdk-next-crypto

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

end of thread, other threads:[~2024-02-01  8:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-03  3:56 [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Suanming Mou
2024-01-03  3:56 ` [PATCH 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
2024-01-04  5:13   ` [EXT] " Anoob Joseph
2024-01-04  8:56     ` Suanming Mou
2024-01-03 11:33 ` [EXT] [PATCH 1/2] app/test-crypto-perf: fix invalid memcmp results Anoob Joseph
2024-01-05  0:03 ` [PATCH v2 " Suanming Mou
2024-01-05  0:03   ` [PATCH v2 2/2] app/test-crypto-perf: fix encrypt operation verify Suanming Mou
2024-01-05  4:44     ` [EXT] " Anoob Joseph
2024-01-12 16:21   ` [PATCH v2 1/2] app/test-crypto-perf: fix invalid memcmp results Power, Ciara
2024-02-01  8:47   ` [EXT] " Akhil Goyal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).