[dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto num qps

[Fiona Trahe]

ts_params->conf.nb_queue_pairs should not be hard coded with device
specific number. It should be retrieved from the device info.
Any test which changes it should restore it to orig value.

Also related cleanup of test code setting number and size of
queue-pairs on a device, e.g.
* Removed irrelevant “for” loop – was hardcoded to only loop once.
* Removed obsolete comment re inability to free and re-allocate queu memory
  and obsolete workaround for it which used to create maximum size queues.

And added freeing of ring memory on queue-pair release in aesni_mb PMD, 
else releasing and setting up queue-pair of a different size fails.

Signed-off-by: Akhil Goyal <akhil.goyal at nxp.com>
Signed-off-by: Fiona Trahe <fiona.trahe at intel.com>
---

v2:
  Fix for broken QAT PMD unit tests exposed by v1
  i.e. In test_device_configure_invalid_queue_pair_ids() after running tests 
  for invalid values restore original nb_queue_pairs.
  Also cleanup of test code setting number and size of queue-pairs on a device
  Also fix for aesni_mb PMD not freeing ring memory on qp release

 app/test/test_cryptodev.c                      | 54 ++++++++++----------------
 app/test/test_cryptodev_perf.c                 | 19 +--------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 10 ++++-
 3 files changed, 30 insertions(+), 53 deletions(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index d960d72..cbcfb3f 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -307,37 +307,27 @@ testsuite_setup(void)
 		return TEST_FAILED;
 
 	/* Set up all the qps on the first of the valid devices found */
-	for (i = 0; i < 1; i++) {
-		dev_id = ts_params->valid_devs[i];
+	dev_id = ts_params->valid_devs[0];
 
-		rte_cryptodev_info_get(dev_id, &info);
+	rte_cryptodev_info_get(dev_id, &info);
 
-		/*
-		 * Since we can't free and re-allocate queue memory always set
-		 * the queues on this device up to max size first so enough
-		 * memory is allocated for any later re-configures needed by
-		 * other tests
-		 */
-
-		ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
-		ts_params->conf.socket_id = SOCKET_ID_ANY;
-		ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
+	ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
+	ts_params->conf.socket_id = SOCKET_ID_ANY;
+	ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
 
-		TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id,
-				&ts_params->conf),
-				"Failed to configure cryptodev %u with %u qps",
-				dev_id, ts_params->conf.nb_queue_pairs);
+	TEST_ASSERT_SUCCESS(rte_cryptodev_configure(dev_id,
+			&ts_params->conf),
+			"Failed to configure cryptodev %u with %u qps",
+			dev_id, ts_params->conf.nb_queue_pairs);
 
-		ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
+	ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
 
-		for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
-			TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
-					dev_id, qp_id, &ts_params->qp_conf,
-					rte_cryptodev_socket_id(dev_id)),
-					"Failed to setup queue pair %u on "
-					"cryptodev %u",
-					qp_id, dev_id);
-		}
+	for (qp_id = 0; qp_id < info.max_nb_queue_pairs; qp_id++) {
+		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
+			dev_id, qp_id, &ts_params->qp_conf,
+			rte_cryptodev_socket_id(dev_id)),
+			"Failed to setup queue pair %u on cryptodev %u",
+			qp_id, dev_id);
 	}
 
 	return TEST_SUCCESS;
@@ -372,7 +362,6 @@ ut_setup(void)
 	memset(ut_params, 0, sizeof(*ut_params));
 
 	/* Reconfigure device to default parameters */
-	ts_params->conf.nb_queue_pairs = DEFAULT_NUM_QPS_PER_QAT_DEVICE;
 	ts_params->conf.socket_id = SOCKET_ID_ANY;
 	ts_params->conf.session_mp.nb_objs = DEFAULT_NUM_OPS_INFLIGHT;
 
@@ -381,12 +370,6 @@ ut_setup(void)
 			"Failed to configure cryptodev %u",
 			ts_params->valid_devs[0]);
 
-	/*
-	 * Now reconfigure queues to size we actually want to use in this
-	 * test suite.
-	 */
-	ts_params->qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
-
 	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,
@@ -396,7 +379,6 @@ ut_setup(void)
 			qp_id, ts_params->valid_devs[0]);
 	}
 
-
 	rte_cryptodev_stats_reset(ts_params->valid_devs[0]);
 
 	/* Start the device */
@@ -490,6 +472,7 @@ static int
 test_device_configure_invalid_queue_pair_ids(void)
 {
 	struct crypto_testsuite_params *ts_params = &testsuite_params;
+	uint16_t orig_nb_qps = ts_params->conf.nb_queue_pairs;
 
 	/* Stop the device in case it's started so it can be configured */
 	rte_cryptodev_stop(ts_params->valid_devs[0]);
@@ -544,6 +527,9 @@ test_device_configure_invalid_queue_pair_ids(void)
 			ts_params->valid_devs[0],
 			ts_params->conf.nb_queue_pairs);
 
+	/* revert to original testsuite value */
+	ts_params->conf.nb_queue_pairs = orig_nb_qps;
+
 	return TEST_SUCCESS;
 }
 
diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
index 6af0896..323995e 100644
--- a/app/test/test_cryptodev_perf.c
+++ b/app/test/test_cryptodev_perf.c
@@ -386,14 +386,12 @@ testsuite_setup(void)
 
 	/*
 	 * Using Crypto Device Id 0 by default.
-	 * Since we can't free and re-allocate queue memory always set the queues
-	 * on this device up to max size first so enough memory is allocated for
-	 * any later re-configures needed by other tests
+	 * Set up all the qps on this device.
 	 */
 
 	rte_cryptodev_info_get(ts_params->dev_id, &info);
 
-	ts_params->conf.nb_queue_pairs = DEFAULT_NUM_QPS_PER_QAT_DEVICE;
+	ts_params->conf.nb_queue_pairs = info.max_nb_queue_pairs;
 	ts_params->conf.socket_id = SOCKET_ID_ANY;
 	ts_params->conf.session_mp.nb_objs = info.sym.max_nb_sessions;
 
@@ -402,19 +400,6 @@ testsuite_setup(void)
 			"Failed to configure cryptodev %u",
 			ts_params->dev_id);
 
-
-	ts_params->qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT;
-
-	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs ; qp_id++) {
-		TEST_ASSERT_SUCCESS(rte_cryptodev_queue_pair_setup(
-			ts_params->dev_id, qp_id,
-			&ts_params->qp_conf,
-			rte_cryptodev_socket_id(ts_params->dev_id)),
-			"Failed to setup queue pair %u on cryptodev %u",
-			qp_id, ts_params->dev_id);
-	}
-
-	/*Now reconfigure queues to size we actually want to use in this testsuite.*/
 	ts_params->qp_conf.nb_descriptors = PERF_NUM_OPS_INFLIGHT;
 	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs ; qp_id++) {
 
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index d3c46ac..3d49e2a 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -311,8 +311,14 @@ aesni_mb_pmd_info_get(struct rte_cryptodev *dev,
 static int
 aesni_mb_pmd_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
 {
-	if (dev->data->queue_pairs[qp_id] != NULL) {
-		rte_free(dev->data->queue_pairs[qp_id]);
+	struct aesni_mb_qp *qp = dev->data->queue_pairs[qp_id];
+	struct rte_ring *r = NULL;
+
+	if (qp != NULL) {
+		r = rte_ring_lookup(qp->name);
+		if (r)
+			rte_ring_free(r);
+		rte_free(qp);
 		dev->data->queue_pairs[qp_id] = NULL;
 	}
 	return 0;
-- 
2.5.0