test/cryptodev: fix incomplete data length

PATCH Superseded Kai Ji 4 participants 10 messages Nov 5, 2021 – Nov 11, 2021

Share & tools

Kai Ji

author

2021-11-05 15:42 UTC #0 lore raw

PATCH 1/1 test/cryptodev: fix incomplete data length

This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.

Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")

Cc [email protected]

Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-CMAC")

Cc [email protected]

Signed-off-by Kai Ji <[email protected]>

---
 app/test/test_cryptodev.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

app/test/test_cryptodev.c +9 -7

index 52457596e2..b926412742 100644
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata->cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
 		return retval;

@@ -7335,6 +7335,7 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 	unsigned int plaintext_len;
 	unsigned int ciphertext_pad_len;
 	unsigned int ciphertext_len;
+	unsigned int data_len;
 
 	struct rte_cryptodev_info dev_info;
 	struct rte_crypto_op *op;

@@ -7395,21 +7396,22 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	data_len = RTE_MAX(ciphertext_pad_len, plaintext_pad_len);
 
 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-				ciphertext_pad_len);
+				data_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-				plaintext_pad_len);
+				data_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, plaintext_pad_len);
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "plaintext:", plaintext, plaintext_len);
 	}

Signature · 1 line

2.17.1

Zhang, Roy Fan

Acked-by on v1 1/1

2021-11-05 16:31 UTC #1 lore raw

Quoted 15 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kai Ji
Sent: Friday, November 5, 2021 3:42 PM
To: [email protected]
Cc: Ji, Kai <[email protected]>; De Lara Guarch, Pablo <[email protected]>; [email protected]
Subject: [dpdk-dev] [dpdk-dev v1] test/cryptodev: fix incomplete data length
This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.

Quoted 7 lines

Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")
Cc [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-
CMAC")
Cc: [email protected]

Acked-by Fan Zhang <[email protected]>

Anoob Joseph

2021-11-08 04:05 UTC #2 lore raw

In reply to Kai Ji

Hi Kai,

Patch looks good. Wondering if we need same fix in functions such as " test_zuc_auth_cipher()".

We were also hitting this issue when we enabled few additional features in Marvell PMDs. Upon investigation, we realized that this issue would come up for certain packet size combinations if the padded lengths are not same. We observed the issue only with test_mixed_auth_cipher(), which is getting addressed with this patch. Just wondering if you have checked whether other places also would need a fix.

Thanks,
Anoob

Quoted 92 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kai Ji
Sent: Friday, November 5, 2021 9:12 PM
To: [email protected]
Cc: Kai Ji <[email protected]>; [email protected]; [email protected]
Subject: [EXT] [dpdk-dev] [dpdk-dev v1] test/cryptodev: fix incomplete data length
External Email
----------------------------------------------------------------------
This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")
Cc [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-
CMAC")
Cc: [email protected]
Signed-off-by Kai Ji <[email protected]>
---
 app/test/test_cryptodev.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
app/test/test_cryptodev.c index renamed from app/test/test_cryptodev.c +0 -0
52457596e2..b926412742 100644
app/test/test_cryptodev.c +3 -3
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct
 kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata-
 >cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
app/test/test_cryptodev.c +1 -0
 		return retval;
@@ -7335,6 +7335,7 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	unsigned int plaintext_len;
 	unsigned int ciphertext_pad_len;
 	unsigned int ciphertext_len;
+	unsigned int data_len;
 
 	struct rte_cryptodev_info dev_info;
app/test/test_cryptodev.c +4 -4
 	struct rte_crypto_op *op;
@@ -7395,21 +7396,22 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	data_len = RTE_MAX(ciphertext_pad_len, plaintext_pad_len);
 
 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
-				ciphertext_pad_len);
+				data_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
 ciphertext_pad_len);
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
-				plaintext_pad_len);
+				data_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
plaintext_pad_len);
app/test/test_cryptodev.c +1 -0
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "plaintext:", plaintext,
plaintext_len);
}
--
2.17.1

Anoob Joseph

2021-11-08 04:32 UTC #3 lore raw

Hi Kai,

Also, couple of nits. Please check inline.

Thanks,
Anoob

