HI Fiona Thanks for review. I was just about to send lib patch v4. So for now, I couldn't consider all input but some of them. But , in any case, this version is experimental so it is open for further feedback after 1st version go in.
Rest, please see my feedback inline. >-----Original Message----- >From: Trahe, Fiona [mailto:fiona.tr...@intel.com] >Sent: 03 July 2018 19:42 >To: Verma, Shally <shally.ve...@cavium.com>; De Lara Guarch, Pablo ><pablo.de.lara.gua...@intel.com> >Cc: akhil.go...@nxp.com; dev@dpdk.org; Athreya, Narayana Prasad ><narayanaprasad.athr...@cavium.com>; Sahu, Sunila ><sunila.s...@cavium.com>; Gupta, Ashish <ashish.gu...@cavium.com>; Trahe, >Fiona <fiona.tr...@intel.com> >Subject: RE: [PATCH v3 3/6] lib/cryptodev: add asymmetric crypto capability in >cryptodev > >External Email > >Hi Shally, > >> -----Original Message----- >> From: Shally Verma [mailto:shally.ve...@caviumnetworks.com] >> Sent: Wednesday, May 16, 2018 7:05 AM >> To: De Lara Guarch, Pablo <pablo.de.lara.gua...@intel.com> >> Cc: Trahe, Fiona <fiona.tr...@intel.com>; akhil.go...@nxp.com; dev@dpdk.org; >> pathr...@caviumnetworks.com; Sunila Sahu <sunila.s...@caviumnetworks.com>; >> Ashish Gupta >> <ashish.gu...@caviumnetworks.com> >> Subject: [PATCH v3 3/6] lib/cryptodev: add asymmetric crypto capability in >> cryptodev >> >> Extend cryptodev with asymmetric capability APIs and >> definitions. >> >> changes from v2: >> - remove redundant xform_type from asym capability struct >> - rename rte_cryptodev_get_asym_xform_enum to >> be more consistent with other API names >> >> Signed-off-by: Shally Verma <shally.ve...@caviumnetworks.com> >> Signed-off-by: Sunila Sahu <sunila.s...@caviumnetworks.com> >> Signed-off-by: Ashish Gupta <ashish.gu...@caviumnetworks.com> >> >> --- >//snip// > >> diff --git a/lib/librte_cryptodev/rte_cryptodev.h >> b/lib/librte_cryptodev/rte_cryptodev.h >> index 623459a95..6c13d23f8 100644 >> --- a/lib/librte_cryptodev/rte_cryptodev.h >> +++ b/lib/librte_cryptodev/rte_cryptodev.h >> @@ -178,6 +178,35 @@ struct rte_cryptodev_symmetric_capability { >> }; >> }; >> >> +/** >> + * Asymmetric Xform Crypto Capability >> + * >> + */ >> +struct rte_cryptodev_asymmetric_xfrm_capability { >> + enum rte_crypto_asym_xform_type xform_type; >> + /**< Transform type: RSA/MODEXP/DH/DSA/MODINV */ >> + >> + uint32_t op_types; >> + /**< bitmask for supported rte_crypto_asym_op_type */ >> + >> + __extension__ >> + union { >> + struct rte_crypto_param_range modlen; >> + /**< Range of modulus length supported by modulus based xform. >> + * Value 0 mean implementation default >> + */ >[Fiona] Some other fields may be necessary here, e.g. > - A bitmask for supported RSA padding types > - Whether RSA private-key in quintuple format is supported > - Which hash-algorithms are supported if RSA padding = OAEP or PSS > - whether xform chaining is supported for DH keypair gen >These are not blockers for the first release, but are likely to be >needed before the experimental label is removed. > [Shally] Agree. Part of these capabilities might need to there as per xform capability. But as you indicated, in any case, they are experimental right now, so let's have them added on a requirement basis once current is accepted. >> + }; >> +}; >> + >> +/** >> + * Asymmetric Crypto Capability >> + * >> + */ >> +struct rte_cryptodev_asymmetric_capability { >> + struct rte_cryptodev_asymmetric_xfrm_capability xfrm_capa; >> +}; >[Fiona] Why the extra indirection? >Couldn't this be removed and the previous structure be >renamed rte_cryptodev_asymmetric_capability? [Shally] it is to keep consistency in rte_cryptodev_capability which uses name asymmetric_capability on the similar line as symmetric. But again, change is trivial, so if intended will do in subsequent versions. > >//snip// >> @@ -1164,7 +1265,7 @@ int __rte_experimental >> rte_cryptodev_asym_session_set_private_data( >[Fiona] I'd prefer to call this appl_data or appl_priv_data, I think the term >private_data is >over-used, sometimes means PMD data and sometimes means appl data. >Btw- same is true of sym private_data - but changing that is out of scope for >this patch > [Shally] ok. I can change it to get_app_priv_data and set_app_priv_data Thanks Shally > >> struct rte_cryptodev_asym_session >> *sess, >> void *data, >> - uint16_t size) >> + uint16_t size); >> >> /** >> * Get private data of a session. >> @@ -1178,7 +1279,7 @@ rte_cryptodev_asym_session_set_private_data( >> */ >> void * __rte_experimental >> rte_cryptodev_asym_session_get_private_data( >> - struct rte_cryptodev_asym_session *sess) >> + struct rte_cryptodev_asym_session *sess); >> >> >> #ifdef __cplusplus >> diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map >> b/lib/librte_cryptodev/rte_cryptodev_version.map >> index 62b782444..817cf9f70 100644 >> --- a/lib/librte_cryptodev/rte_cryptodev_version.map >> +++ b/lib/librte_cryptodev/rte_cryptodev_version.map >> @@ -89,13 +89,18 @@ DPDK_17.11 { >> EXPERIMENTAL { >> global: >> >> - rte_cryptodev_asym_get_private_session_size >> + rte_cryptodev_asym_capability_get; >> + rte_cryptodev_asym_get_private_session_size; >> + rte_cryptodev_asym_get_xform_enum; >> + rte_crypto_asym_op_strings; >> rte_cryptodev_asym_session_clear; >> rte_cryptodev_asym_session_create; >> rte_cryptodev_asym_session_free; >> rte_cryptodev_asym_session_init; >> - rte_cryptodev_asym_session_get_private_data >> - rte_cryptodev_asym_session_set_private_data >> + rte_cryptodev_asym_session_get_private_data; >> + rte_cryptodev_asym_session_set_private_data; >> + rte_cryptodev_asym_xfrm_capability_check_optype; >> + rte_crypto_asym_xform_strings; >> rte_cryptodev_sym_session_get_private_data; >> rte_cryptodev_sym_session_set_private_data; >> } DPDK_17.11; >> -- >> 2.14.3
