Hi Hemant,
> -----Original Message-----
> From: Hemant Agrawal [mailto:hemant.agra...@nxp.com]
> Sent: Wednesday, April 19, 2017 6:48 PM
> To: De Lara Guarch, Pablo; Akhil Goyal; dev@dpdk.org
> Cc: Doherty, Declan; Mcnamara, John
> Subject: RE: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> operation support
>
> Hi Pablo,
>
> > -----Original Message-----
> > From: De Lara Guarch, Pablo [mailto:pablo.de.lara.gua...@intel.com]
> > > -----Original Message-----
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of
> > > akhil.go...@nxp.com
> > > Sent: Wednesday, April 19, 2017 4:38 PM
> > > To: dev@dpdk.org
> > > Cc: Doherty, Declan; Mcnamara, John; hemant.agra...@nxp.com
> > > Subject: [dpdk-dev] [PATCH v8 11/13] crypto/dpaa2_sec: add crypto
> > > operation support
> > >
> > > From: Akhil Goyal <akhil.go...@nxp.com>
> > >
> > > Signed-off-by: Akhil Goyal <akhil.go...@nxp.com>
> > > Signed-off-by: Hemant Agrawal <hemant.agra...@nxp.com>
> > > ---
> > > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 1236
> > > +++++++++++++++++++++++++++
> > > drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h | 143 ++++
> > > 2 files changed, 1379 insertions(+)
> > >
> > > diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > index e0e8cfb..7c497c0 100644
> > > --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> > > +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
> >
> > ...
> >
> > > +/** Clear the memory of session so it doesn't leave key material
> > > +behind */ static void dpaa2_sec_session_clear(struct rte_cryptodev
> > > +*dev __rte_unused, void
> > > *sess)
> > > +{
> > > + PMD_INIT_FUNC_TRACE();
> > > + dpaa2_sec_session *s = (dpaa2_sec_session *)sess;
> > > +
> > > + if (s) {
> > > + if (s->ctxt)
> > > + rte_free(s->ctxt);
> > > + if (&s->cipher_key)
> > > + rte_free(s->cipher_key.data);
> > > + if (&s->auth_key)
> > > + rte_free(s->auth_key.data);
> >
> > No need for these checks, rte_free can handle NULL pointers (assuming
> that the
> > structure is initialized to all 0s when created, which looks like it is
> happening
> > below).
> >
> > Unless there are other changes required (I am currently reviewing the
> patchset),
> > I can make this and the change from the other email myself, when
> applying the
> > patchset.
>
> [Hemant] No, we are not expecting other changes.
>
> If you want, I can send the new patchset or you can make the changes -
> either way is fine with us.
> (2nd is preferred 😊)
There are other issues with this patchset.
1 - There are two functions that are not being used:
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:77:1:
error: unused function 'print_fd' [-Werror,-Wunused-function]
print_fd(const struct qbman_fd *fd)
^
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:91:1:
error: unused function 'print_fle' [-Werror,-Wunused-function]
print_fle(const struct qbman_fle *fle)
^
2 - When enabling CONFIG_RTE_LIBRTE_DPAA2_SEC_DEBUG_RX, I see the following
errors
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:554:6:
error: 'bpid_info' undeclared (first use in this function)
bpid_info[DPAA2_GET_FD_BPID(fd)].meta_data_size,
^
/root/dpdk-next-crypto-nxp/build/include/rte_log.h:334:32: note: in definition
of macro 'RTE_LOG'
RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
^~~~~~~~~~~
/root/dpdk-next-crypto-nxp/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c:551:2:
note: in expansion of macro 'PMD_RX_LOG'
PMD_RX_LOG(DEBUG, "fdaddr =%p bpid =%d meta =%d off =%d, len =%d",
^~~~~~~~~~
So, I think these errors deserve a v9, sorry I just spotted them.
Pablo
> >
> > Thanks,
> > Pablo
