From: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>
To: Akhil Goyal <gakhil@marvell.com>,
"Power, Ciara" <ciara.power@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Cc: "david.marchand@redhat.com" <david.marchand@redhat.com>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
Anoob Joseph <anoobj@marvell.com>,
"Trahe, Fiona" <fiona.trahe@intel.com>,
"Doherty, Declan" <declan.doherty@intel.com>,
"matan@nvidia.com" <matan@nvidia.com>,
"g.singh@nxp.com" <g.singh@nxp.com>,
"jianjay.zhou@huawei.com" <jianjay.zhou@huawei.com>,
"asomalap@amd.com" <asomalap@amd.com>,
"ruifeng.wang@arm.com" <ruifeng.wang@arm.com>,
"Nicolau, Radu" <radu.nicolau@intel.com>,
"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
Nagadheeraj Rottela <rnagadheeraj@marvell.com>,
Ankur Dwivedi <adwivedi@marvell.com>,
"Wang, Haiyue" <haiyue.wang@intel.com>,
"jiawenwu@trustnetic.com" <jiawenwu@trustnetic.com>,
"jianwang@trustnetic.com" <jianwang@trustnetic.com>,
"Jerin Jacob Kollanukkaran" <jerinj@marvell.com>,
Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework
Date: Thu, 21 Oct 2021 08:43:38 +0000 [thread overview]
Message-ID: <MW5PR11MB580989F42F82ED2D3BB5BB39B8BF9@MW5PR11MB5809.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CO6PR18MB4484857C79B4BBF9253B29E6D8BE9@CO6PR18MB4484.namprd18.prod.outlook.com>
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, October 20, 2021 7:05 PM
> To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; thomas@monjalon.net; Zhang,
> Roy Fan <roy.fan.zhang@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Cc: david.marchand@redhat.com; hemant.agrawal@nxp.com; Anoob Joseph
> <anoobj@marvell.com>; Trahe, Fiona <fiona.trahe@intel.com>; Doherty,
> Declan <declan.doherty@intel.com>; matan@nvidia.com; g.singh@nxp.com;
> jianjay.zhou@huawei.com; asomalap@amd.com; ruifeng.wang@arm.com;
> Nicolau, Radu <radu.nicolau@intel.com>; ajit.khaparde@broadcom.com;
> Nagadheeraj Rottela <rnagadheeraj@marvell.com>; Ankur Dwivedi
> <adwivedi@marvell.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> jiawenwu@trustnetic.com; jianwang@trustnetic.com; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>
> Subject: RE: [PATCH v3 0/8] crypto/security session framework rework
>
> > > > I am seeing test failures for cryptodev_scheduler_autotest:
> > > > + Tests Total : 638
> > > > + Tests Skipped : 280
> > > > + Tests Executed : 638
> > > > + Tests Unsupported: 0
> > > > + Tests Passed : 18
> > > > + Tests Failed : 340
> > > >
> > > > The error showing for each testcase:
> > > > scheduler_pmd_sym_session_configure() line 487: unable to config
> sym
> > > > session
> > > > CRYPTODEV: rte_cryptodev_sym_session_init() line 1743: dev_id 2
> failed
> > to
> > > > configure session details
> > > >
> > > > I believe the problem happens in
> > scheduler_pmd_sym_session_configure.
> > > > The full sess object is no longer accessible in here, but it is required to
> be
> > > > passed to rte_cryptodev_sym_session_init.
> > > > The init function expects access to sess rather than the private data,
> and
> > > now
> > > > fails as a result.
> > > >
> > > > static int
> > > > scheduler_pmd_sym_session_configure(struct rte_cryptodev *dev,
> > > > struct rte_crypto_sym_xform *xform, void *sess,
> > > > rte_iova_t sess_iova __rte_unused)
> > > > {
> > > > struct scheduler_ctx *sched_ctx = dev->data->dev_private;
> > > > uint32_t i;
> > > > int ret;
> > > > for (i = 0; i < sched_ctx->nb_workers; i++) {
> > > > struct scheduler_worker *worker = &sched_ctx->workers[i];
> > > > ret = rte_cryptodev_sym_session_init(worker->dev_id, sess,
> > > > xform);
> > > > if (ret < 0) {
> > > > CR_SCHED_LOG(ERR, "unable to config sym session");
> > > > return ret;
> > > > }
> > > > }
> > > > return 0;
> > > > }
> > > >
> > > It looks like scheduler PMD is managing the stuff on its own for other
> > PMDs.
> > > The APIs are designed such that the app can call session_init multiple
> times
> > > With different dev_id on same sess.
> > > But here scheduler PMD internally want to configure other PMDs
> sess_priv
> > > By calling session_init.
> > >
> > > I wonder, why we have this 2 step session_create and session_init?
> > > Why can't we have it similar to security session create and let the
> scheduler
> > > PMD have its big session private data which can hold priv_data of as many
> > > PMDs
> > > as it want to schedule.
> > >
> > > Konstantin/Fan/Pablo what are your thoughts on this issue?
> > > Can we resolve this issue at priority in RC1(or probably RC2) for this
> release
> > > or
> > > else we defer it for next ABI break release?
> > >
> > > Thomas,
> > > Can we defer this for RC2? It does not seem to be fixed in 1 day.
> >
> > On another thought, this can be fixed with current patch also by having a
> big
> > session
> > Private data for scheduler PMD which is big enough to hold all other PMDs
> > data which
> > it want to schedule and then call the sess_configure function pointer of dev
> > directly.
> > What say? And this PMD change can be done in RC2. And this patchset go
> as
> > is in RC1.
> Here is the diff in scheduler PMD which should fix this issue in current
> patchset.
>
> diff --git a/drivers/crypto/scheduler/scheduler_pmd_ops.c
> b/drivers/crypto/scheduler/scheduler_pmd_ops.c
> index b92ffd6026..0611ea2c6a 100644
> --- a/drivers/crypto/scheduler/scheduler_pmd_ops.c
> +++ b/drivers/crypto/scheduler/scheduler_pmd_ops.c
> @@ -450,9 +450,8 @@ scheduler_pmd_qp_setup(struct rte_cryptodev *dev,
> uint16_t qp_id,
> }
>
> static uint32_t
> -scheduler_pmd_sym_session_get_size(struct rte_cryptodev *dev
> __rte_unused)
> +get_max_session_priv_size(struct scheduler_ctx *sched_ctx)
> {
> - struct scheduler_ctx *sched_ctx = dev->data->dev_private;
> uint8_t i = 0;
> uint32_t max_priv_sess_size = 0;
>
> @@ -469,20 +468,35 @@ scheduler_pmd_sym_session_get_size(struct
> rte_cryptodev *dev __rte_unused)
> return max_priv_sess_size;
> }
>
> +static uint32_t
> +scheduler_pmd_sym_session_get_size(struct rte_cryptodev *dev)
> +{
> + struct scheduler_ctx *sched_ctx = dev->data->dev_private;
> +
> + return get_max_session_priv_size(sched_ctx) * sched_ctx-
> >nb_workers;
> +}
> +
> static int
> scheduler_pmd_sym_session_configure(struct rte_cryptodev *dev,
> struct rte_crypto_sym_xform *xform, void *sess,
> rte_iova_t sess_iova __rte_unused)
> {
> struct scheduler_ctx *sched_ctx = dev->data->dev_private;
> + uint32_t worker_sess_priv_sz = get_max_session_priv_size(sched_ctx);
> uint32_t i;
> int ret;
>
> for (i = 0; i < sched_ctx->nb_workers; i++) {
> struct scheduler_worker *worker = &sched_ctx->workers[i];
> + struct rte_cryptodev *worker_dev =
> + rte_cryptodev_pmd_get_dev(worker->dev_id);
> + uint8_t index = worker_dev->driver_id;
>
> - ret = rte_cryptodev_sym_session_init(worker->dev_id, sess,
> - xform);
> + ret = worker_dev->dev_ops->sym_session_configure(
> + worker_dev,
> + xform,
> + (uint8_t *)sess + (index * worker_sess_priv_sz),
> + sess_iova + (index * worker_sess_priv_sz));
This won't work. This will make the session configuration finish successfully
but the private data the worker initialized is not the private data the worker
will use during enqueue/dequeue (workers only uses the session private
data based on its driver id).
> if (ret < 0) {
> CR_SCHED_LOG(ERR, "unable to config sym session");
> return ret;
next prev parent reply other threads:[~2021-10-21 8:43 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-30 14:50 [dpdk-dev] [PATCH 0/3] " Akhil Goyal
2021-09-30 14:50 ` [dpdk-dev] [PATCH 1/3] security: rework session framework Akhil Goyal
2021-09-30 14:50 ` [dpdk-dev] [PATCH 2/3] drivers/net: temporary disable ixgbe and txgbe Akhil Goyal
2021-10-12 12:26 ` Zhang, Roy Fan
2021-10-12 12:29 ` Akhil Goyal
2021-10-12 13:32 ` Zhang, Roy Fan
2021-09-30 14:50 ` [dpdk-dev] [PATCH 3/3] cryptodev: rework session framework Akhil Goyal
2021-10-01 15:53 ` Zhang, Roy Fan
2021-10-04 19:07 ` Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 0/7] crypto/security session framework rework Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 1/7] security: rework session framework Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 1/8] security: rework session framework Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 2/8] security: hide security session struct Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 3/8] net/cnxk: rework security session framework Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 4/8] security: pass session iova in PMD sess create Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 5/8] drivers/crypto: support security session get size op Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 6/8] cryptodev: rework session framework Akhil Goyal
2021-10-20 19:27 ` Ananyev, Konstantin
2021-10-21 6:53 ` Akhil Goyal
2021-10-21 10:38 ` Ananyev, Konstantin
2021-10-21 12:30 ` Akhil Goyal
2021-10-21 13:11 ` Ananyev, Konstantin
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 7/8] cryptodev: hide sym session structure Akhil Goyal
2021-10-18 21:34 ` [dpdk-dev] [PATCH v3 8/8] cryptodev: pass session iova in configure session Akhil Goyal
2021-10-20 14:36 ` [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework Hemant Agrawal
2021-10-20 15:45 ` Power, Ciara
2021-10-20 16:41 ` Akhil Goyal
2021-10-20 16:48 ` Akhil Goyal
2021-10-20 18:04 ` Akhil Goyal
2021-10-21 8:43 ` Zhang, Roy Fan [this message]
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 2/7] security: hide security session struct Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 3/7] net/cnxk: rework security session framework Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 4/7] security: pass session iova in PMD sess create Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 5/7] cryptodev: rework session framework Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 6/7] cryptodev: hide sym session structure Akhil Goyal
2021-10-13 19:22 ` [dpdk-dev] [PATCH v2 7/7] cryptodev: pass session iova in configure session Akhil Goyal
2021-10-14 11:47 ` [dpdk-dev] [PATCH v2 0/7] crypto/security session framework rework Akhil Goyal
2021-10-14 12:30 ` Zhang, Roy Fan
2021-10-14 12:34 ` Akhil Goyal
2021-10-14 17:07 ` Zhang, Roy Fan
2021-10-14 18:23 ` Akhil Goyal
2021-10-14 18:57 ` Akhil Goyal
2021-10-15 15:33 ` Zhang, Roy Fan
2021-10-15 17:42 ` Akhil Goyal
2021-10-15 18:47 ` Akhil Goyal
2021-10-16 13:31 ` Zhang, Roy Fan
2021-10-16 13:21 ` Zhang, Roy Fan
2021-10-15 8:12 ` Zhang, Roy Fan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MW5PR11MB580989F42F82ED2D3BB5BB39B8BF9@MW5PR11MB5809.namprd11.prod.outlook.com \
--to=roy.fan.zhang@intel.com \
--cc=adwivedi@marvell.com \
--cc=ajit.khaparde@broadcom.com \
--cc=anoobj@marvell.com \
--cc=asomalap@amd.com \
--cc=ciara.power@intel.com \
--cc=david.marchand@redhat.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=fiona.trahe@intel.com \
--cc=g.singh@nxp.com \
--cc=gakhil@marvell.com \
--cc=haiyue.wang@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerinj@marvell.com \
--cc=jianjay.zhou@huawei.com \
--cc=jianwang@trustnetic.com \
--cc=jiawenwu@trustnetic.com \
--cc=konstantin.ananyev@intel.com \
--cc=matan@nvidia.com \
--cc=ndabilpuram@marvell.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=radu.nicolau@intel.com \
--cc=rnagadheeraj@marvell.com \
--cc=ruifeng.wang@arm.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).