Quoted 84 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Anoob Joseph
Sent: Monday, November 8, 2021 9:36 AM
To: Kai Ji <[email protected]>; [email protected]; Akhil Goyal <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [EXT] [dpdk-dev v1] test/cryptodev: fix incomplete data length
Hi Kai,
Patch looks good. Wondering if we need same fix in functions such as " test_zuc_auth_cipher()".
We were also hitting this issue when we enabled few additional features in Marvell PMDs. Upon investigation, we realized that this issue would come up for certain packet size combinations if the padded lengths are not same. We observed the issue only with test_mixed_auth_cipher(), which is getting addressed with this patch. Just wondering if you have checked whether other places also would need a fix.
Thanks,
Anoob
Quoted 9 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Kai Ji
Sent: Friday, November 5, 2021 9:12 PM
To: [email protected]
Cc: Kai Ji <[email protected]>; [email protected]; [email protected]
Subject: [EXT] [dpdk-dev] [dpdk-dev v1] test/cryptodev: fix incomplete data length
Quoted 7 lines
External Email
----------------------------------------------------------------------
This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data
lengths for some tests.
Quoted 42 lines
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless
algorithms")
Cc: [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for
AES-CTR-
CMAC")
Cc: [email protected]
Signed-off-by Kai Ji <[email protected]>
---
 app/test/test_cryptodev.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
app/test/test_cryptodev.c +0 -0
index
52457596e2..b926412742 100644
app/test/test_cryptodev.c +3 -3
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct
 kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata-
 >cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
app/test/test_cryptodev.c +1 -0
 		return retval;
@@ -7335,6 +7335,7 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	unsigned int plaintext_len;
 	unsigned int ciphertext_pad_len;
 	unsigned int ciphertext_len;
+	unsigned int data_len;
 

Quoted 8 lines

app/test/test_cryptodev.c test_mixed_auth_cipher(const struct · line 7400

Quoted 8 lines

app/test/test_cryptodev.c +0 -0

 	struct rte_cryptodev_info dev_info;

app/test/test_cryptodev.c +1 -0

 	struct rte_crypto_op *op;

@@ -7395,21 +7396,22 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	data_len = RTE_MAX(ciphertext_pad_len, plaintext_pad_len);

[Anoob] Isn't ciphertext_pad_len guaranteed to be the larger one of the two? Do we need another variable and the RTE_MAX?

Quoted 15 lines

app/test/test_cryptodev.c +2 -2

 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
-				ciphertext_pad_len);
+				data_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
 ciphertext_pad_len);
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params-

Quoted 8 lines

app/test/test_cryptodev.c

Quoted 8 lines

app/test/test_cryptodev.c +1 -2

 >ibuf,
-				plaintext_pad_len);
+				data_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,

plaintext_pad_len);

app/test/test_cryptodev.c +1 -0

+			rte_pktmbuf_append(ut_params->obuf, data_len);

[Anoob] Now that more things are common across the branches, can we move out some bits outside the if condition? Like, the above line is definitely same and be kept outside condition. The append call prior to this can also be kept common if we can rename the local variable.

Quoted 6 lines

Quoted 1 line
debug_hexdump(stdout, "plaintext:", plaintext,
plaintext_len);
Quoted 4 lines
}
--
2.17.1

Ji, Kai

author

2021-11-09 00:53 UTC #4 lore raw

Hi Anoob,

Please see my commit inline below.

Thanks Kai

Quoted 32 lines

-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Monday, November 8, 2021 4:32 AM
To: Anoob Joseph <[email protected]>; Ji, Kai <[email protected]>; [email protected]; Akhil Goyal <[email protected]>
Cc: De Lara Guarch, Pablo <[email protected]>; [email protected]; Zhang, Roy Fan <[email protected]>
Subject: RE: [dpdk-dev] [EXT] [dpdk-dev v1] test/cryptodev: fix incomplete data length
Hi Kai,
Also, couple of nits. Please check inline.
Thanks,
Anoob
Quoted 15 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Anoob Joseph
Sent: Monday, November 8, 2021 9:36 AM
To: Kai Ji <[email protected]>; [email protected]; Akhil Goyal <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [dpdk-dev] [EXT] [dpdk-dev v1] test/cryptodev: fix incomplete data length
Hi Kai,
Patch looks good. Wondering if we need same fix in functions such as " test_zuc_auth_cipher()".

