From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
by dpdk.org (Postfix) with ESMTP id 4D8065B16
for <dev@dpdk.org>; Tue, 15 Jan 2019 13:49:33 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga002.fm.intel.com ([10.253.24.26])
by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
15 Jan 2019 04:49:32 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.56,481,1539673200"; d="scan'208";a="134765719"
Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96])
by fmsmga002.fm.intel.com with ESMTP; 15 Jan 2019 04:49:31 -0800
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.116]) by
IRSMSX154.ger.corp.intel.com ([169.254.12.253]) with mapi id 14.03.0415.000;
Tue, 15 Jan 2019 12:49:30 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Iremonger, Bernard" <bernard.iremonger@intel.com>, "dev@dpdk.org"
<dev@dpdk.org>
Thread-Topic: [PATCH] test/ipsec: fix test suite setup function
Thread-Index: AQHUrMRNVcZQp3tOgkm/NjnU7fqUBKWwNbjggAAN8gCAAAJQYA==
Date: Tue, 15 Jan 2019 12:49:29 +0000
Message-ID: <2601191342CEEE43887BDE71AB977258010D904263@irsmsx105.ger.corp.intel.com>
References: <1547551191-28863-1-git-send-email-bernard.iremonger@intel.com>
<2601191342CEEE43887BDE71AB977258010D90422F@irsmsx105.ger.corp.intel.com>
<8CEF83825BEC744B83065625E567D7C260D4ECBD@IRSMSX108.ger.corp.intel.com>
In-Reply-To: <8CEF83825BEC744B83065625E567D7C260D4ECBD@IRSMSX108.ger.corp.intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjFhNjIyMWMtNDBkYS00YWNjLWI2OTEtZTNmNzE1YTZmNzgyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYnJVeXBMNXVWRjQ3Mmp5UGpkd3d1YSt3QU9EVmpOclpad3RCS3EyK1dLcjZEYUthYitSQlR6MFpmelM4Y05hNCJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH] test/ipsec: fix test suite setup function
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 15 Jan 2019 12:49:33 -0000
> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, January 15, 2019 12:34 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: RE: [PATCH] test/ipsec: fix test suite setup function
>=20
> Hi Konstantin
>=20
> <snip>
>=20
> > > Subject: [PATCH] test/ipsec: fix test suite setup function
> > >
> > > Check for valid crypto_null devices before continuing.
> > >
> > > Fixes: 05fe65eb66b2 ("test/ipsec: introduce functional test")
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > > ---
> > > test/test/test_ipsec.c | 17 +++++++++++++++--
> > > 1 file changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/test/test/test_ipsec.c b/test/test/test_ipsec.c index
> > > ff1a1c4..4dfc55b 100644
> > > --- a/test/test/test_ipsec.c
> > > +++ b/test/test/test_ipsec.c
> > > @@ -46,6 +46,8 @@
> > > #define BURST_SIZE 32
> > > #define REORDER_PKTS 1
> > >
> > > +static int gbl_driver_id;
> > > +
> >
> > Why do you need that global here?
>=20
> test_ipsec.c is based on test_cryptodev.c.
> gbl_driver_id used to store the ID of the required driver.
Sorry but referencing someone else code is not an answer.
Why do *you* need it *here*?=20
>=20
> >
> > > struct user_params {
> > > enum rte_crypto_sym_xform_type auth;
> > > enum rte_crypto_sym_xform_type cipher; @@ -218,7 +220,7 @@
> > > testsuite_setup(void) {
> > > struct ipsec_testsuite_params *ts_params =3D &testsuite_params;
> > > struct rte_cryptodev_info info;
> > > - uint32_t nb_devs, dev_id;
> > > + uint32_t i, nb_devs, dev_id;
> > > size_t sess_sz;
> > >
> > > memset(ts_params, 0, sizeof(*ts_params)); @@ -251,7 +253,18 @@
> > > testsuite_setup(void)
> > > return TEST_FAILED;
> > > }
> > >
> > > - ts_params->valid_devs[ts_params->valid_dev_count++] =3D 0;
> > > + gbl_driver_id =3D rte_cryptodev_driver_id_get(
> > > + RTE_STR(CRYPTODEV_NAME_NULL_PMD));
>=20
> These tests only work with the crypto_null PMD's, gbl_driver_id is set =
to the crypto_null PMD id here.
>=20
> > > +
> > > + /* Create list of valid crypto devs */
> > > + for (i =3D 0; i < nb_devs; i++) {
> > > + rte_cryptodev_info_get(i, &info);
> > > + if (info.driver_id =3D=3D gbl_driver_id)
> > > + ts_params->valid_devs[ts_params->valid_dev_count++]
> > =3D i;
> > > + }
> >
> > I think you need to check driver capabilities, instead of relying on dr=
iver name.
>=20
> I don't think it is necessary to check the driver capabilities.
I still think that the valid way to check supported algorithms is to check =
device capabilities,
not the driver name.
> This is how it is done in test_cryptodev.c.
> I think it makes sense to mirror the test_cryptodev.c implementation.
>=20
> > > +
> > > + if (ts_params->valid_dev_count < 1)
> > > + return TEST_FAILED;
> > >
> > > /* Set up all the qps on the first of the valid devices found */
> > > dev_id =3D ts_params->valid_devs[0];
> >
> > If we always use just valid_dev[0] to determine private session size, w=
hy do you
> > keep going though all devs in the loop above?
>=20
> There may be several crypto devs present for example, crypto_aesni_mb0, c=
rypto_aseni_mb1, crypto_null0 and crypto_null1.
Yes.
> The valid_dev[] array will contain all devs of the requested type, in thi=
s case crypto_null0 and crypto_null1.
But we need/use only one.
>=20
> > Another thing, as I mentioned off-line - later you still use all vald_d=
evs[] to init
> > session:
> > s =3D rte_cryptodev_sym_session_create(qp->mp_session);
> > if (s =3D=3D NULL)
> > return -ENOMEM;
> >
> > /* initiliaze SA crypto session for all supported devices */
> > for (i =3D 0; i !=3D devnum; i++) {
> > rc =3D rte_cryptodev_sym_session_init(devid[i], s,
> > ut->crypto_xforms, qp->mp_session_private);
> > if (rc !=3D 0)
> > break;
> > }
> >
> > I think we need either to determine max private session size based on *=
all*
> > valid_devs[], or just use one device that can do NULL algorithm.
>=20
> The valid_devs[] array only contains crypto_null PMD's
> The code is using the crypto_null PMD only.
In fact there is no reason to be crypto_null only.
I think it could be any crypto-dev that does support NULL auth/cipher.
>=20
> > As we always enqueue/dequeuer into valid_devs[0] - I think there is no =
point to
> > have an arrays here, just single valid_dev should be sufficient.
>=20
> The test program may be started with several crypto_dev PMD's for example=
:
>=20
> test -c f -n 4 --vdev crypto_aesni_mb0 --vdev crypto_null0 --vdev crypto_=
aesni_mb1 --vdev crypto_dev_null1
>=20
> In this case the valid_devs[] array will contain crypto_dev_null0 and cry=
pto_dev_null1.
>=20
> > Konstantin
> >
> >
>=20
> Regards,
>=20
> Bernard.