Hi,
> -----Original Message-----
> From: Akhil Goyal <[email protected]>
> Sent: Friday, October 15, 2021 7:47 PM
> To: Zhang, Roy Fan <[email protected]>; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; Anoob Joseph <[email protected]>; De Lara
> Guarch, Pablo <[email protected]>; Trahe, Fiona
> <[email protected]>; Doherty, Declan <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Ananyev, Konstantin
> <[email protected]>; Nicolau, Radu <[email protected]>;
> [email protected]; Nagadheeraj Rottela
> <[email protected]>; Ankur Dwivedi <[email protected]>;
> Power, Ciara <[email protected]>; Wang, Haiyue
> <[email protected]>; [email protected];
> [email protected]
> 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.
>
>
>