Quoted 8 lines

Quoted 6 lines
We were also hitting this issue when we enabled few additional features in Marvell PMDs. Upon investigation, we realized that this issue would come up for certain packet size combinations if the padded lengths are not same. We observed the issue only with test_mixed_auth_cipher(), which is getting addressed with this patch. Just wondering if you have checked whether other places also would need
a fix.
Quoted 1 line

[Kai] Yes, this fix should apply to test_zuc_auth_cipher(). However, I don't see any zuc test failed from my side as there is no zuc test vector to cover the OOP partial digest case. I think I will add this fix into the zuc test anyway.

Quoted 34 lines

Quoted 34 lines
Thanks,
Anoob
Quoted 11 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Kai Ji
Sent: Friday, November 5, 2021 9:12 PM
To: [email protected]
Cc: Kai Ji <[email protected]>; [email protected]; [email protected]
Subject: [EXT] [dpdk-dev] [dpdk-dev v1] test/cryptodev: fix incomplete data length
External Email
Quoted 7 lines
--------------------------------------------------------------------
-- This patch fixes incorrect data lengths computation in cryptodev unit test.
Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data
lengths for some tests.
Quoted 12 lines
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless
algorithms")
Cc: [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for
AES-CTR-
CMAC")
Cc: [email protected]
Signed-off-by Kai Ji <[email protected]>
---

<snip>

Quoted 7 lines

app/test/test_cryptodev.c +1 -0

@@ -7335,6 +7335,7 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	unsigned int plaintext_len;
 	unsigned int ciphertext_pad_len;
 	unsigned int ciphertext_len;
+	unsigned int data_len;

Quoted 11 lines

Quoted 8 lines
app/test/test_cryptodev.c test_mixed_auth_cipher(const struct · line 7400
Quoted 8 lines
app/test/test_cryptodev.c +0 -0
 	struct rte_cryptodev_info dev_info;
app/test/test_cryptodev.c +1 -0
 	struct rte_crypto_op *op;
@@ -7395,21 +7396,22 @@ test_mixed_auth_cipher(const struct
 mixed_cipher_auth_test_data *tdata,
 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	data_len = RTE_MAX(ciphertext_pad_len, plaintext_pad_len);
[Anoob] Isn't ciphertext_pad_len guaranteed to be the larger one of the two? Do we need another variable and the RTE_MAX?

[Kai] the ciphertext_pad_len should be always bigger than plaintext_pad_len, code changed in v2.

Quoted 17 lines

app/test/test_cryptodev.c +2 -2

 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
-				ciphertext_pad_len);
+				data_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
 ciphertext_pad_len);
+			rte_pktmbuf_append(ut_params->obuf, data_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,

Quoted 12 lines

Quoted 7 lines
Review
Quoted 7 lines
app/test/test_cryptodev.c +1 -2
-				plaintext_pad_len);
+				data_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
 		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
plaintext_pad_len);
(quoted diff) +1 -0
+			rte_pktmbuf_append(ut_params->obuf, data_len);
[Anoob] Now that more things are common across the branches, can we move out some bits outside the if condition? Like, the above line is definitely same and be kept outside condition. The append call prior to this can also be kept common if we can rename the local variable.

[Kai] code changed in v2

Quoted 7 lines

Quoted 7 lines
Quoted 2 lines
debug_hexdump(stdout, "plaintext:", plaintext,
plaintext_len);
Quoted 4 lines
}
--
2.17.1

Kai Ji

author

2021-11-09 10:27 UTC #5 lore raw

PATCH v2 1/1 test/cryptodev: fix incomplete data length

In reply to Kai Ji

Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")

Cc [email protected]

Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-CMAC")

Cc [email protected]

Fixes b1c1df46878d ("test/crypto: add ZUC test cases for auth-cipher")

Cc [email protected]

Signed-off-by Kai Ji <[email protected]>

---
 app/test/test_cryptodev.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

app/test/test_cryptodev.c +15 ~~-13~~

index 52457596e2..2dd1bbb64c 100644
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata->cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
 		return retval;

