test/crypto: free memory in error and skip paths

PATCH Superseded Anoob Joseph 3 participants 9 messages May 12, 2023 – May 24, 2023

Share & tools

Anoob Joseph

author

2023-05-12 14:10 UTC #0 lore raw

PATCH 1/2 test/crypto: free memory in error and skip paths

In multi session tests, multiple sessions get created. So the handling in ut_teardown won't guard against any memory that is not freed by the test case. Test case should free sessions as well as local memory that was used to save session pointers both in case of unsupported cases as well as operation failures.

Signed-off-by Anoob Joseph <[email protected]>

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

app/test/test_cryptodev.c +39 ~~-16~~

index fb4fc4e805..86e63a33fc 100644
@@ -12751,8 +12751,8 @@ test_multi_session(void)
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
 	struct crypto_unittest_params *ut_params = &unittest_params;
 	struct rte_cryptodev_info dev_info;
+	int i, nb_sess, ret = TEST_SUCCESS;
 	void **sessions;
-	uint16_t i;
 
 	/* Verify the capabilities */
 	struct rte_cryptodev_sym_capability_idx cap_idx;

@@ -12782,21 +12782,25 @@ test_multi_session(void)
 		sessions[i] = rte_cryptodev_sym_session_create(
 			ts_params->valid_devs[0], &ut_params->auth_xform,
 				ts_params->session_mpool);
-		if (sessions[i] == NULL && rte_errno == ENOTSUP)
-			return TEST_SKIPPED;
+		if (sessions[i] == NULL && rte_errno == ENOTSUP) {
+			nb_sess = i;
+			ret = TEST_SKIPPED;
+			break;
+		}
 
 		TEST_ASSERT_NOT_NULL(sessions[i],
 				"Session creation failed at session number %u",
 				i);
+
 		/* Attempt to send a request on each session */
-		TEST_ASSERT_SUCCESS( test_AES_CBC_HMAC_SHA512_decrypt_perform(
+		ret = test_AES_CBC_HMAC_SHA512_decrypt_perform(
 			sessions[i],
 			ut_params,
 			ts_params,
 			catch_22_quote_2_512_bytes_AES_CBC_ciphertext,
 			catch_22_quote_2_512_bytes_AES_CBC_HMAC_SHA512_digest,
-			aes_cbc_iv),
-			"Failed to perform decrypt on request number %u.", i);
+			aes_cbc_iv);
+
 		/* free crypto operation structure */
 		rte_crypto_op_free(ut_params->op);

@@ -12815,16 +12819,25 @@ test_multi_session(void)
 			rte_pktmbuf_free(ut_params->ibuf);
 			ut_params->ibuf = 0;
 		}
+
+		if (ret == TEST_SKIPPED) {
+			i++;
+			break;
+		}
+
+		TEST_ASSERT_SUCCESS(ret, "Failed to perform decrypt on request number %u.", i);
 	}
 
