diff mbox

[dpdk-dev] app/test: Remove hard coding for nb_queue_pairs in test_cryptodev

Message ID 20160926163300.22990-2-akhil.goyal@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Pablo de Lara Guarch
Headers

Commit Message

Akhil Goyal Sept. 26, 2016, 4:32 p.m. UTC 
  From: Akhil Goyal <akhil.goyal@nxp.com>

nb_queue_pairs should not be hard coded with device specific number.
It should be retrieved from the device infos.
Also in ut_setup, ts_params->conf.nb_queue_pairs is already set in
testsuite_setup and we are not modifying it.

Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
 app/test/test_cryptodev.c      | 1 -
 app/test/test_cryptodev_perf.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

Comments

De Lara Guarch, Pablo Sept. 26, 2016, 7:36 p.m. UTC | #1 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> akhil.goyal@nxp.com
> Sent: Monday, September 26, 2016 9:33 AM
> To: dev@dpdk.org
> Cc: Akhil Goyal
> Subject: [dpdk-dev] [PATCH] app/test: Remove hard coding for
> nb_queue_pairs in test_cryptodev
> 
> From: Akhil Goyal <akhil.goyal@nxp.com>
> 
> nb_queue_pairs should not be hard coded with device specific number.
> It should be retrieved from the device infos.
> Also in ut_setup, ts_params->conf.nb_queue_pairs is already set in
> testsuite_setup and we are not modifying it.
> 
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Fiona Trahe Sept. 29, 2016, 2:12 p.m. UTC | #2 
Hi Ahkil

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Monday, September 26, 2016 8:37 PM
> To: akhil.goyal@nxp.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/test: Remove hard coding for
> nb_queue_pairs in test_cryptodev
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> > akhil.goyal@nxp.com
> > Sent: Monday, September 26, 2016 9:33 AM
> > To: dev@dpdk.org
> > Cc: Akhil Goyal
> > Subject: [dpdk-dev] [PATCH] app/test: Remove hard coding for
> > nb_queue_pairs in test_cryptodev
> >
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> >
> > nb_queue_pairs should not be hard coded with device specific number.
> > It should be retrieved from the device infos.
> > Also in ut_setup, ts_params->conf.nb_queue_pairs is already set in
> > testsuite_setup and we are not modifying it.
> >
> > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

The above code is correct, however it exposes a bug in QAT PMD unit tests.
And some cleanup needed for unnecessary qp setup code.
That cleanup then exposed a bug in aesni_mb PMD which prevents re-creating queue pairs of a different size.
 
I have a fix and cleanup patch ready. 
Just not sure how best to push it?
The original patch also needs rebasing, doesn't apply cleanly to the latest dpdk-next-crypto

Pablo should I push all as a reply to the first patch - waiting first for that to be rebased?
Or 
It would save Akhil a rebase and be simpler if I can include the original change in my patch and push all as a v2 superceding the original patch?  Is this possible?
Or 
should I Nack the original patch and push all instead?
Thomas Monjalon Sept. 29, 2016, 2:25 p.m. UTC | #3 
2016-09-29 14:12, Trahe, Fiona:
> > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > >
> > > nb_queue_pairs should not be hard coded with device specific number.
> > > It should be retrieved from the device infos.
> > > Also in ut_setup, ts_params->conf.nb_queue_pairs is already set in
> > > testsuite_setup and we are not modifying it.
> > >
> > > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > 
> > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> The above code is correct, however it exposes a bug in QAT PMD unit tests.
> And some cleanup needed for unnecessary qp setup code.
> That cleanup then exposed a bug in aesni_mb PMD which prevents re-creating queue pairs of a different size.
>  
> I have a fix and cleanup patch ready. 
> Just not sure how best to push it?
> The original patch also needs rebasing, doesn't apply cleanly to the latest dpdk-next-crypto
> 
> Pablo should I push all as a reply to the first patch - waiting first for that to be rebased?
> Or 
> It would save Akhil a rebase and be simpler if I can include the original change in my patch and push all as a v2 superceding the original patch?  Is this possible?
> Or 
> should I Nack the original patch and push all instead?

My preference goes to a v2.
De Lara Guarch, Pablo Sept. 29, 2016, 2:29 p.m. UTC | #4 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, September 29, 2016 7:25 AM
> To: Trahe, Fiona
> Cc: dev@dpdk.org; De Lara Guarch, Pablo; akhil.goyal@nxp.com
> Subject: Re: [dpdk-dev] [PATCH] app/test: Remove hard coding for
> nb_queue_pairs in test_cryptodev
> 
> 2016-09-29 14:12, Trahe, Fiona:
> > > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > >
> > > > nb_queue_pairs should not be hard coded with device specific number.
> > > > It should be retrieved from the device infos.
> > > > Also in ut_setup, ts_params->conf.nb_queue_pairs is already set in
> > > > testsuite_setup and we are not modifying it.
> > > >
> > > > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > >
> > > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >
> > The above code is correct, however it exposes a bug in QAT PMD unit tests.
> > And some cleanup needed for unnecessary qp setup code.
> > That cleanup then exposed a bug in aesni_mb PMD which prevents re-
> creating queue pairs of a different size.
> >
> > I have a fix and cleanup patch ready.
> > Just not sure how best to push it?
> > The original patch also needs rebasing, doesn't apply cleanly to the latest
> dpdk-next-crypto
> >
> > Pablo should I push all as a reply to the first patch - waiting first for that to
> be rebased?
> > Or
> > It would save Akhil a rebase and be simpler if I can include the original
> change in my patch and push all as a v2 superceding the original patch?  Is
> this possible?
> > Or
> > should I Nack the original patch and push all instead?
> 
> My preference goes to a v2.

Agree, send a v2, including your name and Akhil's. 

Thanks,
Pablo
diff mbox

Patch

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index 647787d..12b6b04 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -338,7 +338,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 =
 			(gbl_cryptodev_type == RTE_CRYPTODEV_QAT_SYM_PMD) ?
diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
index 2398d84..0ea7ec1 100644
--- a/app/test/test_cryptodev_perf.c
+++ b/app/test/test_cryptodev_perf.c
@@ -301,7 +301,7 @@  testsuite_setup(void)
 
 	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;