[PATCH v2 2/5] crypto/ipsec_mb: fix sessionless cleanup

[Power, Ciara]

Hi Pablo,

> -----Original Message-----
> From: De Lara Guarch, Pablo <pablo.de.lara.guarch at intel.com>
> Sent: Thursday 15 September 2022 13:39
> To: Power, Ciara <ciara.power at intel.com>; Zhang, Roy Fan
> <roy.fan.zhang at intel.com>
> Cc: dev at dpdk.org; Ji, Kai <kai.ji at intel.com>; Mrozowicz, SlawomirX
> <slawomirx.mrozowicz at intel.com>
> Subject: RE: [PATCH v2 2/5] crypto/ipsec_mb: fix sessionless cleanup
> 
> Hi Ciara,
> 
> > -----Original Message-----
> > From: Power, Ciara <ciara.power at intel.com>
> > Sent: Thursday, August 25, 2022 3:29 PM
> > To: Zhang, Roy Fan <roy.fan.zhang at intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch at intel.com>
> > Cc: dev at dpdk.org; Ji, Kai <kai.ji at intel.com>; Power, Ciara
> > <ciara.power at intel.com>; Mrozowicz, SlawomirX
> > <slawomirx.mrozowicz at intel.com>
> > Subject: [PATCH v2 2/5] crypto/ipsec_mb: fix sessionless cleanup
> >
> > Currently, for a sessionless op, the session created is reset before
> > being put back into the mempool. This causes issues as the object
> > isn't correctly released into the mempool.
> >
> > Fixes: c68d7aa354f6 ("crypto/aesni_mb: use architecture independent
> > macros")
> > Fixes: b3bbd9e5f265 ("cryptodev: support device independent sessions")
> > Fixes: f16662885472 ("crypto/ipsec_mb: add chacha_poly PMD")
> > Cc: roy.fan.zhang at intel.com
> > Cc: slawomirx.mrozowicz at intel.com
> > Cc: kai.ji at intel.com
> >
> > Signed-off-by: Ciara Power <ciara.power at intel.com>
> > ---
> >  drivers/crypto/ipsec_mb/pmd_aesni_mb.c    | 4 ----
> >  drivers/crypto/ipsec_mb/pmd_chacha_poly.c | 4 ----
> >  drivers/crypto/ipsec_mb/pmd_kasumi.c      | 5 -----
> >  drivers/crypto/ipsec_mb/pmd_snow3g.c      | 4 ----
> >  drivers/crypto/ipsec_mb/pmd_zuc.c         | 4 ----
> >  5 files changed, 21 deletions(-)
> >
> > diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> > b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> > index 6d5d3ce8eb..944fce0261 100644
> > --- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> > +++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> > @@ -1770,10 +1770,6 @@ post_process_mb_job(struct ipsec_mb_qp *qp,
> > IMB_JOB *job)
> >
> >  	/* Free session if a session-less crypto op */
> >  	if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
> > -		memset(sess, 0, sizeof(struct aesni_mb_session));
> > -		memset(op->sym->session, 0,
> > -
> 
> This will leave some info leftover, so it may cause a problem if this object is
> reused? Is this memset clearing mempool object header and that's the reason
> why it cannot be released properly?
> Maybe Fan/Kai/Slawomir will know more on this.
[CP] 
Yes, I believe this would leave data leftover, my initial solution was incorrect.

I have sent a v3 fix which takes a different approach, after debugging the issue a little more.
I found the sessionless tests were reusing data in old session objects from previous session testcases,
which had not been reset before being put back into the mempool.
Once that reset was added, the sessionless tests failed due to session->nb_drivers being 0 - this was due
to the value never being set for sessionless operations. Instead of pulling from the mempool directly,
I added a call to sym_session_create(), which pulls from the mempool, and also sets values such as nb_drivers.

These changes can be seen here: https://patchwork.dpdk.org/project/dpdk/patch/20220921125036.9104-3-ciara.power@intel.com/

Thanks for the review.