@@ -6332,20 +6332,20 @@ test_zuc_auth_cipher(const struct wireless_test_data *tdata,
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
 					ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 			ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case */
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-					plaintext_pad_len);
+					ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, plaintext_pad_len);
 		debug_hexdump(stdout, "plaintext:", plaintext,
 			plaintext_len);
 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
+
 	/* Create ZUC operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 		tdata->digest.data, tdata->digest.len,

@@ -7395,24 +7395,26 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	if (ciphertext_pad_len > plaintext_pad_len)
+		printf("NOOB: \n");
 
 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
 				ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case */
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-				plaintext_pad_len);
+				ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, plaintext_pad_len);
 		debug_hexdump(stdout, "plaintext:", plaintext, plaintext_len);
 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
+
 	/* Create the operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 			tdata->digest_enc.data, tdata->digest_enc.len,

Signature · 1 line

2.17.1

Anoob Joseph

2021-11-09 10:32 UTC #6 lore raw

Hi Kai,

Minor nit inline.

Thanks,
Anoob

Quoted 86 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kai Ji
Sent: Tuesday, November 9, 2021 3:57 PM
To: [email protected]
Cc: Kai Ji <[email protected]>; [email protected]; [email protected]; [email protected]
Subject: [EXT] [dpdk-dev] [dpdk-dev v2] test/cryptodev: fix incomplete data length
External Email
----------------------------------------------------------------------
This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")
Cc [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-
CMAC")
Cc: [email protected]
Fixes b1c1df46878d ("test/crypto: add ZUC test cases for auth-cipher")
Cc [email protected]
Signed-off-by Kai Ji <[email protected]>
---
 app/test/test_cryptodev.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)
app/test/test_cryptodev.c index renamed from app/test/test_cryptodev.c +0 -0
52457596e2..2dd1bbb64c 100644
app/test/test_cryptodev.c +3 -3
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct
 kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata-
 >cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
app/test/test_cryptodev.c +4 -5
 		return retval;
@@ -6332,20 +6332,20 @@ test_zuc_auth_cipher(const struct
 wireless_test_data *tdata,
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
 					ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
 ciphertext_pad_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 			ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case
 */
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params-
 >ibuf,
-					plaintext_pad_len);
+					ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,
 plaintext_pad_len);
 		debug_hexdump(stdout, "plaintext:", plaintext,
 			plaintext_len);
 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf,
ciphertext_pad_len);
app/test/test_cryptodev.c +1 -0
+
 	/* Create ZUC operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 		tdata->digest.data, tdata->digest.len, @@ -7395,24 +7395,26
@@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,

app/test/test_cryptodev.c test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata, · line 7399

Quoted 5 lines

app/test/test_cryptodev.c +2 -0

 	plaintext_len = ceil_byte_length(tdata->plaintext.len_bits);
 	ciphertext_pad_len = RTE_ALIGN_CEIL(ciphertext_len, 16);
 	plaintext_pad_len = RTE_ALIGN_CEIL(plaintext_len, 16);
+	if (ciphertext_pad_len > plaintext_pad_len)
+		printf("NOOB: \n");

[Anoob] Is the above intentional?

Quoted 35 lines

app/test/test_cryptodev.c +0 -0

 	if (verify) {
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params-

Quoted 1 line

ibuf,

app/test/test_cryptodev.c +0 -2

 				ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,

ciphertext_pad_len);

app/test/test_cryptodev.c +1 -0

 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case

*/
plaintext = (uint8_t *)rte_pktmbuf_append(ut_params-

Quoted 1 line

ibuf,

app/test/test_cryptodev.c +1 -3

-				plaintext_pad_len);
+				ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf,

plaintext_pad_len);
debug_hexdump(stdout, "plaintext:", plaintext,
plaintext_len);

app/test/test_cryptodev.c +2 -0

 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf,

ciphertext_pad_len);

app/test/test_cryptodev.c +1 -1

