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.zh...@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; >>> };