From patchwork Thu May 4 15:31:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tomasz Kulasek X-Patchwork-Id: 24091 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 5612E7CD1; Thu, 4 May 2017 17:33:17 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id F24C37CCF for ; Thu, 4 May 2017 17:33:15 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP; 04 May 2017 08:33:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.38,287,1491289200"; d="scan'208"; a="1143636400" Received: from unknown (HELO Sent) ([10.103.102.163]) by fmsmga001.fm.intel.com with SMTP; 04 May 2017 08:33:06 -0700 Received: by Sent (sSMTP sendmail emulation); Thu, 04 May 2017 17:31:21 +0200 From: Tomasz Kulasek To: dev@dpdk.org Cc: declan.doherty@intel.com Date: Thu, 4 May 2017 17:31:14 +0200 Message-Id: <1493911874-5888-1-git-send-email-tomaszx.kulasek@intel.com> X-Mailer: git-send-email 2.1.4 Subject: [dpdk-dev] [RFC PATCH] cryptodev: make crypto session device independent X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This RFC describes a proposal to change crypto device's session management to make it device independent and simplify architecture when session is intended to be used on more than one device. 1. Overview Limitations of existing implementation: a) Each session can store only private data for one device type. It makes impossible to use the same session with different device types (e.g. QAT, openssl) and requires to create multiple instances of session for each device type. b) Session objects are stored in the device instances. Use of session created on other instance of the device with the same type is not possible. Removing limitations listed above, should: a) Share one session with different devices in a transparent way, b) Simplify architecture by coupling sessions not with device instance, but external session mempool created and managed by application. c) Increase performance, consistency and reliability in terms of: - Session management time, - Number of sessions that should be created and initialized, - One session can be used on multiple devices (with transparent access to the session private data) - Simplify session management and utilizaton in the application, - Sessions are stored outside of the device instance and can be used and managed regardless of the state of the device (e.g. failures) 2. Scope of work a) Make the create sessions private data be agnostic to underlying device by adding an indirection in the sessions private_data using the crypto device identifier. b) Make necessary changes to the crypto session mempool infrastructure to remove coupling with crypto PMD. c) A single session can contain indirections to multiple device types. d) Lazy retrieval of private session material at run time in each PMD 3. Proposal architecture SESSION MEMPOOL <---|---> DEVICE INSTANCES -------------------------------------------- .-------------. .----------------------------. | Crypto |. | mempool obj | | sym-session |<--. | .--------------------------+ | mempool || | | | rte_cryptodev_session | .-------------. `-------------'| | | | - session type | | DEVICE | `-----|-------' | | | - supported device mask | | INSTANCE | | `------- mempool | | - device id | +------------>| | - private data[type idx] | .--| - session | | `-+=================|..|=====' | | mempool | | | | | `-------------' | | | | | .----------------------. | | | .-------------. +---->| mempool obj | | | .--------. | | DEVICE | | | .--------------------+ | | | DRIVER | | | INSTANCE | | | | PMD's private data |<---+------| - type |<--+--| | | +-+====================+ | `--------' | ... | | | | | | | `-------------' | | | .----------------------. | .-------------. `---->| mempool obj | | .--------. | DEVICE | | .--------------------+ | | DRIVER | | INSTANCE | | | PMD's private data |<------+---| - type |<-----| | `-+====================' `--------' | ... | | | `-------------' Crypto sym-session mempool Mempool created to store rte_cryptodev_session objects as well as private data for supported device types. This mempool should replace existing session mempools in device instances to ease session management and improve consistency (especially if the session is intended to be used for operations performed on different devices). Device type id Device type id is an unique number identifing device driver type. rte_crypto_session New rte_cryptodev_session structure is taken from crypto session mempool and can store more than one PMD's private data object (one for each device type supported by sesion). Crypto operations will use device type id (unique for each driver) to retrieve private data from rte_crypto_session right for the device type they are performed on. 4. API proposal Crypto session pool /**< * Create symmetric crypto session pool * * @param nb_objs Number of elements to allocate in mempool * @param device_ids Array of crypto device identifiers, which * identify which device types the mempool will * support. * The device types is used to determine the * required size of the mempool elements, which * depends on the session private size of the * devices. If NULL the session mempool will * set the element size baseon the largest * private data requirements for all devices * @param nb_devices Number of devices in device_ids * @param cache_size Object cache size * @param socket_id Socket where memory will be used to create * mempool * * @return * - On success return pointer to crypto symmetric session mempool * - On failure returns NULL */ struct rte_cryptodev_sym_sess_mempool * rte_cryptodev_sym_session_pool_create(unsigned int nb_objs, unsigned int *device_ids, unsigned int nb_devices, unsigned int cache_size, unsigned int socket_id); Session management struct rte_cryptodev_sym_session { struct rte_mempool *mp; /**< Mempool session allocated from */ uint32_t sess_mask; /**< Mask containing which private data is set (faster free) */ void *sess_private_data[32]; /**< Device type specific session private data */ } /**< * Create symmetric crypto session (generic with no private data) * * @param mempool Symmetric session mempool to allocate session * objects from * @return * - On success return pointer to sym-session * - On failure returns NULL */ struct rte_cryptodev_sym_session * rte_cryptodev_sym_session_create(struct rte_mempool *mempool); /**< * Fill out private data for the device id, based on its device type * * @param dev_id ID of device that we want the session to be used on * @param sess Session where the private data will be attached to * @param xforms Symmetric crypto transform operations to apply on flow * processed with this session */ int rte_cryptodev_sym_session_init(uint8_t dev_id, struct rte_cryptodev_sym_session *sess, struct rte_crypto_sym_xform *xforms); /**< * Frees symmetric crypto session and all related device type * specific private data objects associated with session */ void rte_crypto_sym_session_free(struct rte_cryptodev_sym_session *sess); Private data retrieval static inline void * get_session_private_data(struct rte_crypto_sym_session *sess, unsigned int device_type_id); #define rte_cryptodev_sym_sess_get_private_data(type, sess, id) \ (type *)get_session_private_data(sess, id) Signed-off-by: Tomasz Kulasek --- lib/librte_cryptodev/rte_cryptodev.c | 256 ++++++++++++++++++++--------------- lib/librte_cryptodev/rte_cryptodev.h | 101 +++++++------- 2 files changed, 198 insertions(+), 159 deletions(-) diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c index 6f5080c..d304800 100644 --- a/lib/librte_cryptodev/rte_cryptodev.c +++ b/lib/librte_cryptodev/rte_cryptodev.c @@ -934,10 +934,6 @@ struct rte_cryptodev * } -static int -rte_cryptodev_sym_session_pool_create(struct rte_cryptodev *dev, - unsigned nb_objs, unsigned obj_cache_size, int socket_id); - int rte_cryptodev_configure(uint8_t dev_id, struct rte_cryptodev_config *config) { @@ -968,14 +964,6 @@ struct rte_cryptodev * return diag; } - /* Setup Session mempool for device */ - diag = rte_cryptodev_sym_session_pool_create(dev, - config->session_mp.nb_objs, - config->session_mp.cache_size, - config->socket_id); - if (diag != 0) - return diag; - return (*dev->dev_ops->dev_configure)(dev, config); } @@ -1280,65 +1263,145 @@ struct rte_cryptodev * rte_spinlock_unlock(&rte_cryptodev_cb_lock); } +int +rte_cryptodev_sym_session_init(uint8_t dev_id, + struct rte_cryptodev_sym_session *sess, + struct rte_crypto_sym_xform *xforms) { -static void -rte_cryptodev_sym_session_init(struct rte_mempool *mp, - void *opaque_arg, - void *_sess, - __rte_unused unsigned i) -{ - struct rte_cryptodev_sym_session *sess = _sess; - struct rte_cryptodev *dev = opaque_arg; + struct rte_cryptodev *dev; + void *sess_private_data; + int index; + + dev = rte_cryptodev_pmd_get_dev(dev_id); - memset(sess, 0, mp->elt_size); + /* FIXIT: for now, we're expecting not more than 32 device types + * for simplification */ + index = dev->dev_type; - sess->dev_id = dev->data->dev_id; - sess->dev_type = dev->dev_type; - sess->mp = mp; + if ((sess->sess_mask & (1 << index)) == 0) { + sess->sess_mask |= (1 << index); - if (dev->dev_ops->session_initialize) - (*dev->dev_ops->session_initialize)(mp, sess); + if (rte_mempool_get(sess->mp, &sess_private_data)) { + CDEV_LOG_ERR("Couldn't get object from session mempool"); + return -1; + } + + if (dev->dev_ops->session_initialize) + (*dev->dev_ops->session_initialize)(sess->mp, sess); + + //RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->session_configure, NULL); + if (dev->dev_ops->session_configure(dev, xforms, + sess_private_data) == NULL) { + CDEV_LOG_ERR("dev_id %d failed to configure session details", + dev_id); + + /* Return session to mempool */ + rte_mempool_put(sess->mp, sess_private_data); + return -1; + } + + set_session_private_data(sess, dev->dev_type, sess_private_data); + } + + return 0; } -static int -rte_cryptodev_sym_session_pool_create(struct rte_cryptodev *dev, - unsigned nb_objs, unsigned obj_cache_size, int socket_id) -{ - char mp_name[RTE_CRYPTODEV_NAME_MAX_LEN]; +struct rte_mempool * +rte_cryptodev_sym_session_pool_create(unsigned int nb_objs, + unsigned int *device_ids, + unsigned int nb_device_ids, + unsigned int obj_cache_size, + unsigned int socket_id) { + + unsigned int max_priv_sess_size = sizeof(struct rte_cryptodev_sym_session); unsigned priv_sess_size; + unsigned int dev_id, i; + struct rte_cryptodev *dev; + struct rte_mempool *mempool; + + char mp_name[RTE_CRYPTODEV_NAME_MAX_LEN]; + /* FIXIT: set the proper name for mempool! */ unsigned n = snprintf(mp_name, sizeof(mp_name), "cdev_%d_sess_mp", - dev->data->dev_id); + socket_id); if (n > sizeof(mp_name)) { CDEV_LOG_ERR("Unable to create unique name for session mempool"); - return -ENOMEM; + return NULL; } - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->session_get_size, -ENOTSUP); - priv_sess_size = (*dev->dev_ops->session_get_size)(dev); - if (priv_sess_size == 0) { - CDEV_LOG_ERR("%s returned and invalid private session size ", + if (device_ids == NULL) { + + /* Walk through all devices */ + for (dev_id = 0; dev_id < rte_cryptodev_globals->nb_devs; + dev_id++) { + + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) + continue; + + dev = rte_cryptodev_pmd_get_dev(dev_id); + + //RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->session_get_size, -ENOTSUP); + if (*dev->dev_ops->session_get_size == NULL) + continue; + + priv_sess_size = (*dev->dev_ops->session_get_size)(dev); + + if (priv_sess_size == 0) { + CDEV_LOG_ERR("%s returned and invalid private session size ", dev->data->name); - return -ENOMEM; + return NULL; + } + + if (max_priv_sess_size < priv_sess_size) + max_priv_sess_size = priv_sess_size; + + } + } else { + for (i = 0; i < nb_device_ids; i++) { + + if (!rte_cryptodev_pmd_is_valid_dev(device_ids[i])) + continue; + + dev = rte_cryptodev_pmd_get_dev(device_ids[i]); + + if (*dev->dev_ops->session_get_size == NULL) + continue; + + priv_sess_size = (*dev->dev_ops->session_get_size)(dev); + + if (priv_sess_size == 0) { + CDEV_LOG_ERR("%s returned and invalid private session size ", + dev->data->name); + return NULL; + } + + if (max_priv_sess_size < priv_sess_size) + max_priv_sess_size = priv_sess_size; + + } } - unsigned elt_size = sizeof(struct rte_cryptodev_sym_session) + - priv_sess_size; + if (max_priv_sess_size == 0) { + CDEV_LOG_ERR("Cannot determinate maximal private data"); + return NULL; + } - dev->data->session_pool = rte_mempool_lookup(mp_name); - if (dev->data->session_pool != NULL) { - if ((dev->data->session_pool->elt_size != elt_size) || - (dev->data->session_pool->cache_size < + unsigned int elt_size = max_priv_sess_size; + + mempool = rte_mempool_lookup(mp_name); + if (mempool != NULL) { + if ((mempool->elt_size != elt_size) || + (mempool->cache_size < obj_cache_size) || - (dev->data->session_pool->size < nb_objs)) { + (mempool->size < nb_objs)) { CDEV_LOG_ERR("%s mempool already exists with different" " initialization parameters", mp_name); - dev->data->session_pool = NULL; - return -ENOMEM; + mempool = NULL; + return NULL; } } else { - dev->data->session_pool = rte_mempool_create( + mempool = rte_mempool_create( mp_name, /* mempool name */ nb_objs, /* number of elements*/ elt_size, /* element size*/ @@ -1346,77 +1414,60 @@ struct rte_cryptodev * 0, /* private data size */ NULL, /* obj initialization constructor */ NULL, /* obj initialization constructor arg */ - rte_cryptodev_sym_session_init, + NULL /* rte_crypto_sym_session_init */, /**< obj constructor*/ - dev, /* obj constructor arg */ + NULL /* dev */, /* obj constructor arg */ socket_id, /* socket id */ 0); /* flags */ - if (dev->data->session_pool == NULL) { + if (mempool == NULL) { CDEV_LOG_ERR("%s mempool allocation failed", mp_name); - return -ENOMEM; + return NULL; } } CDEV_LOG_DEBUG("%s mempool created!", mp_name); - return 0; + + return mempool; } struct rte_cryptodev_sym_session * -rte_cryptodev_sym_session_create(uint8_t dev_id, - struct rte_crypto_sym_xform *xform) -{ - struct rte_cryptodev *dev; +rte_cryptodev_sym_session_create(struct rte_mempool *mempool) { struct rte_cryptodev_sym_session *sess; void *_sess; - if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { - CDEV_LOG_ERR("Invalid dev_id=%d", dev_id); - return NULL; - } - - dev = &rte_crypto_devices[dev_id]; - /* Allocate a session structure from the session pool */ - if (rte_mempool_get(dev->data->session_pool, &_sess)) { + if (rte_mempool_get(mempool, &_sess)) { CDEV_LOG_ERR("Couldn't get object from session mempool"); return NULL; } sess = _sess; - - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->session_configure, NULL); - if (dev->dev_ops->session_configure(dev, xform, sess->_private) == - NULL) { - CDEV_LOG_ERR("dev_id %d failed to configure session details", - dev_id); - - /* Return session to mempool */ - rte_mempool_put(sess->mp, _sess); - return NULL; - } + sess->mp = mempool; return sess; } int -rte_cryptodev_queue_pair_attach_sym_session(uint16_t qp_id, +rte_cryptodev_queue_pair_attach_sym_session(uint8_t dev_id, uint16_t qp_id, struct rte_cryptodev_sym_session *sess) { struct rte_cryptodev *dev; - if (!rte_cryptodev_pmd_is_valid_dev(sess->dev_id)) { - CDEV_LOG_ERR("Invalid dev_id=%d", sess->dev_id); + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { + CDEV_LOG_ERR("Invalid dev_id=%d", dev_id); return -EINVAL; } - dev = &rte_crypto_devices[sess->dev_id]; + dev = &rte_crypto_devices[dev_id]; /* The API is optional, not returning error if driver do not suuport */ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->qp_attach_session, 0); - if (dev->dev_ops->qp_attach_session(dev, qp_id, sess->_private)) { + if (dev->dev_ops->qp_attach_session(dev, qp_id, + rte_cryptodev_sym_sess_get_private_data(void, sess, + dev_id))) { CDEV_LOG_ERR("dev_id %d failed to attach qp: %d with session", - sess->dev_id, qp_id); + dev_id, qp_id); return -EPERM; } @@ -1424,53 +1475,34 @@ struct rte_cryptodev_sym_session * } int -rte_cryptodev_queue_pair_detach_sym_session(uint16_t qp_id, +rte_cryptodev_queue_pair_detach_sym_session(uint8_t dev_id, uint16_t qp_id, struct rte_cryptodev_sym_session *sess) { struct rte_cryptodev *dev; - if (!rte_cryptodev_pmd_is_valid_dev(sess->dev_id)) { - CDEV_LOG_ERR("Invalid dev_id=%d", sess->dev_id); + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { + CDEV_LOG_ERR("Invalid dev_id=%d", dev_id); return -EINVAL; } - dev = &rte_crypto_devices[sess->dev_id]; + dev = &rte_crypto_devices[dev_id]; /* The API is optional, not returning error if driver do not suuport */ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->qp_detach_session, 0); - if (dev->dev_ops->qp_detach_session(dev, qp_id, sess->_private)) { + if (dev->dev_ops->qp_detach_session(dev, qp_id, + rte_cryptodev_sym_sess_get_private_data(void, sess, + dev_id))) { CDEV_LOG_ERR("dev_id %d failed to detach qp: %d from session", - sess->dev_id, qp_id); + dev_id, qp_id); return -EPERM; } return 0; } -struct rte_cryptodev_sym_session * -rte_cryptodev_sym_session_free(uint8_t dev_id, - struct rte_cryptodev_sym_session *sess) -{ - struct rte_cryptodev *dev; - - if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { - CDEV_LOG_ERR("Invalid dev_id=%d", dev_id); - return sess; - } - dev = &rte_crypto_devices[dev_id]; - - /* Check the session belongs to this device type */ - if (sess->dev_type != dev->dev_type) - return sess; - - /* Let device implementation clear session material */ - RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->session_clear, sess); - dev->dev_ops->session_clear(dev, (void *)sess->_private); - - /* Return session to mempool */ - rte_mempool_put(sess->mp, (void *)sess); +void +rte_cryptodev_sym_session_free(__rte_unused struct rte_cryptodev_sym_session *sess) { - return NULL; } /** Initialise rte_crypto_op mempool element */ diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h index 88aeb87..3b9b894 100644 --- a/lib/librte_cryptodev/rte_cryptodev.h +++ b/lib/librte_cryptodev/rte_cryptodev.h @@ -866,65 +866,49 @@ struct rte_cryptodev_data { /** Cryptodev symmetric crypto session */ struct rte_cryptodev_sym_session { - RTE_STD_C11 - struct { - uint8_t dev_id; - /**< Device Id */ - enum rte_cryptodev_type dev_type; - /** Crypto Device type session created on */ - struct rte_mempool *mp; - /**< Mempool session allocated from */ - } __rte_aligned(8); - /**< Public symmetric session details */ - - __extension__ char _private[0]; - /**< Private session material */ + struct rte_mempool *mp; /**< Mempool session allocated from */ + uint32_t sess_mask; /**< Mask containing which private data + is set (faster free) */ + void *sess_private_data[32]; /**< Device type specific session private + data */ }; +/**< + * Frees symmetric crypto session and all related device type specific + * private data objects associated with session + */ +void +rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess); -/** - * Initialise a session for symmetric cryptographic operations. - * - * This function is used by the client to initialize immutable - * parameters of symmetric cryptographic operation. - * To perform the operation the rte_cryptodev_enqueue_burst function is - * used. Each mbuf should contain a reference to the session - * pointer returned from this function contained within it's crypto_op if a - * session-based operation is being provisioned. Memory to contain the session - * information is allocated from within mempool managed by the cryptodev. - * - * The rte_cryptodev_session_free must be called to free allocated - * memory when the session is no longer required. - * - * @param dev_id The device identifier. - * @param xform Crypto transform chain. - +/**< + * Fill out private data for the device id, based on its device type * - * @return - * Pointer to the created session or NULL + * @param dev_id ID of device that we want the session to be used on + * @param sess Session where the private data will be attached to + * @param xforms Symmetric crypto transform operations to apply on flow + * processed with this session */ -extern struct rte_cryptodev_sym_session * -rte_cryptodev_sym_session_create(uint8_t dev_id, - struct rte_crypto_sym_xform *xform); +int +rte_cryptodev_sym_session_init(uint8_t dev_id, + struct rte_cryptodev_sym_session *sess, + struct rte_crypto_sym_xform *xforms); -/** - * Free the memory associated with a previously allocated session. - * - * @param dev_id The device identifier. - * @param session Session pointer previously allocated by - * *rte_cryptodev_sym_session_create*. +/**< + * Create symmetric crypto session (generic with no private data) * + * @param mempool Symmetric session mempool to allocate session + * objects from * @return - * NULL on successful freeing of session. - * Session pointer on failure to free session. + * - On success return pointer to sym-session + * - On failure returns NULL */ -extern struct rte_cryptodev_sym_session * -rte_cryptodev_sym_session_free(uint8_t dev_id, - struct rte_cryptodev_sym_session *session); +struct rte_cryptodev_sym_session * +rte_cryptodev_sym_session_create(struct rte_mempool *mempool); /** * Attach queue pair with sym session. * + * @param dev_id The identifier of the device. * @param qp_id Queue pair to which session will be attached. * @param session Session pointer previously allocated by * *rte_cryptodev_sym_session_create*. @@ -934,12 +918,13 @@ struct rte_cryptodev_sym_session { * - On failure, a negative value. */ int -rte_cryptodev_queue_pair_attach_sym_session(uint16_t qp_id, +rte_cryptodev_queue_pair_attach_sym_session(uint8_t dev_id, uint16_t qp_id, struct rte_cryptodev_sym_session *session); /** * Detach queue pair with sym session. * + * @param dev_id The identifier of the device. * @param qp_id Queue pair to which session is attached. * @param session Session pointer previously allocated by * *rte_cryptodev_sym_session_create*. @@ -949,9 +934,29 @@ struct rte_cryptodev_sym_session { * - On failure, a negative value. */ int -rte_cryptodev_queue_pair_detach_sym_session(uint16_t qp_id, +rte_cryptodev_queue_pair_detach_sym_session(uint8_t dev_id, uint16_t qp_id, struct rte_cryptodev_sym_session *session); +struct rte_mempool * +rte_cryptodev_sym_session_pool_create(unsigned int nb_objs, + unsigned int *device_ids, unsigned int nb_device_ids, + unsigned int obj_cache_size, unsigned int socket_id); + +static inline void +set_session_private_data(struct rte_cryptodev_sym_session *sess, + unsigned int device_type_id, void *private_data) { + sess->sess_private_data[device_type_id] = private_data; +} + + +static inline void * +get_session_private_data(struct rte_cryptodev_sym_session *sess, + unsigned int device_type_id) { + return sess->sess_private_data[device_type_id]; +} + +#define rte_cryptodev_sym_sess_get_private_data(type, sess, id) \ + (type *)get_session_private_data(sess, id) #ifdef __cplusplus }