Hi,
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Friday, October 15, 2021 7:47 PM
> To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; dev@dpdk.org
> Cc: tho...@monjalon.net; david.march...@redhat.com;
> hemant.agra...@nxp.com; Anoob Joseph <ano...@marvell.com>; De Lara
> Guarch, Pablo <pablo.de.lara.gua...@intel.com>; Trahe, Fiona
> <fiona.tr...@intel.com>; Doherty, Declan <declan.dohe...@intel.com>;
> ma...@nvidia.com; g.si...@nxp.com; jianjay.z...@huawei.com;
> asoma...@amd.com; ruifeng.w...@arm.com; Ananyev, Konstantin
> <konstantin.anan...@intel.com>; Nicolau, Radu <radu.nico...@intel.com>;
> ajit.khapa...@broadcom.com; Nagadheeraj Rottela
> <rnagadhee...@marvell.com>; Ankur Dwivedi <adwiv...@marvell.com>;
> Power, Ciara <ciara.po...@intel.com>; Wang, Haiyue
> <haiyue.w...@intel.com>; jiawe...@trustnetic.com;
> jianw...@trustnetic.com
> Subject: RE: [PATCH v2 0/7] crypto/security session framework rework
>
> > > Hi Akhil,
> > >
> > > I tried to fix the problems of seg faults.
> > > The seg-faults are gone now but all asym tests are failing too.
> > > The reason is the rte_cryptodev_queue_pair_setup() checks the session
> > > mempool same for sym and asym.
> > > Since we don't have a rte_cryptodev_asym_session_pool_create() the
> > > session mempool created by
> > > test_cryptodev_asym.c with rte_mempool_create() will fail the
> mempool
> > > check when setting up the queue pair.
> > >
> > > If you think my fix may be useful (although not resolving asym issue) I
> > > can
> > > send it.
> > >
> > Is it a different fix than what I proposed below? If yes, you can send the
> diff.
> > I already made the below changes for all the PMDs.
> > I will try to fix the asym issue, but I suppose it can be dealt in the app
> > Which can be fixed separately in RC2.
> >
> > Also, found the root cause of multi process issue, working on making the
> > patches.
> > Will send v3 soon with all 3 issues(docsis/mp/sessless) fixed atleast.
> > For Asym, may send a separate patch.
> >
> For Asym issue, it looks like the APIs are not written properly and has many
> Issues compared to sym.
> Looking at the API rte_cryptodev_queue_pair_setup(), it only support
> mp_session(or priv_sess_mp) for symmetric sessions even without my
> changes.
>
> Hence, a qp does not have mempool for sessionless Asym processing and
> looking at current
> Drivers, only QAT support asym session less and it does not use mempool
> stored in qp.
>
> Hence IMO, it is safe to remove the check from
> rte_cryptodev_queue_pair_setup()
> if (!qp_conf->mp_session) {
> CDEV_LOG_ERR("Invalid mempools\n");
> return -EINVAL;
> }
> Or we can have give a CDEV_LOG_INFO (to indicate session mempool not
> present, session less won't work) instead of CDEV_LOG_ERR and fall through.
>
Yes this is a valid fix. It will make queue pair setup work as before.
The old code was like this:
if ((qp_conf->mp_session && !qp_conf->mp_session_private) ||
(!qp_conf->mp_session && qp_conf->mp_session_private)) {
CDEV_LOG_ERR("Invalid mempools\n");
return -EINVAL;
}
The requirement was either you provide 2 mempools (one for session and one for
session private) or you don't provide session mempool when creating queue pair
at
all. Only otherwise the error is returned.
> For sym case, it is checking again in next line if session_mp is there or not.
>
> I hope, the asym cases will work once we remove the above check and pass
> Null in the asym app while setting up queue pairs. What say?
It shall be working. Thanks.
>
>
>
