Re: [dpdk-dev] [PATCH 3/6] cryptodev: remove max number of sessions

[Tomasz Duszynski <tdu@semihalf.com>]

On Tue, Jun 12, 2018 at 01:53:36PM +0000, De Lara Guarch, Pablo wrote:
>
>
> > -----Original Message-----
> > From: Tomasz Duszynski [mailto:tdu@semihalf.com]
> > Sent: Tuesday, June 12, 2018 12:38 PM
> > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> > Cc: Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com;
> > ravi1.kumar@amd.com; jerin.jacob@caviumnetworks.com; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>;
> > tdu@semihalf.com; jianjay.zhou@huawei.com; dev@dpdk.org
> > Subject: Re: [PATCH 3/6] cryptodev: remove max number of sessions
> >
> > Hello Pablo,
> >
> > On Fri, Jun 08, 2018 at 11:02:31PM +0100, Pablo de Lara wrote:
> > > Sessions are not created and stored in the crypto device
> > > anymore, since now the session mempool is created
> > > at the application level.
> > >
> > > Therefore the limitation of the maximum number of sessions
> > > that can be created should not be dependent of the crypto device.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>
> ...
>
> > > diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> > b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> > > index 1b6029a56..822b6cac7 100644
> > > --- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> > > +++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> > > @@ -719,7 +719,6 @@ cryptodev_mrvl_crypto_create(const char *name,
> > >  	internals = dev->data->dev_private;
> > >
> > >  	internals->max_nb_qpairs = init_params->max_nb_queue_pairs;
> > > -	internals->max_nb_sessions = init_params->max_nb_sessions;
> > >
> > >  	/*
> > >  	 * ret == -EEXIST is correct, it means DMA
> > > @@ -734,8 +733,6 @@ cryptodev_mrvl_crypto_create(const char *name,
> > >  			"DMA memory has been already initialized by a
> > different driver.");
> > >  	}
> > >
> > > -	sam_params.max_num_sessions = internals->max_nb_sessions;
> >
> > This will not fly since library maintains separate list of sessions.
> > We have to initialize this number to something sane. Since we cannot
> > get it from userspace perhaps make that compile-time configurable
> > by adding separate CONFIG_?
>
> Hi Tomasz,
>
> If you need to have an actual limit, you could define it internally
> (not adding an external configuration option), but bear in mind that
> This won't prevent an application from trying to allocate more sessions.

You can define arbitrary number of session on condition you have enough
memory. So no hard limit here. What bothers me is the case where
app wants to initialize more session than the library internally has.
If this happens userspace will get an error. On the other hand
requesting some arbitrary large number of session from library and
hoping app will never use so many wastes memory (which might be valuable
on resource constrained systems).

That is why keeping the number of sessions in app and library in sync
is important.

Do we have any option in DPDK now to workaround this?

>
> If your PMD has a limitation on the maximum number of sessions, then maybe this change
> won't work for you (removing the maximum number of sessions), so let me know and we can discuss this.
>
> Thanks,
> Pablo
>
> P.S. Please, next time, strip out the code that you are not commenting, as it was hard to find this question :)
>

--
- Tomasz Duszyński