Re: [dpdk-dev] [PATCH v3 0/8] crypto/security session framework rework

[Akhil Goyal <gakhil@marvell.com>]

> > Hi Akhil,
> >
> > >Subject: [PATCH v3 0/8] crypto/security session framework rework
> > >
> > >As discussed in last release deprecation notice, crypto and security session
> > >framework are reworked to reduce the need of two mempool objects and
> > >remove the requirement to expose the rte_security_session and
> > >rte_cryptodev_sym_session structures.
> > >Design methodology is explained in the patch description.
> > >
> > >Similar work will need to be done for asymmetric sessions as well.
> > Asymmetric
> > >session need another rework and is postponed to next release. Since it is
> > still
> > >in experimental stage, we can modify the APIs in next release as well.
> > >
> > >The patches are compilable with all affected PMDs and tested with dpdk-
> > test
> > >and test-crypto-perf app on CN9k platform.
> > <snip>
> >
> > 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.