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

[Akhil Goyal <gakhil@marvell.com>]

> > > 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));
                if (ret < 0) {
                        CR_SCHED_LOG(ERR, "unable to config sym session");
                        return ret;