-	for (i = 0; i < MAX_NB_SESSIONS; i++) {
+	nb_sess = i;
+
+	for (i = 0; i < nb_sess; i++) {
 		rte_cryptodev_sym_session_free(ts_params->valid_devs[0],
 				sessions[i]);
 	}
 
 	rte_free(sessions);
 
-	return TEST_SUCCESS;
+	return ret;
 }
 
 struct multi_session_params {

@@ -12843,8 +12856,9 @@ test_multi_session_random_usage(void)
 {
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
 	struct rte_cryptodev_info dev_info;
+	uint32_t nb_sess, i, j;
+	int ret = TEST_SUCCESS;
 	void **sessions;
-	uint32_t i, j;
 	struct multi_session_params ut_paramz[] = {
 
 		{

@@ -12903,27 +12917,30 @@ test_multi_session_random_usage(void)
 				ts_params->valid_devs[0],
 				&ut_paramz[i].ut_params.auth_xform,
 				ts_params->session_mpool);
-		if (sessions[i] == NULL && rte_errno == ENOTSUP)
-			return TEST_SKIPPED;
+		if (sessions[i] == NULL && rte_errno == ENOTSUP) {
+			nb_sess = i;
+			ret = TEST_SKIPPED;
+			goto session_clear;
+		}
 
 		TEST_ASSERT_NOT_NULL(sessions[i],
 				"Session creation failed at session number %u",
 				i);
 	}
 
+	nb_sess = i;
+
 	srand(time(NULL));
 	for (i = 0; i < 40000; i++) {
 
 		j = rand() % MB_SESSION_NUMBER;
 
-		TEST_ASSERT_SUCCESS(
-			test_AES_CBC_HMAC_SHA512_decrypt_perform(
+		ret = test_AES_CBC_HMAC_SHA512_decrypt_perform(
 					sessions[j],
 					&ut_paramz[j].ut_params,
 					ts_params, ut_paramz[j].cipher,
 					ut_paramz[j].digest,
-					ut_paramz[j].iv),
-			"Failed to perform decrypt on request number %u.", i);
+					ut_paramz[j].iv);
 
 		rte_crypto_op_free(ut_paramz[j].ut_params.op);

@@ -12943,9 +12960,15 @@ test_multi_session_random_usage(void)
 			rte_pktmbuf_free(ut_paramz[j].ut_params.ibuf);
 			ut_paramz[j].ut_params.ibuf = 0;
 		}
+
+		if (ret == TEST_SKIPPED)
+			break;
+
+		TEST_ASSERT_SUCCESS(ret, "Failed to perform decrypt on request number %u.", i);
 	}
 
-	for (i = 0; i < MB_SESSION_NUMBER; i++) {
+session_clear:
+	for (i = 0; i < nb_sess; i++) {
 		rte_cryptodev_sym_session_free(ts_params->valid_devs[0],
 				sessions[i]);
 	}

Signature · 1 line

2.25.1

Anoob Joseph

author

2023-05-12 14:10 UTC #1 lore raw

PATCH 2/2 test/crypto: handle return code

The sub test case, test_snow3g_decryption, may fail for non-critical reasons such as lack of support. Handle the return value gracefully to allow TEST_SKIPPED return value to be propagated correctly.

Signed-off-by Anoob Joseph <[email protected]>

---
 app/test/test_cryptodev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

app/test/test_cryptodev.c +4 -2

index 86e63a33fc..d212e71ba6 100644
@@ -6947,6 +6947,7 @@ test_snow3g_decryption_with_digest_test_case_1(void)
 	struct snow3g_hash_test_data snow3g_hash_data;
 	struct rte_cryptodev_info dev_info;
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	int ret;
 
 	rte_cryptodev_info_get(ts_params->valid_devs[0], &dev_info);
 	uint64_t feat_flags = dev_info.feature_flags;

@@ -6962,8 +6963,9 @@ test_snow3g_decryption_with_digest_test_case_1(void)
 	 */
 	snow3g_hash_test_vector_setup(&snow3g_test_case_7, &snow3g_hash_data);
 
-	if (test_snow3g_decryption(&snow3g_test_case_7))
-		return TEST_FAILED;
+	ret = test_snow3g_decryption(&snow3g_test_case_7);
+	if (ret != TEST_SUCCESS)
+		return ret;
 
 	return test_snow3g_authentication_verify(&snow3g_hash_data);
 }

Signature · 1 line

2.25.1

Akhil Goyal

Acked-by on v1 2/2

2023-05-16 10:12 UTC #2 lore raw

RE: [PATCH 2/2] test/crypto: handle return code

Quoted 7 lines

Subject: [PATCH 2/2] test/crypto: handle return code
The sub test case, test_snow3g_decryption, may fail for non-critical reasons such as lack of support. Handle the return value gracefully to allow TEST_SKIPPED return value to be propagated correctly.
Signed-off-by Anoob Joseph <[email protected]>

Acked-by Akhil Goyal <[email protected]>

Power, Ciara

2023-05-16 10:46 UTC #3 lore raw

RE: [PATCH 2/2] test/crypto: handle return code

Hi Anoob,

Quoted 10 lines

-----Original Message-----
From: Akhil Goyal <[email protected]>
Sent: Tuesday 16 May 2023 11:13
To: Anoob Joseph <[email protected]>; Fan Zhang <[email protected]>; Power, Ciara <[email protected]>
Cc: Hemant Agrawal <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; Tejasree Kondoj <[email protected]>; Ji, Kai <[email protected]>; [email protected]
Subject: RE: [PATCH 2/2] test/crypto: handle return code

Quoted 8 lines

Quoted 7 lines
Subject: [PATCH 2/2] test/crypto: handle return code
The sub test case, test_snow3g_decryption, may fail for non-critical reasons such as lack of support. Handle the return value gracefully to allow TEST_SKIPPED return value to be propagated correctly.
Signed-off-by Anoob Joseph <[email protected]>
Acked-by Akhil Goyal <[email protected]>

This looks like a duplicate of a patch recently merged: https://patches.dpdk.org/project/dpdk/patch/[email protected]/

Thanks,
Ciara

Anoob Joseph

author

2023-05-16 10:53 UTC #4 lore raw

RE: [PATCH 2/2] test/crypto: handle return code

Hi Ciara,

Quoted 1 line

This looks like a duplicate of a patch recently merged:

Indeed. Missed it.

@Akhil, we can abandon this patch.

Thanks,
Anoob

Quoted 47 lines

-----Original Message-----
From: Power, Ciara <[email protected]>
Sent: Tuesday, May 16, 2023 4:16 PM
To: Akhil Goyal <[email protected]>; Anoob Joseph <[email protected]>; Fan Zhang <[email protected]>
Cc: Hemant Agrawal <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; Tejasree Kondoj <[email protected]>; Ji, Kai <[email protected]>; [email protected]
Subject: [EXT] RE: [PATCH 2/2] test/crypto: handle return code
External Email
----------------------------------------------------------------------
Hi Anoob,
Quoted 6 lines
-----Original Message-----
From: Akhil Goyal <[email protected]>
Sent: Tuesday 16 May 2023 11:13
To: Anoob Joseph <[email protected]>; Fan Zhang <[email protected]>; Power, Ciara <[email protected]>
Cc: Hemant Agrawal <[email protected]>; Jerin Jacob
Kollanukkaran
Quoted 4 lines
<[email protected]>; Tejasree Kondoj <[email protected]>; Ji, Kai <[email protected]>; [email protected]
Subject: RE: [PATCH 2/2] test/crypto: handle return code
Quoted 8 lines
Quoted 7 lines
Subject: [PATCH 2/2] test/crypto: handle return code
The sub test case, test_snow3g_decryption, may fail for non-critical reasons such as lack of support. Handle the return value gracefully to allow TEST_SKIPPED return value to be propagated correctly.
Signed-off-by Anoob Joseph <[email protected]>
Acked-by Akhil Goyal <[email protected]>
This looks like a duplicate of a patch recently merged:
https://urldefense.proofpoint.com/v2/url?u=https-
3A__patches.dpdk.org_project_dpdk_patch_20230414135526.4271-2D1- 2Dsaoirse.odonovan- 40intel.com_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRSxy LWs2n6B- WYLn1v9SyTMrT5EQqh2TU&m=O_paIbxHa0g1eO0nnvRRDxs52JIZh_Xteso9I wXdLnFAYQyoPk4iryD2h4_uQRdB&s=QG45QCchVqKQyfhL8vENmY- JXiwsRS6zrbV3XxJoPTA&e=
Thanks,
Ciara

Akhil Goyal

Acked-by on v1 1/2

2023-05-16 10:06 UTC #5 lore raw

In reply to Anoob Joseph

Quoted 9 lines

Subject: [PATCH 1/2] test/crypto: free memory in error and skip paths
In multi session tests, multiple sessions get created. So the handling in ut_teardown won't guard against any memory that is not freed by the test case. Test case should free sessions as well as local memory that was used to save session pointers both in case of unsupported cases as well as operation failures.
Signed-off-by Anoob Joseph <[email protected]>

Acked-by Akhil Goyal <[email protected]>

Thanks for the cleanup

Anoob Joseph

author

2023-05-17 10:55 UTC #6 lore raw

PATCH v2 1/1 test/crypto: free memory in error and skip paths

In reply to Anoob Joseph

Signed-off-by Anoob Joseph <[email protected]>

---
v2:
* Moved 'ASSERT' to the end to allow cleanup in all cases.
---

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

app/test/test_cryptodev.c +43 ~~-16~~

index aaa2ce428d..77187e1fd3 100644
@@ -12753,8 +12753,8 @@ test_multi_session(void)
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
 	struct crypto_unittest_params *ut_params = &unittest_params;
 	struct rte_cryptodev_info dev_info;
+	int i, nb_sess, ret = TEST_SUCCESS;
 	void **sessions;
-	uint16_t i;
 
 	/* Verify the capabilities */
 	struct rte_cryptodev_sym_capability_idx cap_idx;

@@ -12784,21 +12784,25 @@ test_multi_session(void)
 		sessions[i] = rte_cryptodev_sym_session_create(
 			ts_params->valid_devs[0], &ut_params->auth_xform,
 				ts_params->session_mpool);
-		if (sessions[i] == NULL && rte_errno == ENOTSUP)
-			return TEST_SKIPPED;
+		if (sessions[i] == NULL && rte_errno == ENOTSUP) {
+			nb_sess = i;
+			ret = TEST_SKIPPED;
+			break;
+		}
 
 		TEST_ASSERT_NOT_NULL(sessions[i],
 				"Session creation failed at session number %u",
 				i);
+
 		/* Attempt to send a request on each session */
-		TEST_ASSERT_SUCCESS( test_AES_CBC_HMAC_SHA512_decrypt_perform(
+		ret = test_AES_CBC_HMAC_SHA512_decrypt_perform(
 			sessions[i],
 			ut_params,
 			ts_params,
 			catch_22_quote_2_512_bytes_AES_CBC_ciphertext,
 			catch_22_quote_2_512_bytes_AES_CBC_HMAC_SHA512_digest,
-			aes_cbc_iv),
-			"Failed to perform decrypt on request number %u.", i);
+			aes_cbc_iv);
+
 		/* free crypto operation structure */
 		rte_crypto_op_free(ut_params->op);

@@ -12817,16 +12821,26 @@ test_multi_session(void)
 			rte_pktmbuf_free(ut_params->ibuf);
 			ut_params->ibuf = 0;
 		}
