diff mbox series

test/crypto: add cryptodev reconfig test

Message ID 20230405074117.2212923-1-asasidharan@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series test/crypto: add cryptodev reconfig test |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Aakash Sasidharan April 5, 2023, 7:41 a.m. UTC 
  Add cryptodev tests to verify that the device supports
reconfiguration any number of times via
rte_cryptodev_configure API.

Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
---
 app/test/test_cryptodev.c | 81 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

Comments

Zhang, Fan April 6, 2023, 2:21 p.m. UTC | #1 
Hi Aakash,

I really like the idea to create a test for device reconfiguration.

I am wondering if we need to test all PMDs against this patch first.

Regards,

Fan

On 4/5/2023 8:41 AM, Aakash Sasidharan wrote:
> Add cryptodev tests to verify that the device supports
> reconfiguration any number of times via
> rte_cryptodev_configure API.
>
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>   app/test/test_cryptodev.c | 81 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 81 insertions(+)
>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 9c670e9a35..3376ed91a3 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -12368,6 +12368,85 @@ test_stats(void)
>   	return TEST_SUCCESS;
>   }
>   
> +static int
> +test_device_reconfigure(void)
> +{
> +	struct crypto_testsuite_params *ts_params = &testsuite_params;
> +	uint16_t orig_nb_qps = ts_params->conf.nb_queue_pairs;
> +	struct rte_cryptodev_qp_conf qp_conf = {
> +		.nb_descriptors = MAX_NUM_OPS_INFLIGHT,
> +		.mp_session = ts_params->session_mpool
> +	};
> +	uint16_t qp_id, dev_id, num_devs = 0;
> +
> +	TEST_ASSERT((num_devs = rte_cryptodev_count()) >= 1,
> +			"Need at least %d devices for test", 1);
> +
> +	dev_id = ts_params->valid_devs[0];
> +
> +	/* Stop the device in case it's started so it can be configured */
> +	rte_cryptodev_stop(dev_id);
> +
> +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params->conf),
> +			"Failed test for rte_cryptodev_configure: "
> +			"dev_num %u", dev_id);
> +
> +	/* Reconfigure with same configure params */
> +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params->conf),
> +			"Failed test for rte_cryptodev_configure: "
> +			"dev_num %u", dev_id);
> +
> +	/* Reconfigure with just one queue pair */
> +	ts_params->conf.nb_queue_pairs = 1;
> +
> +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params->valid_devs[0],
> +			&ts_params->conf),
> +			"Failed to configure cryptodev: dev_id %u, qp_id %u",
> +			ts_params->valid_devs[0], ts_params->conf.nb_queue_pairs);
> +
> +	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> +		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
> +				ts_params->valid_devs[0], qp_id, &qp_conf,
> +				rte_cryptodev_socket_id(
> +						ts_params->valid_devs[0])),
> +				"Failed test for "
> +				"rte_cryptodev_queue_pair_setup: num_inflights "
> +				"%u on qp %u on cryptodev %u",
> +				qp_conf.nb_descriptors, qp_id,
> +				ts_params->valid_devs[0]);
> +	}
> +
> +	/* Reconfigure with max number of queue pairs */
> +	ts_params->conf.nb_queue_pairs = orig_nb_qps;
> +
> +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params->valid_devs[0],
> +			&ts_params->conf),
> +			"Failed to configure cryptodev: dev_id %u, qp_id %u",
> +			ts_params->valid_devs[0], ts_params->conf.nb_queue_pairs);
> +
> +	qp_conf.mp_session = ts_params->session_mpool;
> +
> +	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> +		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
> +				ts_params->valid_devs[0], qp_id, &qp_conf,
> +				rte_cryptodev_socket_id(
> +						ts_params->valid_devs[0])),
> +				"Failed test for "
> +				"rte_cryptodev_queue_pair_setup: num_inflights "
> +				"%u on qp %u on cryptodev %u",
> +				qp_conf.nb_descriptors, qp_id,
> +				ts_params->valid_devs[0]);
> +	}
> +
> +	/* Start the device */
> +	TEST_ASSERT_SUCCESS(rte_cryptodev_start(ts_params->valid_devs[0]),
> +			"Failed to start cryptodev %u",
> +			ts_params->valid_devs[0]);
> +
> +	/* Test expected values */
> +	return test_AES_CBC_HMAC_SHA1_encrypt_digest();
> +}
> +
>   static int MD5_HMAC_create_session(struct crypto_testsuite_params *ts_params,
>   				   struct crypto_unittest_params *ut_params,
>   				   enum rte_crypto_auth_operation op,
> @@ -16022,6 +16101,8 @@ static struct unit_test_suite cryptodev_gen_testsuite  = {
>   	.suite_name = "Crypto General Unit Test Suite",
>   	.setup = crypto_gen_testsuite_setup,
>   	.unit_test_cases = {
> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +				test_device_reconfigure),
>   		TEST_CASE_ST(ut_setup, ut_teardown,
>   				test_device_configure_invalid_dev_id),
>   		TEST_CASE_ST(ut_setup, ut_teardown,
Anoob Joseph April 10, 2023, 7:19 a.m. UTC | #2 
Hi Fan,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Zhang, Fan <fanzhang.oss@gmail.com>
> Sent: Thursday, April 6, 2023 7:51 PM
> To: Aakash Sasidharan <asasidharan@marvell.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Anoob Joseph
> <anoobj@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [PATCH] test/crypto: add cryptodev reconfig test
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Aakash,
> 
> I really like the idea to create a test for device reconfiguration.
> 
> I am wondering if we need to test all PMDs against this patch first.

[Anoob] We have tested the patch with Marvell PMDs, crypto_cn9k & crypto_cn10k as well as with openSSL PMD, cryptodev_openssl. We can wait for few days to see if any PMD owner has any objections. 

Can you review the patch and see if it aligns with DPDK spec?

> 
> Regards,
> 
> Fan
> 
> On 4/5/2023 8:41 AM, Aakash Sasidharan wrote:
> > Add cryptodev tests to verify that the device supports reconfiguration
> > any number of times via rte_cryptodev_configure API.
> >
> > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > ---
> >   app/test/test_cryptodev.c | 81
> +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 81 insertions(+)
> >
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > index 9c670e9a35..3376ed91a3 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -12368,6 +12368,85 @@ test_stats(void)
> >   	return TEST_SUCCESS;
> >   }
> >
> > +static int
> > +test_device_reconfigure(void)
> > +{
> > +	struct crypto_testsuite_params *ts_params = &testsuite_params;
> > +	uint16_t orig_nb_qps = ts_params->conf.nb_queue_pairs;
> > +	struct rte_cryptodev_qp_conf qp_conf = {
> > +		.nb_descriptors = MAX_NUM_OPS_INFLIGHT,
> > +		.mp_session = ts_params->session_mpool
> > +	};
> > +	uint16_t qp_id, dev_id, num_devs = 0;
> > +
> > +	TEST_ASSERT((num_devs = rte_cryptodev_count()) >= 1,
> > +			"Need at least %d devices for test", 1);
> > +
> > +	dev_id = ts_params->valid_devs[0];
> > +
> > +	/* Stop the device in case it's started so it can be configured */
> > +	rte_cryptodev_stop(dev_id);
> > +
> > +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params-
> >conf),
> > +			"Failed test for rte_cryptodev_configure: "
> > +			"dev_num %u", dev_id);
> > +
> > +	/* Reconfigure with same configure params */
> > +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params-
> >conf),
> > +			"Failed test for rte_cryptodev_configure: "
> > +			"dev_num %u", dev_id);
> > +
> > +	/* Reconfigure with just one queue pair */
> > +	ts_params->conf.nb_queue_pairs = 1;
> > +
> > +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params-
> >valid_devs[0],
> > +			&ts_params->conf),
> > +			"Failed to configure cryptodev: dev_id %u, qp_id %u",
> > +			ts_params->valid_devs[0], ts_params-
> >conf.nb_queue_pairs);
> > +
> > +	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > +		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
> > +				ts_params->valid_devs[0], qp_id, &qp_conf,
> > +				rte_cryptodev_socket_id(
> > +						ts_params->valid_devs[0])),
> > +				"Failed test for "
> > +				"rte_cryptodev_queue_pair_setup:
> num_inflights "
> > +				"%u on qp %u on cryptodev %u",
> > +				qp_conf.nb_descriptors, qp_id,
> > +				ts_params->valid_devs[0]);
> > +	}
> > +
> > +	/* Reconfigure with max number of queue pairs */
> > +	ts_params->conf.nb_queue_pairs = orig_nb_qps;
> > +
> > +	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params-
> >valid_devs[0],
> > +			&ts_params->conf),
> > +			"Failed to configure cryptodev: dev_id %u, qp_id %u",
> > +			ts_params->valid_devs[0], ts_params-
> >conf.nb_queue_pairs);
> > +
> > +	qp_conf.mp_session = ts_params->session_mpool;
> > +
> > +	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > +		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
> > +				ts_params->valid_devs[0], qp_id, &qp_conf,
> > +				rte_cryptodev_socket_id(
> > +						ts_params->valid_devs[0])),
> > +				"Failed test for "
> > +				"rte_cryptodev_queue_pair_setup:
> num_inflights "
> > +				"%u on qp %u on cryptodev %u",
> > +				qp_conf.nb_descriptors, qp_id,
> > +				ts_params->valid_devs[0]);
> > +	}
> > +
> > +	/* Start the device */
> > +	TEST_ASSERT_SUCCESS(rte_cryptodev_start(ts_params->valid_devs[0]),
> > +			"Failed to start cryptodev %u",
> > +			ts_params->valid_devs[0]);
> > +
> > +	/* Test expected values */
> > +	return test_AES_CBC_HMAC_SHA1_encrypt_digest();
> > +}
> > +
> >   static int MD5_HMAC_create_session(struct crypto_testsuite_params
> *ts_params,
> >   				   struct crypto_unittest_params *ut_params,
> >   				   enum rte_crypto_auth_operation op, @@ -
> 16022,6 +16101,8 @@
> > static struct unit_test_suite cryptodev_gen_testsuite  = {
> >   	.suite_name = "Crypto General Unit Test Suite",
> >   	.setup = crypto_gen_testsuite_setup,
> >   	.unit_test_cases = {
> > +		TEST_CASE_ST(ut_setup, ut_teardown,
> > +				test_device_reconfigure),
> >   		TEST_CASE_ST(ut_setup, ut_teardown,
> >   				test_device_configure_invalid_dev_id),
> >   		TEST_CASE_ST(ut_setup, ut_teardown,
Zhang, Fan April 18, 2023, 1:29 p.m. UTC | #3 
On 4/5/2023 8:41 AM, Aakash Sasidharan wrote:
> Add cryptodev tests to verify that the device supports
> reconfiguration any number of times via
> rte_cryptodev_configure API.
>
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---

Acked-by: Fan Zhang <fanzhang.oss@gmail.com>
O'Donovan, Saoirse April 19, 2023, 1:37 p.m. UTC | #4 
Hi Aakash,

> -----Original Message-----
> From: Aakash Sasidharan <asasidharan@marvell.com>
> Sent: Wednesday 5 April 2023 08:41
> To: Akhil Goyal <gakhil@marvell.com>; Fan Zhang
> <fanzhang.oss@gmail.com>
> Cc: jerinj@marvell.com; anoobj@marvell.com; dev@dpdk.org;
> asasidharan@marvell.com
> Subject: [PATCH] test/crypto: add cryptodev reconfig test
> 
> Add cryptodev tests to verify that the device supports reconfiguration any
> number of times via rte_cryptodev_configure API.
> 
> Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> ---
>  app/test/test_cryptodev.c | 81
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)

Tested on the following PMDs: QAT + SW Crypto PMDs 
(snow3g, null, chachapoly, kasumi, aesni_mb, aesni_gcm, zuc)
The test results for the testcase were a mix of skipped and passed, based on the capabilities of each PMD, no failures. 

Tested-by: Saoirse O'Donovan <saoirse.odonovan@intel.com>
Akhil Goyal May 3, 2023, 7:13 a.m. UTC | #5 
> Hi Aakash,
> > Subject: [PATCH] test/crypto: add cryptodev reconfig test
> >
> > Add cryptodev tests to verify that the device supports reconfiguration any
> > number of times via rte_cryptodev_configure API.
> >
> > Signed-off-by: Aakash Sasidharan <asasidharan@marvell.com>
> > ---
> >  app/test/test_cryptodev.c | 81
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> 
> Tested on the following PMDs: QAT + SW Crypto PMDs
> (snow3g, null, chachapoly, kasumi, aesni_mb, aesni_gcm, zuc)
> The test results for the testcase were a mix of skipped and passed, based on the
> capabilities of each PMD, no failures.
> 
> Tested-by: Saoirse O'Donovan <saoirse.odonovan@intel.com>

Applied to dpdk-next-crypto
Thanks.
diff mbox series

Patch

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 9c670e9a35..3376ed91a3 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -12368,6 +12368,85 @@  test_stats(void)
 	return TEST_SUCCESS;
 }
 
+static int
+test_device_reconfigure(void)
+{
+	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	uint16_t orig_nb_qps = ts_params->conf.nb_queue_pairs;
+	struct rte_cryptodev_qp_conf qp_conf = {
+		.nb_descriptors = MAX_NUM_OPS_INFLIGHT,
+		.mp_session = ts_params->session_mpool
+	};
+	uint16_t qp_id, dev_id, num_devs = 0;
+
+	TEST_ASSERT((num_devs = rte_cryptodev_count()) >= 1,
+			"Need at least %d devices for test", 1);
+
+	dev_id = ts_params->valid_devs[0];
+
+	/* Stop the device in case it's started so it can be configured */
+	rte_cryptodev_stop(dev_id);
+
+	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params->conf),
+			"Failed test for rte_cryptodev_configure: "
+			"dev_num %u", dev_id);
+
+	/* Reconfigure with same configure params */
+	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id, &ts_params->conf),
+			"Failed test for rte_cryptodev_configure: "
+			"dev_num %u", dev_id);
+
+	/* Reconfigure with just one queue pair */
+	ts_params->conf.nb_queue_pairs = 1;
+
+	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params->valid_devs[0],
+			&ts_params->conf),
+			"Failed to configure cryptodev: dev_id %u, qp_id %u",
+			ts_params->valid_devs[0], ts_params->conf.nb_queue_pairs);
+
+	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
+		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
+				ts_params->valid_devs[0], qp_id, &qp_conf,
+				rte_cryptodev_socket_id(
+						ts_params->valid_devs[0])),
+				"Failed test for "
+				"rte_cryptodev_queue_pair_setup: num_inflights "
+				"%u on qp %u on cryptodev %u",
+				qp_conf.nb_descriptors, qp_id,
+				ts_params->valid_devs[0]);
+	}
+
+	/* Reconfigure with max number of queue pairs */
+	ts_params->conf.nb_queue_pairs = orig_nb_qps;
+
+	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(ts_params->valid_devs[0],
+			&ts_params->conf),
+			"Failed to configure cryptodev: dev_id %u, qp_id %u",
+			ts_params->valid_devs[0], ts_params->conf.nb_queue_pairs);
+
+	qp_conf.mp_session = ts_params->session_mpool;
+
+	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
+		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
+				ts_params->valid_devs[0], qp_id, &qp_conf,
+				rte_cryptodev_socket_id(
+						ts_params->valid_devs[0])),
+				"Failed test for "
+				"rte_cryptodev_queue_pair_setup: num_inflights "
+				"%u on qp %u on cryptodev %u",
+				qp_conf.nb_descriptors, qp_id,
+				ts_params->valid_devs[0]);
+	}
+
+	/* Start the device */
+	TEST_ASSERT_SUCCESS(rte_cryptodev_start(ts_params->valid_devs[0]),
+			"Failed to start cryptodev %u",
+			ts_params->valid_devs[0]);
+
+	/* Test expected values */
+	return test_AES_CBC_HMAC_SHA1_encrypt_digest();
+}
+
 static int MD5_HMAC_create_session(struct crypto_testsuite_params *ts_params,
 				   struct crypto_unittest_params *ut_params,
 				   enum rte_crypto_auth_operation op,
@@ -16022,6 +16101,8 @@  static struct unit_test_suite cryptodev_gen_testsuite  = {
 	.suite_name = "Crypto General Unit Test Suite",
 	.setup = crypto_gen_testsuite_setup,
 	.unit_test_cases = {
+		TEST_CASE_ST(ut_setup, ut_teardown,
+				test_device_reconfigure),
 		TEST_CASE_ST(ut_setup, ut_teardown,
 				test_device_configure_invalid_dev_id),
 		TEST_CASE_ST(ut_setup, ut_teardown,