Re: [dpdk-dev] [PATCH] test/crypto: remove tests for unsupported descriptors

["Dybkowski, AdamX" <adamx.dybkowski@intel.com>]

Hi Anoob.

Thanks for this longer explanation.
I think can remove the test as intended.

Acked-by: Adam Dybkowski <adamx.dybkowski@intel.com>

Adam 

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Tuesday, 12 May, 2020 15:27
> To: Dybkowski, AdamX <adamx.dybkowski@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Ankur Dwivedi
> <adwivedi@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] test/crypto: remove tests for unsupported
> descriptors
> 
> Hi Adam,
> 
> The crypto library doesn't specify an upper limit for the nb_descriptors to be
> passed for queue_pair_setup. So technically all the values that you have
> passed as invalid is not correct.
> 
> I did check few PMDs when I saw this issue with OCTEON TX2 crypto PMD.
> QAT support max 4096 descriptors and so the invalids that is present is
> exactly following that. I checked DPAA drivers, but in that the entire conf is
> left unused. So I'm not sure whether DPAA passes these cases. I couldn't
> check the entire list that you have mentioned. But it doesn't much sense for
> s/w PMDs to limit max_nb_descriptors.
> 
> If we need to have an invalid check, we should probably add
> max_nb_descriptors in dev_info. And then we can have these tests. I'm open
> for that idea also. But I don't know whether that is allowed now or not. I
> leave that to Akhil.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Dybkowski, AdamX <adamx.dybkowski@intel.com>
> > Sent: Tuesday, May 12, 2020 6:37 PM
> > To: Anoob Joseph <anoobj@marvell.com>; Akhil Goyal
> > <akhil.goyal@nxp.com>; Doherty, Declan <declan.doherty@intel.com>
> > Cc: Narayana Prasad Raju Athreya <pathreya@marvell.com>; Ankur
> Dwivedi
> > <adwivedi@marvell.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> > Subject: [EXT] RE: [dpdk-dev] [PATCH] test/crypto: remove tests for
> > unsupported descriptors
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Hi Anoob.
> >
> > What's wrong with this test? Is this unit test failing on any PMD now?
> > I've checked on several (QAT, OpenSSL, SW ZUC, SW KASUMI, SW SNOW3G,
> > SCHEDULER) and it passes everywhere. Then why should we remove it
> > completely?
> >
> > Adam
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Anoob Joseph
> > > Sent: Tuesday, 12 May, 2020 09:12
> > > To: Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> > > <declan.doherty@intel.com>
> > > Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad
> > > <pathreya@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; De
> > > Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] test/crypto: remove tests for
> > > unsupported descriptors
> > >
> > > Cryptodev doesn't limit the number of descriptors that can be
> > > supported by the PMD.
> > >
> > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > ---
> > >  app/test/test_cryptodev.c | 45
> > > ---------------------------------------------
> > >  1 file changed, 45 deletions(-)
> > >
> > > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> > > index
> > > c624018..1ed2df8 100644
> > > --- a/app/test/test_cryptodev.c
> > > +++ b/app/test/test_cryptodev.c
> > > @@ -891,36 +891,6 @@ test_queue_pair_descriptor_setup(void)
> > >  				ts_params->valid_devs[0]);
> > >  	}
> > >
> > > -	/* invalid number of descriptors - max supported + 2 */
> > > -	qp_conf.nb_descriptors = MAX_NUM_OPS_INFLIGHT + 2;
> > > -
> > > -	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > > -		TEST_ASSERT_FAIL(rte_cryptodev_queue_pair_setup(
> > > -				ts_params->valid_devs[0], qp_id, &qp_conf,
> > > -				rte_cryptodev_socket_id(
> > > -						ts_params->valid_devs[0])),
> > > -				"Unexpectedly passed 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]);
> > > -	}
> > > -
> > > -	/* invalid number of descriptors - max value of parameter */
> > > -	qp_conf.nb_descriptors = UINT32_MAX-1;
> > > -
> > > -	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > > -		TEST_ASSERT_FAIL(rte_cryptodev_queue_pair_setup(
> > > -				ts_params->valid_devs[0], qp_id, &qp_conf,
> > > -				rte_cryptodev_socket_id(
> > > -						ts_params->valid_devs[0])),
> > > -				"Unexpectedly passed 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]);
> > > -	}
> > > -
> > >  	qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
> > >
> > >  	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > > @@ -935,21 +905,6 @@ test_queue_pair_descriptor_setup(void)
> > >  				ts_params->valid_devs[0]);
> > >  	}
> > >
> > > -	/* invalid number of descriptors - max supported + 1 */
> > > -	qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT + 1;
> > > -
> > > -	for (qp_id = 0; qp_id < ts_params->conf.nb_queue_pairs; qp_id++) {
> > > -		TEST_ASSERT_FAIL(rte_cryptodev_queue_pair_setup(
> > > -				ts_params->valid_devs[0], qp_id, &qp_conf,
> > > -				rte_cryptodev_socket_id(
> > > -						ts_params->valid_devs[0])),
> > > -				"Unexpectedly passed 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]);
> > > -	}
> > > -
> > >  	/* test invalid queue pair id */
> > >  	qp_conf.nb_descriptors = DEFAULT_NUM_OPS_INFLIGHT;
> > > 	/*valid */
> > >
> > > --
> > > 2.7.4