+
+		if (ret != TEST_SUCCESS) {
+			i++;
+			break;
+		}
 	}
 
-	for (i = 0; i < MAX_NB_SESSIONS; i++) {
+	nb_sess = i;
+
+	for (i = 0; i < nb_sess; i++) {
 		rte_cryptodev_sym_session_free(ts_params->valid_devs[0],
 				sessions[i]);
 	}
 
 	rte_free(sessions);
 
-	return TEST_SUCCESS;
+	if (ret != TEST_SKIPPED)
+		TEST_ASSERT_SUCCESS(ret, "Failed to perform decrypt on request number %u.", i - 1);
+
+	return ret;
 }
 
 struct multi_session_params {

@@ -12845,8 +12859,9 @@ test_multi_session_random_usage(void)
 {
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
 	struct rte_cryptodev_info dev_info;
+	int index = 0, ret = TEST_SUCCESS;
+	uint32_t nb_sess, i, j;
 	void **sessions;
-	uint32_t i, j;
 	struct multi_session_params ut_paramz[] = {
 
 		{

@@ -12905,27 +12920,30 @@ test_multi_session_random_usage(void)
 				ts_params->valid_devs[0],
 				&ut_paramz[i].ut_params.auth_xform,
 				ts_params->session_mpool);
-		if (sessions[i] == NULL && rte_errno == ENOTSUP)
-			return TEST_SKIPPED;
+		if (sessions[i] == NULL && rte_errno == ENOTSUP) {
+			nb_sess = i;
+			ret = TEST_SKIPPED;
+			goto session_clear;
+		}
 
 		TEST_ASSERT_NOT_NULL(sessions[i],
 				"Session creation failed at session number %u",
 				i);
 	}
 
+	nb_sess = i;
+
 	srand(time(NULL));
 	for (i = 0; i < 40000; i++) {
 
 		j = rand() % MB_SESSION_NUMBER;
 
-		TEST_ASSERT_SUCCESS(
-			test_AES_CBC_HMAC_SHA512_decrypt_perform(
+		ret = test_AES_CBC_HMAC_SHA512_decrypt_perform(
 					sessions[j],
 					&ut_paramz[j].ut_params,
 					ts_params, ut_paramz[j].cipher,
 					ut_paramz[j].digest,
-					ut_paramz[j].iv),
-			"Failed to perform decrypt on request number %u.", i);
+					ut_paramz[j].iv);
 
 		rte_crypto_op_free(ut_paramz[j].ut_params.op);

@@ -12945,15 +12963,24 @@ test_multi_session_random_usage(void)
 			rte_pktmbuf_free(ut_paramz[j].ut_params.ibuf);
 			ut_paramz[j].ut_params.ibuf = 0;
 		}
+
+		if (ret != TEST_SKIPPED) {
+			index = i;
+			break;
+		}
 	}
 
-	for (i = 0; i < MB_SESSION_NUMBER; i++) {
+session_clear:
+	for (i = 0; i < nb_sess; i++) {
 		rte_cryptodev_sym_session_free(ts_params->valid_devs[0],
 				sessions[i]);
 	}
 
 	rte_free(sessions);
 
+	if (ret != TEST_SKIPPED)
+		TEST_ASSERT_SUCCESS(ret, "Failed to perform decrypt on request number %u.", index);
+
 	return TEST_SUCCESS;
 }

Signature · 1 line

2.25.1

Power, Ciara

Acked-by on v2 1/1

2023-05-17 15:45 UTC #7 lore raw

Quoted 23 lines

-----Original Message-----
From: Anoob Joseph <[email protected]>
Sent: Wednesday 17 May 2023 11:55
To: Akhil Goyal <[email protected]>; Fan Zhang <[email protected]>; Power, Ciara <[email protected]>
Cc: Hemant Agrawal <[email protected]>; Jerin Jacob <[email protected]>; Tejasree Kondoj <[email protected]>; [email protected]
Subject: [PATCH v2] test/crypto: free memory in error and skip paths
In multi session tests, multiple sessions get created. So the handling in ut_teardown won't guard against any memory that is not freed by the test case. Test case should free sessions as well as local memory that was used to save session pointers both in case of unsupported cases as well as operation failures.
Signed-off-by Anoob Joseph <[email protected]>
---
v2:
* Moved 'ASSERT' to the end to allow cleanup in all cases.
---

<snip>

Acked-by Ciara Power <[email protected]>

Akhil Goyal

Applied

2023-05-24 12:41 UTC #8 lore raw

In reply to Anoob Joseph

Quoted 8 lines

Subject: [PATCH v2] test/crypto: free memory in error and skip paths
In multi session tests, multiple sessions get created. So the handling in ut_teardown won't guard against any memory that is not freed by the test case. Test case should free sessions as well as local memory that was used to save session pointers both in case of unsupported cases as well as operation failures.

Quoted 4 lines

Signed-off-by Anoob Joseph <[email protected]>
---
v2:
* Moved 'ASSERT' to the end to allow cleanup in all cases.

Applied to dpdk-next-crypto
Thanks.