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

[Anoob Joseph <anoobj@marvell.com>]

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