+
 	/* Create the operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 			tdata->digest_enc.data, tdata->digest_enc.len,
--

2.17.1

Kai Ji

author

2021-11-09 10:42 UTC #7 lore raw

PATCH v3 1/1 test/cryptodev: fix incomplete data length

In reply to Kai Ji

Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")

Cc [email protected]

Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-CMAC")

Cc [email protected]

Fixes b1c1df46878d ("test/crypto: add ZUC test cases for auth-cipher")

Cc [email protected]

Signed-off-by Kai Ji <[email protected]>

---

v2:
- review comments addressed

v3:
- remove unneeded debug code

 app/test/test_cryptodev.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

app/test/test_cryptodev.c +13 ~~-13~~

index 52457596e2..52e975b74c 100644
@@ -4102,9 +4102,9 @@ test_kasumi_decryption(const struct kasumi_test_data *tdata)
 
 	/* Create KASUMI operation */
 	retval = create_wireless_algo_cipher_operation(tdata->cipher_iv.data,
-					tdata->cipher_iv.len,
-					tdata->ciphertext.len,
-					tdata->validCipherOffsetInBits.len);
+			tdata->cipher_iv.len,
+			RTE_ALIGN_CEIL(tdata->validCipherLenInBits.len, 8),
+			tdata->validCipherOffsetInBits.len);
 	if (retval < 0)
 		return retval;

@@ -6332,20 +6332,20 @@ test_zuc_auth_cipher(const struct wireless_test_data *tdata,
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
 					ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 			ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case */
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-					plaintext_pad_len);
+					ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, plaintext_pad_len);
 		debug_hexdump(stdout, "plaintext:", plaintext,
 			plaintext_len);
 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
+
 	/* Create ZUC operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 		tdata->digest.data, tdata->digest.len,

@@ -7400,19 +7400,19 @@ test_mixed_auth_cipher(const struct mixed_cipher_auth_test_data *tdata,
 		ciphertext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
 				ciphertext_pad_len);
 		memcpy(ciphertext, tdata->ciphertext.data, ciphertext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
 		debug_hexdump(stdout, "ciphertext:", ciphertext,
 				ciphertext_len);
 	} else {
+		/* make sure enough space to cover partial digest verify case */
 		plaintext = (uint8_t *)rte_pktmbuf_append(ut_params->ibuf,
-				plaintext_pad_len);
+				ciphertext_pad_len);
 		memcpy(plaintext, tdata->plaintext.data, plaintext_len);
-		if (op_mode == OUT_OF_PLACE)
-			rte_pktmbuf_append(ut_params->obuf, plaintext_pad_len);
 		debug_hexdump(stdout, "plaintext:", plaintext, plaintext_len);
 	}
 
+	if (op_mode == OUT_OF_PLACE)
+		rte_pktmbuf_append(ut_params->obuf, ciphertext_pad_len);
+
 	/* Create the operation */
 	retval = create_wireless_algo_auth_cipher_operation(
 			tdata->digest_enc.data, tdata->digest_enc.len,

--
2.17.1

Anoob Joseph

Acked-by on v3 1/1

2021-11-09 16:25 UTC #8 lore raw

Hi Kai,

Thanks for addressing the comments.

Acked-by Anoob Joseph <[email protected]>

Quoted 29 lines

----------------------------------------------------------------------
This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")
Cc [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-
CMAC")
Cc: [email protected]
Fixes b1c1df46878d ("test/crypto: add ZUC test cases for auth-cipher")
Cc [email protected]
Signed-off-by Kai Ji <[email protected]>
---
v2:
- review comments addressed
v3:
- remove unneeded debug code
 app/test/test_cryptodev.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

Akhil Goyal

Applied

2021-11-11 11:17 UTC #9 lore raw

In reply to Kai Ji

Quoted 10 lines

This patch fixes incorrect data lengths computation in cryptodev unit test. Previously some data lengths were incorrectly set, which was insensitive for crypto op unit tets but is critical for raw data path API unit tests. The patch addressed the issue by setting the correct data lengths for some tests.
Fixes 681f540da52b ("cryptodev: do not use AAD in wireless algorithms")
Cc [email protected]
Fixes e847fc512817 ("test/crypto: add encrypted digest case for AES-CTR-

Quoted 8 lines

CMAC")
Cc: [email protected]
Fixes b1c1df46878d ("test/crypto: add ZUC test cases for auth-cipher")
Cc [email protected]
Signed-off-by Kai Ji <[email protected]>
---

Applied to dpdk-next-crypto

Cc [email protected]