Re: [dpdk-dev] [PATCH v3 5/6] crypto/openssl: add asym crypto support

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Sunday, June 17, 2018 4:48 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com;
> dev@dpdk.org; Athreya, Narayana Prasad
> <NarayanaPrasad.Athreya@cavium.com>; Sahu, Sunila
> <Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>
> Subject: RE: [PATCH v3 5/6] crypto/openssl: add asym crypto support
> 
> Hi Pablo
> 
> >-----Original Message-----
> >From: De Lara Guarch, Pablo [mailto:pablo.de.lara.guarch@intel.com]
> >Sent: 17 June 2018 18:55
> >To: Verma, Shally <Shally.Verma@cavium.com>
> >Cc: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com;
> >dev@dpdk.org; Athreya, Narayana Prasad
> ><NarayanaPrasad.Athreya@cavium.com>; Sahu, Sunila
> ><Sunila.Sahu@cavium.com>; Gupta, Ashish <Ashish.Gupta@cavium.com>
> >Subject: RE: [PATCH v3 5/6] crypto/openssl: add asym crypto support
> >
> >External Email
> >
> >> -----Original Message-----
> >> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> >> Sent: Wednesday, May 16, 2018 7:05 AM
> >> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> >> Cc: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com;
> >> dev@dpdk.org; pathreya@caviumnetworks.com; Sunila Sahu
> >> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> >> <ashish.gupta@caviumnetworks.com>
> >> Subject: [PATCH v3 5/6] crypto/openssl: add asym crypto support
> >>
> >> Add asymmetric crypto operation support in openssl PMD.
> >> Current list of supported asym xforms:
> >> * RSA
> >> * DSA
> >> * Deffie-hellman
> >> * Modular Operations
> >>
> >> changes from v2:
> >> - Update the pmd capability as per new capability structure
> >>
> >> changes from v1:
> >> - resolve new line error in dod/guides/cryptodevs/openssl.rst
> >>
> >> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> >> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> >> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> >> ---
> .. [snip] ..
> 
> >> +;
> >> +; Supported Asymmetric algorithms of the 'openssl' crypto driver.
> >> +;
> >> +[Asymmetric]
> >> +RSA = Y
> >> +DSA = Y
> >> +Modular Exponentiation = Y
> >> +Modular Inversion = Y
> >> +Deffie-hellman = Y
> >
> >You need to add these parameters in default.ini file, otherwise they
> >are not shown when building the documentation.
> >
> [Shally] Ok
> 
> ..[snip]..
> 
> >>
> >> diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c
> >> b/drivers/crypto/openssl/rte_openssl_pmd.c
> >> index f584d0d6f..527e42773 100644
> >> --- a/drivers/crypto/openssl/rte_openssl_pmd.c
> >> +++ b/drivers/crypto/openssl/rte_openssl_pmd.c
> >
> >...
> >
> >> +             }
> >> +             dh_key->priv_key = BN_bin2bn(op->priv_key.data,
> >> +                                          op->priv_key.length,
> >> +                                          dh_key->priv_key);
> >
> >As on the previous patch, I am getting a compilation issue:
> >
> >drivers/crypto/openssl/rte_openssl_pmd.c:1711:9: error:
> >dereferencing pointer to incomplete type 'DH {aka struct dh_st}'
> >   dh_key->priv_key = BN_bin2bn(op->priv_key.data,
> >
> [Shally] as  I mentioned before Openssl PMD support 1.0.2 version
> series(http://doc.dpdk.org/guides/cryptodevs/openssl.html) and thus the error
> 
> >...
> >
> >> diff --git a/drivers/crypto/openssl/rte_openssl_pmd_ops.c
> >> b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
> >> index 1cb87d59a..76f7410cb 100644
> >> --- a/drivers/crypto/openssl/rte_openssl_pmd_ops.c
> >> +++ b/drivers/crypto/openssl/rte_openssl_pmd_ops.c
> >
> >...
> >
> >>  struct rte_cryptodev_ops openssl_pmd_ops = {
> >>               .dev_configure          = openssl_pmd_config,
> >>               .dev_start              = openssl_pmd_start,
> >> @@ -750,8 +1138,11 @@ struct rte_cryptodev_ops openssl_pmd_ops = {
> >>               .queue_pair_count       = openssl_pmd_qp_count,
> >>
> >>               .session_get_size       = openssl_pmd_session_get_size,
> >> +             .asym_session_get_size  =
> >> openssl_pmd_asym_session_get_size,
> >>               .session_configure      = openssl_pmd_session_configure,
> >> -             .session_clear          = openssl_pmd_session_clear
> >> +             .asym_session_configure =
> >> openssl_pmd_asym_session_configure,
> >> +             .session_clear          = openssl_pmd_session_clear,
> >> +             .asym_session_clear     = openssl_pmd_asym_session_clear
> >>  };
> >
> >I think session_get_size, session_configure and session_clear should be
> >renamed to sym_session_*, to avoid confusion. Could you add another patch
> renaming these?
> >
> [Shally] I see in latest cryptodev these typedefs are already named to
> cryptodev_sym_configure_session_t, _sym_get_session_private_size_t and
> _sym_free_.. so these changes would come while rebase.
> or you are referring to change openssl_pmd_session_configure to
> openssl_pmd_sym_session_configure and so others?

Yes, but the name is still session_configure/clear, not sym_session_configure/clear, right?
And also, I think we should rename the OpenSSL functions, openssl_pmd_session_*,
to openssl_pmd_sym_session_*.

Pablo

> 
> Thanks for review.
> 
> >Thanks,
> >Pablo
> >