[dpdk-dev,v2] app/test: remove hard-coding of crypto num qps
Commit Message
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@nxp.com>
Signed-off-by: Fiona Trahe <fiona.trahe@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(-)
Comments
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Thursday, September 29, 2016 10:18 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Trahe, Fiona; Akhil Goyal
> Subject: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto num
> qps
>
> 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@nxp.com>
> Signed-off-by: Fiona Trahe <fiona.trahe@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
Sorry, I missed this patch. Could you split this patch into different patches?
It looks like you are making (three?) changes in different places.
Thanks,
Pablo
> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, October 5, 2016 1:50 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: RE: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto
> num qps
>
> Hi,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> > Sent: Thursday, September 29, 2016 10:18 AM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo; Trahe, Fiona; Akhil Goyal
> > Subject: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto
> > num qps
> >
> > 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@nxp.com>
> > Signed-off-by: Fiona Trahe <fiona.trahe@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
>
> Sorry, I missed this patch. Could you split this patch into different patches?
> It looks like you are making (three?) changes in different places.
>
> Thanks,
> Pablo
Ok, will do,
Fiona
@@ -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;
}
@@ -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++) {
@@ -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;