Hi Pablo,
> -----Original Message-----
> From: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com>
> Sent: Thursday 15 September 2022 13:39
> To: Power, Ciara <ciara.po...@intel.com>; Zhang, Roy Fan
> <roy.fan.zh...@intel.com>
> Cc: dev@dpdk.org; Ji, Kai <kai...@intel.com>; Mrozowicz, SlawomirX
> <slawomirx.mrozow...@intel.com>
> Subject: RE: [PATCH v2 2/5] crypto/ipsec_mb: fix sessionless cleanup
>
> Hi Ciara,
>
> > -----Original Message-----
> > From: Power, Ciara <ciara.po...@intel.com>
> > Sent: Thursday, August 25, 2022 3:29 PM
> > To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.gua...@intel.com>
> > Cc: dev@dpdk.org; Ji, Kai <kai...@intel.com>; Power, Ciara
> > <ciara.po...@intel.com>; Mrozowicz, SlawomirX
> > <slawomirx.mrozow...@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.zh...@intel.com
> > Cc: slawomirx.mrozow...@intel.com
> > Cc: kai...@intel.com
> >
> > Signed-off-by: Ciara Power <ciara.po...@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.po...@intel.com/
Thanks for the review.
