Hi Anoob.
Thanks for this longer explanation.
I think can remove the test as intended.
Acked-by: Adam Dybkowski <adamx.dybkow...@intel.com>
Adam
> -----Original Message-----
> From: Anoob Joseph <ano...@marvell.com>
> Sent: Tuesday, 12 May, 2020 15:27
> To: Dybkowski, AdamX <adamx.dybkow...@intel.com>; Akhil Goyal
> <akhil.go...@nxp.com>; Doherty, Declan <declan.dohe...@intel.com>
> Cc: Narayana Prasad Raju Athreya <pathr...@marvell.com>; Ankur Dwivedi
> <adwiv...@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@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.dybkow...@intel.com>
> > Sent: Tuesday, May 12, 2020 6:37 PM
> > To: Anoob Joseph <ano...@marvell.com>; Akhil Goyal
> > <akhil.go...@nxp.com>; Doherty, Declan <declan.dohe...@intel.com>
> > Cc: Narayana Prasad Raju Athreya <pathr...@marvell.com>; Ankur
> Dwivedi
> > <adwiv...@marvell.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.gua...@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-boun...@dpdk.org> On Behalf Of Anoob Joseph
> > > Sent: Tuesday, 12 May, 2020 09:12
> > > To: Akhil Goyal <akhil.go...@nxp.com>; Doherty, Declan
> > > <declan.dohe...@intel.com>
> > > Cc: Anoob Joseph <ano...@marvell.com>; Narayana Prasad
> > > <pathr...@marvell.com>; Ankur Dwivedi <adwiv...@marvell.com>; De
> > > Lara Guarch, Pablo <pablo.de.lara.gua...@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 <ano...@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
