| Message ID | 20190906131330.40185-2-roy.fan.zhang@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | akhil goyal |
| Headers | show |
| Series |
|
| Related | show |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Performance-Testing | fail | build patch failure |
> +/** > + * Security vector structure, contains pointer to vector array and the length > + * of the array > + */ > +struct rte_security_vec { > + struct iovec *vec; > + uint32_t num; > +}; > + > +/** > + * Processing bulk crypto workload with CPU > + * > + * @param instance security instance. > + * @param sess security session > + * @param buf array of buffer SGL vectors > + * @param iv array of IV pointers > + * @param aad array of AAD pointers > + * @param digest array of digest pointers > + * @param status array of status for the function to return Need to specify what are expected status values. I suppose zero for success, negative errno for some error happens? > + * @param num number of elements in each array > + * > + */ > +__rte_experimental > +void > +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > + struct rte_security_session *sess, > + struct rte_security_vec buf[], void *iv[], void *aad[], > + void *digest[], int status[], uint32_t num); > + > #ifdef __cplusplus > } > #endif
Some comments inline. On 06-Sep-19 6:43 PM, Fan Zhang wrote: > This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO action type to > security library. The type represents performing crypto operation with CPU > cycles. The patch also includes a new API to process crypto operations in > bulk and the function pointers for PMDs. > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> > --- > lib/librte_security/rte_security.c | 16 +++++++++ > lib/librte_security/rte_security.h | 51 +++++++++++++++++++++++++++- > lib/librte_security/rte_security_driver.h | 19 +++++++++++ > lib/librte_security/rte_security_version.map | 1 + > 4 files changed, 86 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c > index bc81ce15d..0f85c1b59 100644 > --- a/lib/librte_security/rte_security.c > +++ b/lib/librte_security/rte_security.c > @@ -141,3 +141,19 @@ rte_security_capability_get(struct rte_security_ctx *instance, > > return NULL; > } > + > +void > +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > + struct rte_security_session *sess, > + struct rte_security_vec buf[], void *iv[], void *aad[], > + void *digest[], int status[], uint32_t num) > +{ > + uint32_t i; > + > + for (i = 0; i < num; i++) > + status[i] = -1; > + > + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); > + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, > + aad, digest, status, num); > +} > diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h > index 96806e3a2..5a0f8901b 100644 > --- a/lib/librte_security/rte_security.h > +++ b/lib/librte_security/rte_security.h > @@ -18,6 +18,7 @@ extern "C" { > #endif > > #include <sys/types.h> > +#include <sys/uio.h> > > #include <netinet/in.h> > #include <netinet/ip.h> > @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { > uint32_t hfn_threshold; > }; > > +struct rte_security_cpu_crypto_xform { > + /** For cipher/authentication crypto operation the authentication may > + * cover more content then the cipher. E.g., for IPSec ESP encryption > + * with AES-CBC and SHA1-HMAC, the encryption happens after the ESP > + * header but whole packet (apart from MAC header) is authenticated. > + * The cipher_offset field is used to deduct the cipher data pointer > + * from the buffer to be processed. > + * > + * NOTE this parameter shall be ignored by AEAD algorithms, since it > + * uses the same offset for cipher and authentication. > + */ > + int32_t cipher_offset; > +}; > + > /** > * Security session action type. > */ > @@ -286,10 +301,14 @@ enum rte_security_session_action_type { > /**< All security protocol processing is performed inline during > * transmission > */ > - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL > + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, > /**< All security protocol processing including crypto is performed > * on a lookaside accelerator > */ > + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > + /**< Crypto processing for security protocol is processed by CPU > + * synchronously > + */ though you are naming it cpu crypto, but it is more like raw packet crypto, where you want to skip mbuf/crypto ops and directly wants to work on raw buffer. > }; > > /** Security session protocol definition */ > @@ -315,6 +334,7 @@ struct rte_security_session_conf { > struct rte_security_ipsec_xform ipsec; > struct rte_security_macsec_xform macsec; > struct rte_security_pdcp_xform pdcp; > + struct rte_security_cpu_crypto_xform cpucrypto; > }; > /**< Configuration parameters for security session */ > struct rte_crypto_sym_xform *crypto_xform; > @@ -639,6 +659,35 @@ const struct rte_security_capability * > rte_security_capability_get(struct rte_security_ctx *instance, > struct rte_security_capability_idx *idx); > > +/** > + * Security vector structure, contains pointer to vector array and the length > + * of the array > + */ > +struct rte_security_vec { > + struct iovec *vec; > + uint32_t num; > +}; > + Just wondering if you want to change it to *in_vec and *out_vec, that will be helpful in future, if the out-of-place processing is required for CPU usecase as well? > +/** > + * Processing bulk crypto workload with CPU > + * > + * @param instance security instance. > + * @param sess security session > + * @param buf array of buffer SGL vectors > + * @param iv array of IV pointers > + * @param aad array of AAD pointers > + * @param digest array of digest pointers > + * @param status array of status for the function to return > + * @param num number of elements in each array > + * > + */ > +__rte_experimental > +void > +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > + struct rte_security_session *sess, > + struct rte_security_vec buf[], void *iv[], void *aad[], > + void *digest[], int status[], uint32_t num); > + Why not make the return as int, to indicate whether this API completely failed or processed or have some valid status to look into? > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h > index 1b561f852..70fcb0c26 100644 > --- a/lib/librte_security/rte_security_driver.h > +++ b/lib/librte_security/rte_security_driver.h > @@ -132,6 +132,23 @@ typedef int (*security_get_userdata_t)(void *device, > typedef const struct rte_security_capability *(*security_capabilities_get_t)( > void *device); > > +/** > + * Process security operations in bulk using CPU accelerated method. > + * > + * @param sess Security session structure. > + * @param buf Buffer to the vectors to be processed. > + * @param iv IV pointers. > + * @param aad AAD pointers. > + * @param digest Digest pointers. > + * @param status Array of status value. > + * @param num Number of elements in each array. > + */ > + > +typedef void (*security_process_cpu_crypto_bulk_t)( > + struct rte_security_session *sess, > + struct rte_security_vec buf[], void *iv[], void *aad[], > + void *digest[], int status[], uint32_t num); > + > /** Security operations function pointer table */ > struct rte_security_ops { > security_session_create_t session_create; > @@ -150,6 +167,8 @@ struct rte_security_ops { > /**< Get userdata associated with session which processed the packet. */ > security_capabilities_get_t capabilities_get; > /**< Get security capabilities. */ > + security_process_cpu_crypto_bulk_t process_cpu_crypto_bulk; > + /**< Process data in bulk. */ > }; > > #ifdef __cplusplus > diff --git a/lib/librte_security/rte_security_version.map b/lib/librte_security/rte_security_version.map > index 53267bf3c..2132e7a00 100644 > --- a/lib/librte_security/rte_security_version.map > +++ b/lib/librte_security/rte_security_version.map > @@ -18,4 +18,5 @@ EXPERIMENTAL { > rte_security_get_userdata; > rte_security_session_stats_get; > rte_security_session_update; > + rte_security_process_cpu_crypto_bulk; > };
Hi Hemant, > > On 06-Sep-19 6:43 PM, Fan Zhang wrote: > > This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO action type to > > security library. The type represents performing crypto operation with CPU > > cycles. The patch also includes a new API to process crypto operations in > > bulk and the function pointers for PMDs. > > > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> > > --- > > lib/librte_security/rte_security.c | 16 +++++++++ > > lib/librte_security/rte_security.h | 51 +++++++++++++++++++++++++++- > > lib/librte_security/rte_security_driver.h | 19 +++++++++++ > > lib/librte_security/rte_security_version.map | 1 + > > 4 files changed, 86 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c > > index bc81ce15d..0f85c1b59 100644 > > --- a/lib/librte_security/rte_security.c > > +++ b/lib/librte_security/rte_security.c > > @@ -141,3 +141,19 @@ rte_security_capability_get(struct rte_security_ctx *instance, > > > > return NULL; > > } > > + > > +void > > +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > > + struct rte_security_session *sess, > > + struct rte_security_vec buf[], void *iv[], void *aad[], > > + void *digest[], int status[], uint32_t num) > > +{ > > + uint32_t i; > > + > > + for (i = 0; i < num; i++) > > + status[i] = -1; > > + > > + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); > > + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, > > + aad, digest, status, num); > > +} > > diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h > > index 96806e3a2..5a0f8901b 100644 > > --- a/lib/librte_security/rte_security.h > > +++ b/lib/librte_security/rte_security.h > > @@ -18,6 +18,7 @@ extern "C" { > > #endif > > > > #include <sys/types.h> > > +#include <sys/uio.h> > > > > #include <netinet/in.h> > > #include <netinet/ip.h> > > @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { > > uint32_t hfn_threshold; > > }; > > > > +struct rte_security_cpu_crypto_xform { > > + /** For cipher/authentication crypto operation the authentication may > > + * cover more content then the cipher. E.g., for IPSec ESP encryption > > + * with AES-CBC and SHA1-HMAC, the encryption happens after the ESP > > + * header but whole packet (apart from MAC header) is authenticated. > > + * The cipher_offset field is used to deduct the cipher data pointer > > + * from the buffer to be processed. > > + * > > + * NOTE this parameter shall be ignored by AEAD algorithms, since it > > + * uses the same offset for cipher and authentication. > > + */ > > + int32_t cipher_offset; > > +}; > > + > > /** > > * Security session action type. > > */ > > @@ -286,10 +301,14 @@ enum rte_security_session_action_type { > > /**< All security protocol processing is performed inline during > > * transmission > > */ > > - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL > > + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, > > /**< All security protocol processing including crypto is performed > > * on a lookaside accelerator > > */ > > + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > > + /**< Crypto processing for security protocol is processed by CPU > > + * synchronously > > + */ > though you are naming it cpu crypto, but it is more like raw packet > crypto, where you want to skip mbuf/crypto ops and directly wants to > work on raw buffer. Yes, but we do wat to do that (skip mbuf/crypto ops and use raw buffer), because this API is destined for SW backed implementation. For that case crypto-ops , mbuf, enqueue/dequeue are just unnecessary overhead. > > }; > > > > /** Security session protocol definition */ > > @@ -315,6 +334,7 @@ struct rte_security_session_conf { > > struct rte_security_ipsec_xform ipsec; > > struct rte_security_macsec_xform macsec; > > struct rte_security_pdcp_xform pdcp; > > + struct rte_security_cpu_crypto_xform cpucrypto; > > }; > > /**< Configuration parameters for security session */ > > struct rte_crypto_sym_xform *crypto_xform; > > @@ -639,6 +659,35 @@ const struct rte_security_capability * > > rte_security_capability_get(struct rte_security_ctx *instance, > > struct rte_security_capability_idx *idx); > > > > +/** > > + * Security vector structure, contains pointer to vector array and the length > > + * of the array > > + */ > > +struct rte_security_vec { > > + struct iovec *vec; > > + uint32_t num; > > +}; > > + > > Just wondering if you want to change it to *in_vec and *out_vec, that > will be helpful in future, if the out-of-place processing is required > for CPU usecase as well? I suppose this is doable, though right now we don't plan to support such model. > > > +/** > > + * Processing bulk crypto workload with CPU > > + * > > + * @param instance security instance. > > + * @param sess security session > > + * @param buf array of buffer SGL vectors > > + * @param iv array of IV pointers > > + * @param aad array of AAD pointers > > + * @param digest array of digest pointers > > + * @param status array of status for the function to return > > + * @param num number of elements in each array > > + * > > + */ > > +__rte_experimental > > +void > > +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > > + struct rte_security_session *sess, > > + struct rte_security_vec buf[], void *iv[], void *aad[], > > + void *digest[], int status[], uint32_t num); > > + > > Why not make the return as int, to indicate whether this API completely > failed or processed or have some valid status to look into? Good point, will change as suggested. > > > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h > > index 1b561f852..70fcb0c26 100644 > > --- a/lib/librte_security/rte_security_driver.h > > +++ b/lib/librte_security/rte_security_driver.h > > @@ -132,6 +132,23 @@ typedef int (*security_get_userdata_t)(void *device, > > typedef const struct rte_security_capability *(*security_capabilities_get_t)( > > void *device); > > > > +/** > > + * Process security operations in bulk using CPU accelerated method. > > + * > > + * @param sess Security session structure. > > + * @param buf Buffer to the vectors to be processed. > > + * @param iv IV pointers. > > + * @param aad AAD pointers. > > + * @param digest Digest pointers. > > + * @param status Array of status value. > > + * @param num Number of elements in each array. > > + */ > > + > > +typedef void (*security_process_cpu_crypto_bulk_t)( > > + struct rte_security_session *sess, > > + struct rte_security_vec buf[], void *iv[], void *aad[], > > + void *digest[], int status[], uint32_t num); > > + > > /** Security operations function pointer table */ > > struct rte_security_ops { > > security_session_create_t session_create; > > @@ -150,6 +167,8 @@ struct rte_security_ops { > > /**< Get userdata associated with session which processed the packet. */ > > security_capabilities_get_t capabilities_get; > > /**< Get security capabilities. */ > > + security_process_cpu_crypto_bulk_t process_cpu_crypto_bulk; > > + /**< Process data in bulk. */ > > }; > > > > #ifdef __cplusplus > > diff --git a/lib/librte_security/rte_security_version.map b/lib/librte_security/rte_security_version.map > > index 53267bf3c..2132e7a00 100644 > > --- a/lib/librte_security/rte_security_version.map > > +++ b/lib/librte_security/rte_security_version.map > > @@ -18,4 +18,5 @@ EXPERIMENTAL { > > rte_security_get_userdata; > > rte_security_session_stats_get; > > rte_security_session_update; > > + rte_security_process_cpu_crypto_bulk; > > };
Hi Konstantin, n 06-Sep-19 6:43 PM, Fan Zhang wrote: >>> This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO action type to >>> security library. The type represents performing crypto operation with CPU >>> cycles. The patch also includes a new API to process crypto operations in >>> bulk and the function pointers for PMDs. >>> >>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> >>> --- >>> lib/librte_security/rte_security.c | 16 +++++++++ >>> lib/librte_security/rte_security.h | 51 +++++++++++++++++++++++++++- >>> lib/librte_security/rte_security_driver.h | 19 +++++++++++ >>> lib/librte_security/rte_security_version.map | 1 + >>> 4 files changed, 86 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c >>> index bc81ce15d..0f85c1b59 100644 >>> --- a/lib/librte_security/rte_security.c >>> +++ b/lib/librte_security/rte_security.c >>> @@ -141,3 +141,19 @@ rte_security_capability_get(struct rte_security_ctx *instance, >>> >>> return NULL; >>> } >>> + >>> +void >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, >>> + struct rte_security_session *sess, >>> + struct rte_security_vec buf[], void *iv[], void *aad[], >>> + void *digest[], int status[], uint32_t num) >>> +{ >>> + uint32_t i; >>> + >>> + for (i = 0; i < num; i++) >>> + status[i] = -1; >>> + >>> + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); >>> + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, >>> + aad, digest, status, num); >>> +} >>> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h >>> index 96806e3a2..5a0f8901b 100644 >>> --- a/lib/librte_security/rte_security.h >>> +++ b/lib/librte_security/rte_security.h >>> @@ -18,6 +18,7 @@ extern "C" { >>> #endif >>> >>> #include <sys/types.h> >>> +#include <sys/uio.h> >>> >>> #include <netinet/in.h> >>> #include <netinet/ip.h> >>> @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { >>> uint32_t hfn_threshold; >>> }; >>> >>> +struct rte_security_cpu_crypto_xform { >>> + /** For cipher/authentication crypto operation the authentication may >>> + * cover more content then the cipher. E.g., for IPSec ESP encryption >>> + * with AES-CBC and SHA1-HMAC, the encryption happens after the ESP >>> + * header but whole packet (apart from MAC header) is authenticated. >>> + * The cipher_offset field is used to deduct the cipher data pointer >>> + * from the buffer to be processed. >>> + * >>> + * NOTE this parameter shall be ignored by AEAD algorithms, since it >>> + * uses the same offset for cipher and authentication. >>> + */ >>> + int32_t cipher_offset; >>> +}; >>> + >>> /** >>> * Security session action type. >>> */ >>> @@ -286,10 +301,14 @@ enum rte_security_session_action_type { >>> /**< All security protocol processing is performed inline during >>> * transmission >>> */ >>> - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL >>> + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, >>> /**< All security protocol processing including crypto is performed >>> * on a lookaside accelerator >>> */ >>> + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO >>> + /**< Crypto processing for security protocol is processed by CPU >>> + * synchronously >>> + */ >> though you are naming it cpu crypto, but it is more like raw packet >> crypto, where you want to skip mbuf/crypto ops and directly wants to >> work on raw buffer. > Yes, but we do wat to do that (skip mbuf/crypto ops and use raw buffer), > because this API is destined for SW backed implementation. > For that case crypto-ops , mbuf, enqueue/dequeue are just unnecessary overhead. I agree, we are also planning to take advantage of it for some specific use-cases in future. >>> }; >>> >>> /** Security session protocol definition */ >>> @@ -315,6 +334,7 @@ struct rte_security_session_conf { >>> struct rte_security_ipsec_xform ipsec; >>> struct rte_security_macsec_xform macsec; >>> struct rte_security_pdcp_xform pdcp; >>> + struct rte_security_cpu_crypto_xform cpucrypto; >>> }; >>> /**< Configuration parameters for security session */ >>> struct rte_crypto_sym_xform *crypto_xform; >>> @@ -639,6 +659,35 @@ const struct rte_security_capability * >>> rte_security_capability_get(struct rte_security_ctx *instance, >>> struct rte_security_capability_idx *idx); >>> >>> +/** >>> + * Security vector structure, contains pointer to vector array and the length >>> + * of the array >>> + */ >>> +struct rte_security_vec { >>> + struct iovec *vec; >>> + uint32_t num; >>> +}; >>> + >> Just wondering if you want to change it to *in_vec and *out_vec, that >> will be helpful in future, if the out-of-place processing is required >> for CPU usecase as well? > I suppose this is doable, though right now we don't plan to support such model. They will come handy in future. I plan to use it in future and we can skip the API/ABI breakage, if the placeholder are present > >>> +/** >>> + * Processing bulk crypto workload with CPU >>> + * >>> + * @param instance security instance. >>> + * @param sess security session >>> + * @param buf array of buffer SGL vectors >>> + * @param iv array of IV pointers >>> + * @param aad array of AAD pointers >>> + * @param digest array of digest pointers >>> + * @param status array of status for the function to return >>> + * @param num number of elements in each array >>> + * >>> + */ >>> +__rte_experimental >>> +void >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, >>> + struct rte_security_session *sess, >>> + struct rte_security_vec buf[], void *iv[], void *aad[], >>> + void *digest[], int status[], uint32_t num); >>> + >> Why not make the return as int, to indicate whether this API completely >> failed or processed or have some valid status to look into? > Good point, will change as suggested. I have another suggestions w.r.t iv, aad, digest etc. Why not put them in a structure, so that you will be able to add/remove the variable without breaking the API prototype. > >> >>> #ifdef __cplusplus >>> } >>> #endif >>> diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h >>> index 1b561f852..70fcb0c26 100644 >>> --- a/lib/librte_security/rte_security_driver.h >>> +++ b/lib/librte_security/rte_security_driver.h >>> @@ -132,6 +132,23 @@ typedef int (*security_get_userdata_t)(void *device, >>> typedef const struct rte_security_capability *(*security_capabilities_get_t)( >>> void *device); >>> >>> +/** >>> + * Process security operations in bulk using CPU accelerated method. >>> + * >>> + * @param sess Security session structure. >>> + * @param buf Buffer to the vectors to be processed. >>> + * @param iv IV pointers. >>> + * @param aad AAD pointers. >>> + * @param digest Digest pointers. >>> + * @param status Array of status value. >>> + * @param num Number of elements in each array. >>> + */ >>> + >>> +typedef void (*security_process_cpu_crypto_bulk_t)( >>> + struct rte_security_session *sess, >>> + struct rte_security_vec buf[], void *iv[], void *aad[], >>> + void *digest[], int status[], uint32_t num); >>> + >>> /** Security operations function pointer table */ >>> struct rte_security_ops { >>> security_session_create_t session_create; >>> @@ -150,6 +167,8 @@ struct rte_security_ops { >>> /**< Get userdata associated with session which processed the packet. */ >>> security_capabilities_get_t capabilities_get; >>> /**< Get security capabilities. */ >>> + security_process_cpu_crypto_bulk_t process_cpu_crypto_bulk; >>> + /**< Process data in bulk. */ >>> }; >>> >>> #ifdef __cplusplus >>> diff --git a/lib/librte_security/rte_security_version.map b/lib/librte_security/rte_security_version.map >>> index 53267bf3c..2132e7a00 100644 >>> --- a/lib/librte_security/rte_security_version.map >>> +++ b/lib/librte_security/rte_security_version.map >>> @@ -18,4 +18,5 @@ EXPERIMENTAL { >>> rte_security_get_userdata; >>> rte_security_session_stats_get; >>> rte_security_session_update; >>> + rte_security_process_cpu_crypto_bulk; >>> };
Hi Hemant, > >>> This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO action type to > >>> security library. The type represents performing crypto operation with CPU > >>> cycles. The patch also includes a new API to process crypto operations in > >>> bulk and the function pointers for PMDs. > >>> > >>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> > >>> --- > >>> lib/librte_security/rte_security.c | 16 +++++++++ > >>> lib/librte_security/rte_security.h | 51 +++++++++++++++++++++++++++- > >>> lib/librte_security/rte_security_driver.h | 19 +++++++++++ > >>> lib/librte_security/rte_security_version.map | 1 + > >>> 4 files changed, 86 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c > >>> index bc81ce15d..0f85c1b59 100644 > >>> --- a/lib/librte_security/rte_security.c > >>> +++ b/lib/librte_security/rte_security.c > >>> @@ -141,3 +141,19 @@ rte_security_capability_get(struct rte_security_ctx *instance, > >>> > >>> return NULL; > >>> } > >>> + > >>> +void > >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > >>> + struct rte_security_session *sess, > >>> + struct rte_security_vec buf[], void *iv[], void *aad[], > >>> + void *digest[], int status[], uint32_t num) > >>> +{ > >>> + uint32_t i; > >>> + > >>> + for (i = 0; i < num; i++) > >>> + status[i] = -1; > >>> + > >>> + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); > >>> + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, > >>> + aad, digest, status, num); > >>> +} > >>> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h > >>> index 96806e3a2..5a0f8901b 100644 > >>> --- a/lib/librte_security/rte_security.h > >>> +++ b/lib/librte_security/rte_security.h > >>> @@ -18,6 +18,7 @@ extern "C" { > >>> #endif > >>> > >>> #include <sys/types.h> > >>> +#include <sys/uio.h> > >>> > >>> #include <netinet/in.h> > >>> #include <netinet/ip.h> > >>> @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { > >>> uint32_t hfn_threshold; > >>> }; > >>> > >>> +struct rte_security_cpu_crypto_xform { > >>> + /** For cipher/authentication crypto operation the authentication may > >>> + * cover more content then the cipher. E.g., for IPSec ESP encryption > >>> + * with AES-CBC and SHA1-HMAC, the encryption happens after the ESP > >>> + * header but whole packet (apart from MAC header) is authenticated. > >>> + * The cipher_offset field is used to deduct the cipher data pointer > >>> + * from the buffer to be processed. > >>> + * > >>> + * NOTE this parameter shall be ignored by AEAD algorithms, since it > >>> + * uses the same offset for cipher and authentication. > >>> + */ > >>> + int32_t cipher_offset; > >>> +}; > >>> + > >>> /** > >>> * Security session action type. > >>> */ > >>> @@ -286,10 +301,14 @@ enum rte_security_session_action_type { > >>> /**< All security protocol processing is performed inline during > >>> * transmission > >>> */ > >>> - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL > >>> + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, > >>> /**< All security protocol processing including crypto is performed > >>> * on a lookaside accelerator > >>> */ > >>> + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > >>> + /**< Crypto processing for security protocol is processed by CPU > >>> + * synchronously > >>> + */ > >> though you are naming it cpu crypto, but it is more like raw packet > >> crypto, where you want to skip mbuf/crypto ops and directly wants to > >> work on raw buffer. > > Yes, but we do wat to do that (skip mbuf/crypto ops and use raw buffer), > > because this API is destined for SW backed implementation. > > For that case crypto-ops , mbuf, enqueue/dequeue are just unnecessary overhead. > I agree, we are also planning to take advantage of it for some specific > use-cases in future. > >>> }; > >>> > >>> /** Security session protocol definition */ > >>> @@ -315,6 +334,7 @@ struct rte_security_session_conf { > >>> struct rte_security_ipsec_xform ipsec; > >>> struct rte_security_macsec_xform macsec; > >>> struct rte_security_pdcp_xform pdcp; > >>> + struct rte_security_cpu_crypto_xform cpucrypto; > >>> }; > >>> /**< Configuration parameters for security session */ > >>> struct rte_crypto_sym_xform *crypto_xform; > >>> @@ -639,6 +659,35 @@ const struct rte_security_capability * > >>> rte_security_capability_get(struct rte_security_ctx *instance, > >>> struct rte_security_capability_idx *idx); > >>> > >>> +/** > >>> + * Security vector structure, contains pointer to vector array and the length > >>> + * of the array > >>> + */ > >>> +struct rte_security_vec { > >>> + struct iovec *vec; > >>> + uint32_t num; > >>> +}; > >>> + > >> Just wondering if you want to change it to *in_vec and *out_vec, that > >> will be helpful in future, if the out-of-place processing is required > >> for CPU usecase as well? > > I suppose this is doable, though right now we don't plan to support such model. > They will come handy in future. I plan to use it in future and we can > skip the API/ABI breakage, if the placeholder are present > > > >>> +/** > >>> + * Processing bulk crypto workload with CPU > >>> + * > >>> + * @param instance security instance. > >>> + * @param sess security session > >>> + * @param buf array of buffer SGL vectors > >>> + * @param iv array of IV pointers > >>> + * @param aad array of AAD pointers > >>> + * @param digest array of digest pointers > >>> + * @param status array of status for the function to return > >>> + * @param num number of elements in each array > >>> + * > >>> + */ > >>> +__rte_experimental > >>> +void > >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > >>> + struct rte_security_session *sess, > >>> + struct rte_security_vec buf[], void *iv[], void *aad[], > >>> + void *digest[], int status[], uint32_t num); > >>> + > >> Why not make the return as int, to indicate whether this API completely > >> failed or processed or have some valid status to look into? > > Good point, will change as suggested. > > I have another suggestions w.r.t iv, aad, digest etc. Why not put them > in a structure, so that you will > > be able to add/remove the variable without breaking the API prototype. Just to confirm, you are talking about something like: struct rte_security_vec { struct iovec *vec; uint32_t num; }; struct rte_security_sym_vec { struct rte_security_vec buf; void *iv; void *aad; void *digest; }; rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, struct rte_security_session *sess, struct rte_security_sym_vec buf[], int status[], uint32_t num); ? We thought about such way, though for PMD it would be more plausible to have same type of params grouped together, i.e. void *in[], void *out[], void *digest[], ... Another thing - above grouping wouldn't help to avoid ABI breakage, in case we'll need to add new field into rte_security_sym_vec (though it might help to avoid API breakage). In theory other way is also possible: struct rte_security_sym_vec { struct rte_security_vec *buf; void **iv; void **aad; void **digest; }; rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, struct rte_security_session *sess, struct rte_security_sym_vec *buf, int status[], uint32_t num); And that might help for both ABI and API stability, but it looks really weird that way (at least to me). Also this API is experimental and I suppose needs to stay experimental for few releases before we are sure nothing important is missing, so probably API/ABI stability is not that high concern for it right now. Konstantin
Hi Konstantin, > > >>> This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > > >>> action type to security library. The type represents performing > > >>> crypto operation with CPU cycles. The patch also includes a new > > >>> API to process crypto operations in bulk and the function pointers for > PMDs. > > >>> > > >>> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> > > >>> --- > > >>> lib/librte_security/rte_security.c | 16 +++++++++ > > >>> lib/librte_security/rte_security.h | 51 > +++++++++++++++++++++++++++- > > >>> lib/librte_security/rte_security_driver.h | 19 +++++++++++ > > >>> lib/librte_security/rte_security_version.map | 1 + > > >>> 4 files changed, 86 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/lib/librte_security/rte_security.c > > >>> b/lib/librte_security/rte_security.c > > >>> index bc81ce15d..0f85c1b59 100644 > > >>> --- a/lib/librte_security/rte_security.c > > >>> +++ b/lib/librte_security/rte_security.c > > >>> @@ -141,3 +141,19 @@ rte_security_capability_get(struct > > >>> rte_security_ctx *instance, > > >>> > > >>> return NULL; > > >>> } > > >>> + > > >>> +void > > >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx > *instance, > > >>> + struct rte_security_session *sess, > > >>> + struct rte_security_vec buf[], void *iv[], void *aad[], > > >>> + void *digest[], int status[], uint32_t num) { > > >>> + uint32_t i; > > >>> + > > >>> + for (i = 0; i < num; i++) > > >>> + status[i] = -1; > > >>> + > > >>> + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); > > >>> + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, > > >>> + aad, digest, status, num); > > >>> +} > > >>> diff --git a/lib/librte_security/rte_security.h > > >>> b/lib/librte_security/rte_security.h > > >>> index 96806e3a2..5a0f8901b 100644 > > >>> --- a/lib/librte_security/rte_security.h > > >>> +++ b/lib/librte_security/rte_security.h > > >>> @@ -18,6 +18,7 @@ extern "C" { > > >>> #endif > > >>> > > >>> #include <sys/types.h> > > >>> +#include <sys/uio.h> > > >>> > > >>> #include <netinet/in.h> > > >>> #include <netinet/ip.h> > > >>> @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { > > >>> uint32_t hfn_threshold; > > >>> }; > > >>> > > >>> +struct rte_security_cpu_crypto_xform { > > >>> + /** For cipher/authentication crypto operation the authentication > may > > >>> + * cover more content then the cipher. E.g., for IPSec ESP encryption > > >>> + * with AES-CBC and SHA1-HMAC, the encryption happens after the > ESP > > >>> + * header but whole packet (apart from MAC header) is > authenticated. > > >>> + * The cipher_offset field is used to deduct the cipher data pointer > > >>> + * from the buffer to be processed. > > >>> + * > > >>> + * NOTE this parameter shall be ignored by AEAD algorithms, since it > > >>> + * uses the same offset for cipher and authentication. > > >>> + */ > > >>> + int32_t cipher_offset; > > >>> +}; > > >>> + > > >>> /** > > >>> * Security session action type. > > >>> */ > > >>> @@ -286,10 +301,14 @@ enum rte_security_session_action_type { > > >>> /**< All security protocol processing is performed inline during > > >>> * transmission > > >>> */ > > >>> - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL > > >>> + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, > > >>> /**< All security protocol processing including crypto is performed > > >>> * on a lookaside accelerator > > >>> */ > > >>> + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > > >>> + /**< Crypto processing for security protocol is processed by CPU > > >>> + * synchronously > > >>> + */ > > >> though you are naming it cpu crypto, but it is more like raw packet > > >> crypto, where you want to skip mbuf/crypto ops and directly wants > > >> to work on raw buffer. > > > Yes, but we do wat to do that (skip mbuf/crypto ops and use raw > > > buffer), because this API is destined for SW backed implementation. > > > For that case crypto-ops , mbuf, enqueue/dequeue are just unnecessary > overhead. > > I agree, we are also planning to take advantage of it for some > > specific use-cases in future. > > >>> }; > > >>> > > >>> /** Security session protocol definition */ @@ -315,6 +334,7 @@ > > >>> struct rte_security_session_conf { > > >>> struct rte_security_ipsec_xform ipsec; > > >>> struct rte_security_macsec_xform macsec; > > >>> struct rte_security_pdcp_xform pdcp; > > >>> + struct rte_security_cpu_crypto_xform cpucrypto; > > >>> }; > > >>> /**< Configuration parameters for security session */ > > >>> struct rte_crypto_sym_xform *crypto_xform; @@ -639,6 +659,35 > > >>> @@ const struct rte_security_capability * > > >>> rte_security_capability_get(struct rte_security_ctx *instance, > > >>> struct rte_security_capability_idx *idx); > > >>> > > >>> +/** > > >>> + * Security vector structure, contains pointer to vector array > > >>> +and the length > > >>> + * of the array > > >>> + */ > > >>> +struct rte_security_vec { > > >>> + struct iovec *vec; > > >>> + uint32_t num; > > >>> +}; > > >>> + > > >> Just wondering if you want to change it to *in_vec and *out_vec, > > >> that will be helpful in future, if the out-of-place processing is > > >> required for CPU usecase as well? > > > I suppose this is doable, though right now we don't plan to support such > model. > > They will come handy in future. I plan to use it in future and we can > > skip the API/ABI breakage, if the placeholder are present > > > > > >>> +/** > > >>> + * Processing bulk crypto workload with CPU > > >>> + * > > >>> + * @param instance security instance. > > >>> + * @param sess security session > > >>> + * @param buf array of buffer SGL vectors > > >>> + * @param iv array of IV pointers > > >>> + * @param aad array of AAD pointers > > >>> + * @param digest array of digest pointers > > >>> + * @param status array of status for the function to > return > > >>> + * @param num number of elements in each array > > >>> + * > > >>> + */ > > >>> +__rte_experimental > > >>> +void > > >>> +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx > *instance, > > >>> + struct rte_security_session *sess, > > >>> + struct rte_security_vec buf[], void *iv[], void *aad[], > > >>> + void *digest[], int status[], uint32_t num); > > >>> + > > >> Why not make the return as int, to indicate whether this API > > >> completely failed or processed or have some valid status to look into? > > > Good point, will change as suggested. > > > > I have another suggestions w.r.t iv, aad, digest etc. Why not put them > > in a structure, so that you will > > > > be able to add/remove the variable without breaking the API prototype. > > > Just to confirm, you are talking about something like: > > struct rte_security_vec { > struct iovec *vec; > uint32_t num; > }; [Hemant] My idea is: struct rte_security_vec { struct iovec *vec; struct iovec *out_vec; uint32_t num_in; uint32_t num_out; }; > > struct rte_security_sym_vec { > struct rte_security_vec buf; > void *iv; > void *aad; > void *digest; > }; > [Hemant] or leave the rte_security_vec altogether and make it part of rte_security_sym_vec itself. > rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > struct rte_security_session *sess, struct rte_security_sym_vec buf[], > int status[], uint32_t num); > > ? > We thought about such way, though for PMD it would be more plausible to > have same type of params grouped together, i.e. void *in[], void *out[], void > *digest[], ... > Another thing - above grouping wouldn't help to avoid ABI breakage, in case > we'll need to add new field into rte_security_sym_vec (though it might help > to avoid API breakage). > > In theory other way is also possible: > struct rte_security_sym_vec { > struct rte_security_vec *buf; > void **iv; > void **aad; > void **digest; > }; > > rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, > struct rte_security_session *sess, struct rte_security_sym_vec *buf, > int status[], uint32_t num); > > And that might help for both ABI and API stability, but it looks really weird > that way (at least to me). [Hemant] I am fine either way. > Also this API is experimental and I suppose needs to stay experimental for > few releases before we are sure nothing important is missing, so probably > API/ABI stability is not that high concern for it right now. > > Konstantin > >
diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c index bc81ce15d..0f85c1b59 100644 --- a/lib/librte_security/rte_security.c +++ b/lib/librte_security/rte_security.c @@ -141,3 +141,19 @@ rte_security_capability_get(struct rte_security_ctx *instance, return NULL; } + +void +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, + struct rte_security_session *sess, + struct rte_security_vec buf[], void *iv[], void *aad[], + void *digest[], int status[], uint32_t num) +{ + uint32_t i; + + for (i = 0; i < num; i++) + status[i] = -1; + + RTE_FUNC_PTR_OR_RET(*instance->ops->process_cpu_crypto_bulk); + instance->ops->process_cpu_crypto_bulk(sess, buf, iv, + aad, digest, status, num); +} diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h index 96806e3a2..5a0f8901b 100644 --- a/lib/librte_security/rte_security.h +++ b/lib/librte_security/rte_security.h @@ -18,6 +18,7 @@ extern "C" { #endif #include <sys/types.h> +#include <sys/uio.h> #include <netinet/in.h> #include <netinet/ip.h> @@ -272,6 +273,20 @@ struct rte_security_pdcp_xform { uint32_t hfn_threshold; }; +struct rte_security_cpu_crypto_xform { + /** For cipher/authentication crypto operation the authentication may + * cover more content then the cipher. E.g., for IPSec ESP encryption + * with AES-CBC and SHA1-HMAC, the encryption happens after the ESP + * header but whole packet (apart from MAC header) is authenticated. + * The cipher_offset field is used to deduct the cipher data pointer + * from the buffer to be processed. + * + * NOTE this parameter shall be ignored by AEAD algorithms, since it + * uses the same offset for cipher and authentication. + */ + int32_t cipher_offset; +}; + /** * Security session action type. */ @@ -286,10 +301,14 @@ enum rte_security_session_action_type { /**< All security protocol processing is performed inline during * transmission */ - RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL, /**< All security protocol processing including crypto is performed * on a lookaside accelerator */ + RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO + /**< Crypto processing for security protocol is processed by CPU + * synchronously + */ }; /** Security session protocol definition */ @@ -315,6 +334,7 @@ struct rte_security_session_conf { struct rte_security_ipsec_xform ipsec; struct rte_security_macsec_xform macsec; struct rte_security_pdcp_xform pdcp; + struct rte_security_cpu_crypto_xform cpucrypto; }; /**< Configuration parameters for security session */ struct rte_crypto_sym_xform *crypto_xform; @@ -639,6 +659,35 @@ const struct rte_security_capability * rte_security_capability_get(struct rte_security_ctx *instance, struct rte_security_capability_idx *idx); +/** + * Security vector structure, contains pointer to vector array and the length + * of the array + */ +struct rte_security_vec { + struct iovec *vec; + uint32_t num; +}; + +/** + * Processing bulk crypto workload with CPU + * + * @param instance security instance. + * @param sess security session + * @param buf array of buffer SGL vectors + * @param iv array of IV pointers + * @param aad array of AAD pointers + * @param digest array of digest pointers + * @param status array of status for the function to return + * @param num number of elements in each array + * + */ +__rte_experimental +void +rte_security_process_cpu_crypto_bulk(struct rte_security_ctx *instance, + struct rte_security_session *sess, + struct rte_security_vec buf[], void *iv[], void *aad[], + void *digest[], int status[], uint32_t num); + #ifdef __cplusplus } #endif diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h index 1b561f852..70fcb0c26 100644 --- a/lib/librte_security/rte_security_driver.h +++ b/lib/librte_security/rte_security_driver.h @@ -132,6 +132,23 @@ typedef int (*security_get_userdata_t)(void *device, typedef const struct rte_security_capability *(*security_capabilities_get_t)( void *device); +/** + * Process security operations in bulk using CPU accelerated method. + * + * @param sess Security session structure. + * @param buf Buffer to the vectors to be processed. + * @param iv IV pointers. + * @param aad AAD pointers. + * @param digest Digest pointers. + * @param status Array of status value. + * @param num Number of elements in each array. + */ + +typedef void (*security_process_cpu_crypto_bulk_t)( + struct rte_security_session *sess, + struct rte_security_vec buf[], void *iv[], void *aad[], + void *digest[], int status[], uint32_t num); + /** Security operations function pointer table */ struct rte_security_ops { security_session_create_t session_create; @@ -150,6 +167,8 @@ struct rte_security_ops { /**< Get userdata associated with session which processed the packet. */ security_capabilities_get_t capabilities_get; /**< Get security capabilities. */ + security_process_cpu_crypto_bulk_t process_cpu_crypto_bulk; + /**< Process data in bulk. */ }; #ifdef __cplusplus diff --git a/lib/librte_security/rte_security_version.map b/lib/librte_security/rte_security_version.map index 53267bf3c..2132e7a00 100644 --- a/lib/librte_security/rte_security_version.map +++ b/lib/librte_security/rte_security_version.map @@ -18,4 +18,5 @@ EXPERIMENTAL { rte_security_get_userdata; rte_security_session_stats_get; rte_security_session_update; + rte_security_process_cpu_crypto_bulk; };
This patch introduce new RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO action type to security library. The type represents performing crypto operation with CPU cycles. The patch also includes a new API to process crypto operations in bulk and the function pointers for PMDs. Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com> --- lib/librte_security/rte_security.c | 16 +++++++++ lib/librte_security/rte_security.h | 51 +++++++++++++++++++++++++++- lib/librte_security/rte_security_driver.h | 19 +++++++++++ lib/librte_security/rte_security_version.map | 1 + 4 files changed, 86 insertions(+), 1 deletion(-)