Hi Tomasz, > -----Original Message----- > From: Tomasz Duszynski [mailto:t...@semihalf.com] > Sent: Wednesday, June 13, 2018 7:12 AM > To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com> > Cc: Tomasz Duszynski <t...@semihalf.com>; Doherty, Declan > <declan.dohe...@intel.com>; akhil.go...@nxp.com; ravi1.ku...@amd.com; > jerin.ja...@caviumnetworks.com; Zhang, Roy Fan <roy.fan.zh...@intel.com>; > Trahe, Fiona <fiona.tr...@intel.com>; jianjay.z...@huawei.com; > dev@dpdk.org > Subject: Re: [PATCH 3/6] cryptodev: remove max number of sessions > > On Tue, Jun 12, 2018 at 01:53:36PM +0000, De Lara Guarch, Pablo wrote: > > > > > > > -----Original Message----- > > > From: Tomasz Duszynski [mailto:t...@semihalf.com] > > > Sent: Tuesday, June 12, 2018 12:38 PM > > > To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com> > > > Cc: Doherty, Declan <declan.dohe...@intel.com>; akhil.go...@nxp.com; > > > ravi1.ku...@amd.com; jerin.ja...@caviumnetworks.com; Zhang, Roy Fan > > > <roy.fan.zh...@intel.com>; Trahe, Fiona <fiona.tr...@intel.com>; > > > t...@semihalf.com; jianjay.z...@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.gua...@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?
Ok I see, so actually the MUSDK library needs a maximum number of sessions. I'd say then we should keep this field, but we can add a special case: 0. In this case, the PMD does not have any maximum number of sessions (which would be applicable to most PMDs). So, for this PMD, this special case is not supported. If 0 is passed, either return that unlimited number of sessions is not supported, or set it to a default value (defined inside the PMD, such as 2048). If no value is passed, this number can be set to the default value too. How does this sound? Thanks, Pablo > > > > > 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
