Hi Konstantin,
Sorry for the delay in reviewing this and thanks for your proposals.
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, August 24, 2018 10:48 AM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> <declan.doherty@intel.com>; Ravi Kumar <ravi1.kumar@amd.com>; Jerin Jacob
> <jerin.jacob@caviumnetworks.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Tomasz Duszynski <tdu@semihalf.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Natalie Samsonov <nsamsono@marvell.com>; Dmitri Epshtein
> <dima@marvell.com>; Jay Zhou <jianjay.zhou@huawei.com>
> Subject: [RFC] cryptodev: proposed changes in rte_cryptodev_sym_session
>
> This RFC for proposes several changes inside rte_cryptodev_sym_session.
> Note that this is just RFC not a complete patch, so for now
> I modified only the librte_cryptodev itself,
> some cryptodev PMD, test-crypto-perf and ipsec-secgw example.
> Proposed changes means ABI/API breakage inside cryptodev,
> so looking for feedback from crypto-dev lib and crypto-PMD maintainiers.
> Below are details and reasoning for proposed changes.
>
> 1.rte_cryptodev_sym_session_init()/ rte_cryptodev_sym_session_clear()
> operate based on cytpodev device id, though inside
> rte_cryptodev_sym_session device specific data is addressed
> by driver id (not device id).
> That creates a problem with current implementation when we have
> two or more devices with the same driver used by the same session.
> Consider the following example:
>
> struct rte_cryptodev_sym_session *sess;
> rte_cryptodev_sym_session_init(dev_id=X, sess, ...);
> rte_cryptodev_sym_session_init(dev_id=Y, sess, ...);
> rte_cryptodev_sym_session_clear(dev_id=X, sess);
>
> After that point if X and Y uses the same driver,
> then sess can't be used by device Y any more.
> The reason for that - driver specific (not device specific)
> data per session, plus there is no information
> how many device instances use that data.
> Probably the simplest way to deal with that issue -
> add a reference counter per each driver data.
[Fiona] Ok, I agree with this issue and proposed fix.
We need to also document that it's user's responsibility
not to call either init() or clear() twice on same device, as
that would break the ref count.
The same should be added to asym_session - though I accept
it'sbe outside of the scope of this patch.
> 2.rte_cryptodev_sym_session_set_user_data() and
> rte_cryptodev_sym_session_get_user_data() -
> with current implementation there is no defined way for the user to
> determine what is the max allowed size of the private data.
> Even within rte_cryptodev_sym_session_set_user_data() we just blindly
> copying user provided data without checking memory boundaries violation.
> To overcome that issue I added 'uint16_t priv_size' into
> rte_cryptodev_sym_session structure.
[Fiona] I agree, this is needed.
But I propose to call it user_data_sz NOT priv_size.
See https://patches.dpdk.org/patch/42515/ to understand why.
> 3.rte_cryptodev_sym_session contains an array of variable size for
> driver specific data.
> Though number of elements in that array is determined by static
> variable nb_drivers, that could be modified by
> rte_cryptodev_allocate_driver().
> That construction seems to work ok so far, as right now users register
> all their PMDs at startup, though it doesn't mean that it would always
> remain like that.
> To make it less error prone I added 'uint16_t nb_drivers' into the
> rte_cryptodev_sym_session structure.
> At least that allows related functions to check that provided
> driver id wouldn't overrun variable array boundaries,
> again it allows to determine size of already allocated session
> without accessing global variable.
[Fiona] I agree with both issue and solution.
The same should be added to asym_session - though again
it's outside of the scope of this patch.
> 4.#2 and #3 above implies that now each struct rte_cryptodev_sym_session
> would have sort of readonly type data (init once at allocation time,
> keep unmodified through session life-time).
> That requires more changes in current cryptodev implementation:
> Right now inside cryptodev framework both rte_cryptodev_sym_session
> and driver specific session data are two completely different sctrucures
> (e.g. struct struct null_crypto_session and struct null_crypto_session).
> Though current cryptodev implementation implicitly assumes that driver
> will allocate both of them from within the same mempool.
> Plus this is done in a manner that they override each other fields
> (reuse the same space - sort of implicit C union).
> That's probably not the best programming practice,
> plus make impossible to have readonly fields inside both of them.
> So to overcome that situation I changed an API a bit, to allow
> to use two different mempools for these two distinct data structures.
>
> 5. Add 'uint64_t userdata' inside struct rte_cryptodev_sym_session.
> I suppose that self-explanatory, and might be used in a lot of places
> (would be quite useful for ipsec library we develop).
[Fiona] Seems unnecessary - the set_user_data can be used. Why have 2
separate user data spaces in the session? - it's confusing.
If these is some good reason, then a different name should be used for clarity.
Not private. And not user. Possibly opaque data. Though afaik we had this in the op
and removed it as it was felt appending user_data was enough.
> So the new proposed layout for rte_cryptodev_sym_session:
> struct rte_cryptodev_sym_session {
> uint64_t userdata;
> /**< Can be used for external metadata */
> uint16_t nb_drivers;
> /**< number of elements in sess_data array */
> uint16_t priv_size;
> /**< session private data will be placed after sess_data */
> __extension__ struct {
> void *data;
> uint16_t refcnt;
> } sess_data[0];
> /**< Driver specific session material, variable size */
> };
>
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
> app/test-crypto-perf/cperf.h | 1 +
> app/test-crypto-perf/cperf_ops.c | 11 +-
> app/test-crypto-perf/cperf_ops.h | 2 +-
> app/test-crypto-perf/cperf_test_latency.c | 5 +-
> app/test-crypto-perf/cperf_test_latency.h | 1 +
> app/test-crypto-perf/cperf_test_pmd_cyclecount.c | 5 +-
> app/test-crypto-perf/cperf_test_pmd_cyclecount.h | 1 +
> app/test-crypto-perf/cperf_test_throughput.c | 5 +-
> app/test-crypto-perf/cperf_test_throughput.h | 1 +
> app/test-crypto-perf/cperf_test_verify.c | 5 +-
> app/test-crypto-perf/cperf_test_verify.h | 1 +
> app/test-crypto-perf/main.c | 111 +++++++++++------
> drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 10 +-
> drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 5 +-
> drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 4 +-
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 10 +-
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 5 +-
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 4 +-
> drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 3 +-
> drivers/crypto/dpaa_sec/dpaa_sec.c | 3 +-
> drivers/crypto/null/null_crypto_pmd.c | 14 ++-
> drivers/crypto/null/null_crypto_pmd_ops.c | 5 +-
> drivers/crypto/null/null_crypto_pmd_private.h | 4 +-
> drivers/crypto/scheduler/scheduler_pmd_ops.c | 5 +-
> drivers/crypto/virtio/virtio_cryptodev.c | 6 +-
> examples/ipsec-secgw/ipsec-secgw.c | 116 ++++++++++++------
> examples/ipsec-secgw/ipsec.h | 2 +
> lib/librte_cryptodev/rte_cryptodev.c | 134 ++++++++++++---------
> lib/librte_cryptodev/rte_cryptodev.h | 53 ++++++--
> lib/librte_cryptodev/rte_cryptodev_pmd.h | 16 ++-
> 30 files changed, 356 insertions(+), 192 deletions(-)
>
///snip///
> struct cnt_blk {
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> index 63ae23f00..e25282445 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -943,8 +943,7 @@ rte_cryptodev_close(uint8_t dev_id)
>
> int
> rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> - const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
> - struct rte_mempool *session_pool)
> + const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
>
> {
> struct rte_cryptodev *dev;
> @@ -954,6 +953,12 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> return -EINVAL;
> }
>
> + if (qp_conf == NULL || qp_conf->sess_pool == NULL ||
> + qp_conf->priv_sess_pool == NULL) {
> + CDEV_LOG_ERR("Invalid queue_pair config");
> + return -EINVAL;
> + }
> +
> dev = &rte_crypto_devices[dev_id];
> if (queue_pair_id >= dev->data->nb_queue_pairs) {
> CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
> @@ -969,7 +974,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_setup, -ENOTSUP);
>
> return (*dev->dev_ops->queue_pair_setup)(dev, queue_pair_id, qp_conf,
> - socket_id, session_pool);
> + socket_id);
> }
>
>
> @@ -1146,6 +1151,41 @@ rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
> rte_spinlock_unlock(&rte_cryptodev_cb_lock);
> }
>
> +static void
> +cryptodev_sym_session_init_elem(__rte_unused struct rte_mempool *pool,
> + void *arg, void *obj, __rte_unused uint32_t idx)
> +{
> + struct rte_cryptodev_sym_session *ds;
> + const struct rte_cryptodev_sym_session *ss;
> +
> + ds = obj;
> + ss = arg;
> +
> + *ds = *ss;
> + memset(ds->sess_data, 0, rte_cryptodev_sym_session_data_size(ds));
> +}
> +
> +struct rte_mempool *
> +rte_cryptodev_sym_session_pool_create(const char *name,
> + uint32_t nb_elts, uint32_t cache_size, uint16_t priv_size,
> + int socket_id)
> +{
> + struct rte_mempool *mp;
> + uint32_t elt_size;
> + struct rte_cryptodev_sym_session s = {
> + .nb_drivers = nb_drivers,
> + .priv_size = priv_size,
> + };
> +
> + elt_size = rte_cryptodev_sym_session_max_size(priv_size);
> + mp = rte_mempool_create(name, nb_elts, elt_size, cache_size, 0,
> + NULL, NULL, cryptodev_sym_session_init_elem, &s,
> + socket_id, 0);
> + if (mp == NULL)
> + CDEV_LOG_ERR("%s(name=%s) failed, rte_errno=%d\n",
> + __func__, name, rte_errno);
> + return mp;
> +}
>
> int
> rte_cryptodev_sym_session_init(uint8_t dev_id,
> @@ -1163,12 +1203,15 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
> return -EINVAL;
>
> index = dev->driver_id;
> + if (index > sess->nb_drivers)
> + return -EINVAL;
>
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -ENOTSUP);
>
> - if (sess->sess_private_data[index] == NULL) {
> + if (sess->sess_data[index].refcnt == 0) {
> ret = dev->dev_ops->sym_session_configure(dev, xforms,
> - sess, mp);
> + sess, mp);
> +
> if (ret < 0) {
> CDEV_LOG_ERR(
> "dev_id %d failed to configure session details",
> @@ -1177,6 +1220,7 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
> }
> }
>
> + sess->sess_data[index].refcnt++;
> return 0;
> }
>
> @@ -1229,8 +1273,7 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
> /* Clear device session pointer.
> * Include the flag indicating presence of user data
> */
> - memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> -
> + memset(sess->sess_data, 0, rte_cryptodev_sym_session_data_size(sess));
> return sess;
> }
>
> @@ -1258,16 +1301,20 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
> struct rte_cryptodev_sym_session *sess)
> {
> struct rte_cryptodev *dev;
> + uint32_t idx;
>
> dev = rte_cryptodev_pmd_get_dev(dev_id);
>
> - if (dev == NULL || sess == NULL)
> + if (dev == NULL || sess == NULL || dev->driver_id > sess->nb_drivers)
> return -EINVAL;
>
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_clear, -ENOTSUP);
>
> - dev->dev_ops->sym_session_clear(dev, sess);
> + idx = dev->driver_id;
> + if (--sess->sess_data[idx].refcnt != 0)
> + return -EBUSY;
>
> + dev->dev_ops->sym_session_clear(dev, sess);
> return 0;
> }
>
> @@ -1285,7 +1332,6 @@ rte_cryptodev_asym_session_clear(uint8_t dev_id,
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_clear, -ENOTSUP);
>
> dev->dev_ops->asym_session_clear(dev, sess);
> -
> return 0;
> }
>
> @@ -1293,7 +1339,6 @@ int
> rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
> {
> uint8_t i;
> - void *sess_priv;
> struct rte_mempool *sess_mp;
>
> if (sess == NULL)
> @@ -1301,8 +1346,7 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
>
> /* Check that all device private data has been freed */
> for (i = 0; i < nb_drivers; i++) {
[Fiona] Use the sess.nb_drivers rather than the global.
Actually maybe name slightly differently to reduce the chance of that mistake happening, e.g.
rename sess.nb_drivers to sess.num_drivers?
> - sess_priv = get_sym_session_private_data(sess, i);
> - if (sess_priv != NULL)
> + if (sess->sess_data[i].refcnt != 0)
> return -EBUSY;
> }
>
> @@ -1313,6 +1357,23 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
> return 0;
> }
>
> +unsigned int
> +rte_cryptodev_sym_session_max_data_size(void)
[Fiona] Suggest renaming this
rte_cryptodev_sym_session_max_array_size()
> +{
> + struct rte_cryptodev_sym_session *sess = NULL;
> +
> + return (sizeof(sess->sess_data[0]) * nb_drivers);
> +}
> +
> +size_t
> +rte_cryptodev_sym_session_max_size(uint16_t priv_size)
> +{
> + struct rte_cryptodev_sym_session *sess = NULL;
> +
> + return (sizeof(*sess) + priv_size +
> + rte_cryptodev_sym_session_max_data_size());
> +}
> +
> int __rte_experimental
> rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
> {
> @@ -1337,18 +1398,6 @@ rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
> return 0;
> }
>
> -
> -unsigned int
> -rte_cryptodev_sym_get_header_session_size(void)
> -{
> - /*
> - * Header contains pointers to the private data
> - * of all registered drivers, and a flag which
> - * indicates presence of user data
> - */
> - return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> -}
> -
> unsigned int __rte_experimental
> rte_cryptodev_asym_get_header_session_size(void)
> {
> @@ -1361,11 +1410,9 @@ rte_cryptodev_asym_get_header_session_size(void)
> }
>
> unsigned int
> -rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
> +rte_cryptodev_sym_private_session_size(uint8_t dev_id)
> {
> struct rte_cryptodev *dev;
> - unsigned int header_size = sizeof(void *) * nb_drivers;
> - unsigned int priv_sess_size;
>
> if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
> return 0;
> @@ -1375,18 +1422,7 @@ rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
> if (*dev->dev_ops->sym_session_get_size == NULL)
> return 0;
>
> - priv_sess_size = (*dev->dev_ops->sym_session_get_size)(dev);
> -
> - /*
> - * If size is less than session header size,
> - * return the latter, as this guarantees that
> - * sessionless operations will work
> - */
> - if (priv_sess_size < header_size)
> - return header_size;
> -
> - return priv_sess_size;
> -
> + return (*dev->dev_ops->sym_session_get_size)(dev);
> }
>
> unsigned int __rte_experimental
> @@ -1409,7 +1445,6 @@ rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
> return header_size;
>
> return priv_sess_size;
> -
> }
>
> int __rte_experimental
> @@ -1418,15 +1453,10 @@ rte_cryptodev_sym_session_set_user_data(
> void *data,
> uint16_t size)
> {
> - uint16_t off_set = sizeof(void *) * nb_drivers;
> - uint8_t *user_data_present = (uint8_t *)sess + off_set;
> -
> - if (sess == NULL)
> + if (sess == NULL || sess->priv_size < size)
> return -EINVAL;
>
> - *user_data_present = 1;
> - off_set += sizeof(uint8_t);
> - rte_memcpy((uint8_t *)sess + off_set, data, size);
> + rte_memcpy(sess->sess_data + sess->nb_drivers, data, size);
> return 0;
> }
>
> @@ -1434,14 +1464,10 @@ void * __rte_experimental
> rte_cryptodev_sym_session_get_user_data(
> struct rte_cryptodev_sym_session *sess)
> {
> - uint16_t off_set = sizeof(void *) * nb_drivers;
> - uint8_t *user_data_present = (uint8_t *)sess + off_set;
> -
> - if (sess == NULL || !*user_data_present)
> + if (sess == NULL || sess->priv_size == 0)
> return NULL;
>
> - off_set += sizeof(uint8_t);
> - return (uint8_t *)sess + off_set;
> + return (sess->sess_data + sess->nb_drivers);
> }
>
> /** Initialise rte_crypto_op mempool element */
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index 4099823f1..d88454f02 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -495,6 +495,14 @@ enum rte_cryptodev_event_type {
> /** Crypto device queue pair configuration structure. */
> struct rte_cryptodev_qp_conf {
> uint32_t nb_descriptors; /**< Number of descriptors per queue pair */
> + struct rte_mempool *sess_pool;
> + /**< Pointer to crypto sessions mempool,
> + * used for session-less operations.
> + */
> + struct rte_mempool *priv_sess_pool;
> + /**< Pointer to device specific sessions mempool,
> + * used for session-less operations.
> + */
> };
>
> /**
> @@ -680,17 +688,13 @@ rte_cryptodev_close(uint8_t dev_id);
> * *SOCKET_ID_ANY* if there is no NUMA constraint
> * for the DMA memory allocated for the receive
> * queue pair.
> - * @param session_pool Pointer to device session mempool, used
> - * for session-less operations.
> - *
> * @return
> * - 0: Success, queue pair correctly set up.
> * - <0: Queue pair configuration failed
> */
> extern int
> rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> - const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
> - struct rte_mempool *session_pool);
> + const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
>
> /**
> * Get the number of queue pairs on a specific crypto device
> @@ -954,10 +958,43 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> * has a fixed algo, key, op-type, digest_len etc.
> */
> struct rte_cryptodev_sym_session {
> - __extension__ void *sess_private_data[0];
> - /**< Private symmetric session material */
> + uint64_t userdata;
> + /**< Can be used for external metadata */
> + uint16_t nb_drivers;
> + /**< number of elements in sess_data array */
> + uint16_t priv_size;
> + /**< session private data will be placed after sess_data */
> + __extension__ struct {
> + void *data;
> + uint16_t refcnt;
> + } sess_data[0];
> + /**< Driver specific session material, variable size */
> };
>
> +static inline size_t
> +rte_cryptodev_sym_session_data_size(const struct rte_cryptodev_sym_session *s)
> +{
> + return (sizeof(s->sess_data[0]) * s->nb_drivers);
> +}
> +
> +static inline size_t
> +rte_cryptodev_sym_session_size(const struct rte_cryptodev_sym_session *s)
> +{
> + return (sizeof(*s) + (s)->priv_size +
> + rte_cryptodev_sym_session_data_size(s));
> +}
> +
[Fiona] Are above 2 fns used?
Look like duplicates of the "max" fns?
> +unsigned int
> +rte_cryptodev_sym_session_max_data_size(void);
> +
> +size_t
> +rte_cryptodev_sym_session_max_size(uint16_t priv_size);
> +
> +struct rte_mempool *
> +rte_cryptodev_sym_session_pool_create(const char *name,
> + uint32_t nb_elts, uint32_t cache_size, uint16_t priv_size,
> + int socket_id);
> +
> /** Cryptodev asymmetric crypto session */
> struct rte_cryptodev_asym_session {
> __extension__ void *sess_private_data[0];
> @@ -1123,7 +1160,7 @@ rte_cryptodev_asym_get_header_session_size(void);
> * symmetric session
> */
> unsigned int
> -rte_cryptodev_sym_get_private_session_size(uint8_t dev_id);
> +rte_cryptodev_sym_private_session_size(uint8_t dev_id);
>
> /**
> * Get the size of the private data for asymmetric session
> diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> index 6ff49d64d..2f98f65d1 100644
> --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> @@ -191,13 +191,12 @@ typedef void (*cryptodev_info_get_t)(struct rte_cryptodev *dev,
> * @param qp_id Queue Pair Index
> * @param qp_conf Queue configuration structure
> * @param socket_id Socket Index
> - * @param session_pool Pointer to device session mempool
> *
> * @return Returns 0 on success.
> */
> typedef int (*cryptodev_queue_pair_setup_t)(struct rte_cryptodev *dev,
> uint16_t qp_id, const struct rte_cryptodev_qp_conf *qp_conf,
> - int socket_id, struct rte_mempool *session_pool);
> + int socket_id);
>
> /**
> * Release memory resources allocated by given queue pair.
> @@ -478,20 +477,25 @@ RTE_INIT(init_ ##driver_id)\
>
> static inline void *
> get_sym_session_private_data(const struct rte_cryptodev_sym_session *sess,
> - uint8_t driver_id) {
> - return sess->sess_private_data[driver_id];
> + uint8_t driver_id)
> +{
> + if (driver_id < sess->nb_drivers)
> + return sess->sess_data[driver_id].data;
> + return NULL;
> }
>
> static inline void
> set_sym_session_private_data(struct rte_cryptodev_sym_session *sess,
> uint8_t driver_id, void *private_data)
> {
> - sess->sess_private_data[driver_id] = private_data;
> + if (driver_id < sess->nb_drivers)
> + sess->sess_data[driver_id].data = private_data;
> }
>
> static inline void *
> get_asym_session_private_data(const struct rte_cryptodev_asym_session *sess,
> - uint8_t driver_id) {
> + uint8_t driver_id)
> +{
> return sess->sess_private_data[driver_id];
> }
>
> --
> 2.13.6
Hi Fiona,
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Monday, November 12, 2018 9:01 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> <declan.doherty@intel.com>; Ravi Kumar <ravi1.kumar@amd.com>; Jerin Jacob <jerin.jacob@caviumnetworks.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Tomasz Duszynski <tdu@semihalf.com>; Hemant Agrawal <hemant.agrawal@nxp.com>; Natalie Samsonov
> <nsamsono@marvell.com>; Dmitri Epshtein <dima@marvell.com>; Jay Zhou <jianjay.zhou@huawei.com>; Trahe, Fiona
> <fiona.trahe@intel.com>
> Subject: RE: [RFC] cryptodev: proposed changes in rte_cryptodev_sym_session
>
> Hi Konstantin,
> Sorry for the delay in reviewing this and thanks for your proposals.
NP, thanks for review.
My comments/answers inline.
Can you also have a look at related deprecation note:
http://patches.dpdk.org/patch/46633/
and provide the feedback?
Konstantin
>
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Friday, August 24, 2018 10:48 AM
> > To: dev@dpdk.org
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; Doherty, Declan
> > <declan.doherty@intel.com>; Ravi Kumar <ravi1.kumar@amd.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Trahe, Fiona
> > <fiona.trahe@intel.com>; Tomasz Duszynski <tdu@semihalf.com>; Hemant Agrawal
> > <hemant.agrawal@nxp.com>; Natalie Samsonov <nsamsono@marvell.com>; Dmitri Epshtein
> > <dima@marvell.com>; Jay Zhou <jianjay.zhou@huawei.com>
> > Subject: [RFC] cryptodev: proposed changes in rte_cryptodev_sym_session
> >
> > This RFC for proposes several changes inside rte_cryptodev_sym_session.
> > Note that this is just RFC not a complete patch, so for now
> > I modified only the librte_cryptodev itself,
> > some cryptodev PMD, test-crypto-perf and ipsec-secgw example.
> > Proposed changes means ABI/API breakage inside cryptodev,
> > so looking for feedback from crypto-dev lib and crypto-PMD maintainiers.
> > Below are details and reasoning for proposed changes.
> >
> > 1.rte_cryptodev_sym_session_init()/ rte_cryptodev_sym_session_clear()
> > operate based on cytpodev device id, though inside
> > rte_cryptodev_sym_session device specific data is addressed
> > by driver id (not device id).
> > That creates a problem with current implementation when we have
> > two or more devices with the same driver used by the same session.
> > Consider the following example:
> >
> > struct rte_cryptodev_sym_session *sess;
> > rte_cryptodev_sym_session_init(dev_id=X, sess, ...);
> > rte_cryptodev_sym_session_init(dev_id=Y, sess, ...);
> > rte_cryptodev_sym_session_clear(dev_id=X, sess);
> >
> > After that point if X and Y uses the same driver,
> > then sess can't be used by device Y any more.
> > The reason for that - driver specific (not device specific)
> > data per session, plus there is no information
> > how many device instances use that data.
> > Probably the simplest way to deal with that issue -
> > add a reference counter per each driver data.
> [Fiona] Ok, I agree with this issue and proposed fix.
> We need to also document that it's user's responsibility
> not to call either init() or clear() twice on same device, as
> that would break the ref count.
I suppose it is obvious constrain, but sure, extra wording
can be put into the comments/docs, np with that.
> The same should be added to asym_session - though I accept
> it'sbe outside of the scope of this patch.
Agree on both - yes similar changes need to be done for asym,
and yes that patch targets sym session only.
>
>
> > 2.rte_cryptodev_sym_session_set_user_data() and
> > rte_cryptodev_sym_session_get_user_data() -
> > with current implementation there is no defined way for the user to
> > determine what is the max allowed size of the private data.
> > Even within rte_cryptodev_sym_session_set_user_data() we just blindly
> > copying user provided data without checking memory boundaries violation.
> > To overcome that issue I added 'uint16_t priv_size' into
> > rte_cryptodev_sym_session structure.
> [Fiona] I agree, this is needed.
> But I propose to call it user_data_sz NOT priv_size.
> See https://patches.dpdk.org/patch/42515/ to understand why.
Hmm that differs with mbuf naming scheme
(which I tried to follow here), but ok -
I don't have strong opinion here.
>
>
> > 3.rte_cryptodev_sym_session contains an array of variable size for
> > driver specific data.
> > Though number of elements in that array is determined by static
> > variable nb_drivers, that could be modified by
> > rte_cryptodev_allocate_driver().
> > That construction seems to work ok so far, as right now users register
> > all their PMDs at startup, though it doesn't mean that it would always
> > remain like that.
> > To make it less error prone I added 'uint16_t nb_drivers' into the
> > rte_cryptodev_sym_session structure.
> > At least that allows related functions to check that provided
> > driver id wouldn't overrun variable array boundaries,
> > again it allows to determine size of already allocated session
> > without accessing global variable.
> [Fiona] I agree with both issue and solution.
> The same should be added to asym_session - though again
> it's outside of the scope of this patch.
>
> > 4.#2 and #3 above implies that now each struct rte_cryptodev_sym_session
> > would have sort of readonly type data (init once at allocation time,
> > keep unmodified through session life-time).
> > That requires more changes in current cryptodev implementation:
> > Right now inside cryptodev framework both rte_cryptodev_sym_session
> > and driver specific session data are two completely different sctrucures
> > (e.g. struct struct null_crypto_session and struct null_crypto_session).
> > Though current cryptodev implementation implicitly assumes that driver
> > will allocate both of them from within the same mempool.
> > Plus this is done in a manner that they override each other fields
> > (reuse the same space - sort of implicit C union).
> > That's probably not the best programming practice,
> > plus make impossible to have readonly fields inside both of them.
> > So to overcome that situation I changed an API a bit, to allow
> > to use two different mempools for these two distinct data structures.
> >
> > 5. Add 'uint64_t userdata' inside struct rte_cryptodev_sym_session.
> > I suppose that self-explanatory, and might be used in a lot of places
> > (would be quite useful for ipsec library we develop).
> [Fiona] Seems unnecessary - the set_user_data can be used. Why have 2
> separate user data spaces in the session? - it's confusing.
It allows quickly set/get external metadata associated with that session.
As an example - we plan to use it for pointer to ipsec SA associated
with given session.
Storing it inside priv_data section (user_data in your naming convention)
has several limitations:
- extra function call and extra memory dereference.
- each app would have to take into account that field when calculates size
for session mempool element.
Also note that inside one app could exist several session pools,
possibly with different layout for user private data,
unknown for generic libs.
Again, here I just used current mbuf approach:
userdata - (pointer to) some external metadata
(possibly temporally) associated with given mbuf.
priv_size (you suggest to call it user_data_sz) -
size of the application private data for given mbuf.
> If these is some good reason, then a different name should be used for clarity.
> Not private. And not user. Possibly opaque data.
Ok.
> Though afaik we had this in the op
> and removed it as it was felt appending user_data was enough.
>
> > So the new proposed layout for rte_cryptodev_sym_session:
> > struct rte_cryptodev_sym_session {
> > uint64_t userdata;
> > /**< Can be used for external metadata */
> > uint16_t nb_drivers;
> > /**< number of elements in sess_data array */
> > uint16_t priv_size;
> > /**< session private data will be placed after sess_data */
> > __extension__ struct {
> > void *data;
> > uint16_t refcnt;
> > } sess_data[0];
> > /**< Driver specific session material, variable size */
> > };
> >
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> > app/test-crypto-perf/cperf.h | 1 +
> > app/test-crypto-perf/cperf_ops.c | 11 +-
> > app/test-crypto-perf/cperf_ops.h | 2 +-
> > app/test-crypto-perf/cperf_test_latency.c | 5 +-
> > app/test-crypto-perf/cperf_test_latency.h | 1 +
> > app/test-crypto-perf/cperf_test_pmd_cyclecount.c | 5 +-
> > app/test-crypto-perf/cperf_test_pmd_cyclecount.h | 1 +
> > app/test-crypto-perf/cperf_test_throughput.c | 5 +-
> > app/test-crypto-perf/cperf_test_throughput.h | 1 +
> > app/test-crypto-perf/cperf_test_verify.c | 5 +-
> > app/test-crypto-perf/cperf_test_verify.h | 1 +
> > app/test-crypto-perf/main.c | 111 +++++++++++------
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 10 +-
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 5 +-
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 4 +-
> > drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 10 +-
> > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 5 +-
> > drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 4 +-
> > drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 3 +-
> > drivers/crypto/dpaa_sec/dpaa_sec.c | 3 +-
> > drivers/crypto/null/null_crypto_pmd.c | 14 ++-
> > drivers/crypto/null/null_crypto_pmd_ops.c | 5 +-
> > drivers/crypto/null/null_crypto_pmd_private.h | 4 +-
> > drivers/crypto/scheduler/scheduler_pmd_ops.c | 5 +-
> > drivers/crypto/virtio/virtio_cryptodev.c | 6 +-
> > examples/ipsec-secgw/ipsec-secgw.c | 116 ++++++++++++------
> > examples/ipsec-secgw/ipsec.h | 2 +
> > lib/librte_cryptodev/rte_cryptodev.c | 134 ++++++++++++---------
> > lib/librte_cryptodev/rte_cryptodev.h | 53 ++++++--
> > lib/librte_cryptodev/rte_cryptodev_pmd.h | 16 ++-
> > 30 files changed, 356 insertions(+), 192 deletions(-)
> >
> ///snip///
>
> > struct cnt_blk {
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
> > index 63ae23f00..e25282445 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.c
> > +++ b/lib/librte_cryptodev/rte_cryptodev.c
> > @@ -943,8 +943,7 @@ rte_cryptodev_close(uint8_t dev_id)
> >
> > int
> > rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> > - const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
> > - struct rte_mempool *session_pool)
> > + const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
> >
> > {
> > struct rte_cryptodev *dev;
> > @@ -954,6 +953,12 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> > return -EINVAL;
> > }
> >
> > + if (qp_conf == NULL || qp_conf->sess_pool == NULL ||
> > + qp_conf->priv_sess_pool == NULL) {
> > + CDEV_LOG_ERR("Invalid queue_pair config");
> > + return -EINVAL;
> > + }
> > +
> > dev = &rte_crypto_devices[dev_id];
> > if (queue_pair_id >= dev->data->nb_queue_pairs) {
> > CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
> > @@ -969,7 +974,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_setup, -ENOTSUP);
> >
> > return (*dev->dev_ops->queue_pair_setup)(dev, queue_pair_id, qp_conf,
> > - socket_id, session_pool);
> > + socket_id);
> > }
> >
> >
> > @@ -1146,6 +1151,41 @@ rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
> > rte_spinlock_unlock(&rte_cryptodev_cb_lock);
> > }
> >
> > +static void
> > +cryptodev_sym_session_init_elem(__rte_unused struct rte_mempool *pool,
> > + void *arg, void *obj, __rte_unused uint32_t idx)
> > +{
> > + struct rte_cryptodev_sym_session *ds;
> > + const struct rte_cryptodev_sym_session *ss;
> > +
> > + ds = obj;
> > + ss = arg;
> > +
> > + *ds = *ss;
> > + memset(ds->sess_data, 0, rte_cryptodev_sym_session_data_size(ds));
> > +}
> > +
> > +struct rte_mempool *
> > +rte_cryptodev_sym_session_pool_create(const char *name,
> > + uint32_t nb_elts, uint32_t cache_size, uint16_t priv_size,
> > + int socket_id)
> > +{
> > + struct rte_mempool *mp;
> > + uint32_t elt_size;
> > + struct rte_cryptodev_sym_session s = {
> > + .nb_drivers = nb_drivers,
> > + .priv_size = priv_size,
> > + };
> > +
> > + elt_size = rte_cryptodev_sym_session_max_size(priv_size);
> > + mp = rte_mempool_create(name, nb_elts, elt_size, cache_size, 0,
> > + NULL, NULL, cryptodev_sym_session_init_elem, &s,
> > + socket_id, 0);
> > + if (mp == NULL)
> > + CDEV_LOG_ERR("%s(name=%s) failed, rte_errno=%d\n",
> > + __func__, name, rte_errno);
> > + return mp;
> > +}
> >
> > int
> > rte_cryptodev_sym_session_init(uint8_t dev_id,
> > @@ -1163,12 +1203,15 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
> > return -EINVAL;
> >
> > index = dev->driver_id;
> > + if (index > sess->nb_drivers)
> > + return -EINVAL;
> >
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -ENOTSUP);
> >
> > - if (sess->sess_private_data[index] == NULL) {
> > + if (sess->sess_data[index].refcnt == 0) {
> > ret = dev->dev_ops->sym_session_configure(dev, xforms,
> > - sess, mp);
> > + sess, mp);
> > +
> > if (ret < 0) {
> > CDEV_LOG_ERR(
> > "dev_id %d failed to configure session details",
> > @@ -1177,6 +1220,7 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
> > }
> > }
> >
> > + sess->sess_data[index].refcnt++;
> > return 0;
> > }
> >
> > @@ -1229,8 +1273,7 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
> > /* Clear device session pointer.
> > * Include the flag indicating presence of user data
> > */
> > - memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> > -
> > + memset(sess->sess_data, 0, rte_cryptodev_sym_session_data_size(sess));
> > return sess;
> > }
> >
> > @@ -1258,16 +1301,20 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
> > struct rte_cryptodev_sym_session *sess)
> > {
> > struct rte_cryptodev *dev;
> > + uint32_t idx;
> >
> > dev = rte_cryptodev_pmd_get_dev(dev_id);
> >
> > - if (dev == NULL || sess == NULL)
> > + if (dev == NULL || sess == NULL || dev->driver_id > sess->nb_drivers)
> > return -EINVAL;
> >
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_clear, -ENOTSUP);
> >
> > - dev->dev_ops->sym_session_clear(dev, sess);
> > + idx = dev->driver_id;
> > + if (--sess->sess_data[idx].refcnt != 0)
> > + return -EBUSY;
> >
> > + dev->dev_ops->sym_session_clear(dev, sess);
> > return 0;
> > }
> >
> > @@ -1285,7 +1332,6 @@ rte_cryptodev_asym_session_clear(uint8_t dev_id,
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_clear, -ENOTSUP);
> >
> > dev->dev_ops->asym_session_clear(dev, sess);
> > -
> > return 0;
> > }
> >
> > @@ -1293,7 +1339,6 @@ int
> > rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
> > {
> > uint8_t i;
> > - void *sess_priv;
> > struct rte_mempool *sess_mp;
> >
> > if (sess == NULL)
> > @@ -1301,8 +1346,7 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
> >
> > /* Check that all device private data has been freed */
> > for (i = 0; i < nb_drivers; i++) {
> [Fiona] Use the sess.nb_drivers rather than the global.
Ok, though doesn't really matter here.
get_sym_session_private_data() will return NULL for invalid index anyway.
> Actually maybe name slightly differently to reduce the chance of that mistake happening, e.g.
> rename sess.nb_drivers to sess.num_drivers?
>
> > - sess_priv = get_sym_session_private_data(sess, i);
> > - if (sess_priv != NULL)
> > + if (sess->sess_data[i].refcnt != 0)
> > return -EBUSY;
> > }
> >
> > @@ -1313,6 +1357,23 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
> > return 0;
> > }
> >
> > +unsigned int
> > +rte_cryptodev_sym_session_max_data_size(void)
> [Fiona] Suggest renaming this
> rte_cryptodev_sym_session_max_array_size()
I usually don't care much about names, but that seems just confusing:
totally unclear which array we are talking about.
Any better naming ideas?
>
> > +{
> > + struct rte_cryptodev_sym_session *sess = NULL;
> > +
> > + return (sizeof(sess->sess_data[0]) * nb_drivers);
> > +}
> > +
> > +size_t
> > +rte_cryptodev_sym_session_max_size(uint16_t priv_size)
> > +{
> > + struct rte_cryptodev_sym_session *sess = NULL;
> > +
> > + return (sizeof(*sess) + priv_size +
> > + rte_cryptodev_sym_session_max_data_size());
> > +}
> > +
> > int __rte_experimental
> > rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
> > {
> > @@ -1337,18 +1398,6 @@ rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
> > return 0;
> > }
> >
> > -
> > -unsigned int
> > -rte_cryptodev_sym_get_header_session_size(void)
> > -{
> > - /*
> > - * Header contains pointers to the private data
> > - * of all registered drivers, and a flag which
> > - * indicates presence of user data
> > - */
> > - return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> > -}
> > -
> > unsigned int __rte_experimental
> > rte_cryptodev_asym_get_header_session_size(void)
> > {
> > @@ -1361,11 +1410,9 @@ rte_cryptodev_asym_get_header_session_size(void)
> > }
> >
> > unsigned int
> > -rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
> > +rte_cryptodev_sym_private_session_size(uint8_t dev_id)
> > {
> > struct rte_cryptodev *dev;
> > - unsigned int header_size = sizeof(void *) * nb_drivers;
> > - unsigned int priv_sess_size;
> >
> > if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
> > return 0;
> > @@ -1375,18 +1422,7 @@ rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
> > if (*dev->dev_ops->sym_session_get_size == NULL)
> > return 0;
> >
> > - priv_sess_size = (*dev->dev_ops->sym_session_get_size)(dev);
> > -
> > - /*
> > - * If size is less than session header size,
> > - * return the latter, as this guarantees that
> > - * sessionless operations will work
> > - */
> > - if (priv_sess_size < header_size)
> > - return header_size;
> > -
> > - return priv_sess_size;
> > -
> > + return (*dev->dev_ops->sym_session_get_size)(dev);
> > }
> >
> > unsigned int __rte_experimental
> > @@ -1409,7 +1445,6 @@ rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
> > return header_size;
> >
> > return priv_sess_size;
> > -
> > }
> >
> > int __rte_experimental
> > @@ -1418,15 +1453,10 @@ rte_cryptodev_sym_session_set_user_data(
> > void *data,
> > uint16_t size)
> > {
> > - uint16_t off_set = sizeof(void *) * nb_drivers;
> > - uint8_t *user_data_present = (uint8_t *)sess + off_set;
> > -
> > - if (sess == NULL)
> > + if (sess == NULL || sess->priv_size < size)
> > return -EINVAL;
> >
> > - *user_data_present = 1;
> > - off_set += sizeof(uint8_t);
> > - rte_memcpy((uint8_t *)sess + off_set, data, size);
> > + rte_memcpy(sess->sess_data + sess->nb_drivers, data, size);
> > return 0;
> > }
> >
> > @@ -1434,14 +1464,10 @@ void * __rte_experimental
> > rte_cryptodev_sym_session_get_user_data(
> > struct rte_cryptodev_sym_session *sess)
> > {
> > - uint16_t off_set = sizeof(void *) * nb_drivers;
> > - uint8_t *user_data_present = (uint8_t *)sess + off_set;
> > -
> > - if (sess == NULL || !*user_data_present)
> > + if (sess == NULL || sess->priv_size == 0)
> > return NULL;
> >
> > - off_set += sizeof(uint8_t);
> > - return (uint8_t *)sess + off_set;
> > + return (sess->sess_data + sess->nb_drivers);
> > }
> >
> > /** Initialise rte_crypto_op mempool element */
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> > index 4099823f1..d88454f02 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.h
> > +++ b/lib/librte_cryptodev/rte_cryptodev.h
> > @@ -495,6 +495,14 @@ enum rte_cryptodev_event_type {
> > /** Crypto device queue pair configuration structure. */
> > struct rte_cryptodev_qp_conf {
> > uint32_t nb_descriptors; /**< Number of descriptors per queue pair */
> > + struct rte_mempool *sess_pool;
> > + /**< Pointer to crypto sessions mempool,
> > + * used for session-less operations.
> > + */
> > + struct rte_mempool *priv_sess_pool;
> > + /**< Pointer to device specific sessions mempool,
> > + * used for session-less operations.
> > + */
> > };
> >
> > /**
> > @@ -680,17 +688,13 @@ rte_cryptodev_close(uint8_t dev_id);
> > * *SOCKET_ID_ANY* if there is no NUMA constraint
> > * for the DMA memory allocated for the receive
> > * queue pair.
> > - * @param session_pool Pointer to device session mempool, used
> > - * for session-less operations.
> > - *
> > * @return
> > * - 0: Success, queue pair correctly set up.
> > * - <0: Queue pair configuration failed
> > */
> > extern int
> > rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
> > - const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
> > - struct rte_mempool *session_pool);
> > + const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
> >
> > /**
> > * Get the number of queue pairs on a specific crypto device
> > @@ -954,10 +958,43 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
> > * has a fixed algo, key, op-type, digest_len etc.
> > */
> > struct rte_cryptodev_sym_session {
> > - __extension__ void *sess_private_data[0];
> > - /**< Private symmetric session material */
> > + uint64_t userdata;
> > + /**< Can be used for external metadata */
> > + uint16_t nb_drivers;
> > + /**< number of elements in sess_data array */
> > + uint16_t priv_size;
> > + /**< session private data will be placed after sess_data */
> > + __extension__ struct {
> > + void *data;
> > + uint16_t refcnt;
> > + } sess_data[0];
> > + /**< Driver specific session material, variable size */
> > };
> >
> > +static inline size_t
> > +rte_cryptodev_sym_session_data_size(const struct rte_cryptodev_sym_session *s)
> > +{
> > + return (sizeof(s->sess_data[0]) * s->nb_drivers);
> > +}
> > +
> > +static inline size_t
> > +rte_cryptodev_sym_session_size(const struct rte_cryptodev_sym_session *s)
> > +{
> > + return (sizeof(*s) + (s)->priv_size +
> > + rte_cryptodev_sym_session_data_size(s));
> > +}
> > +
> [Fiona] Are above 2 fns used?
> Look like duplicates of the "max" fns?
Not really: rte_cryptodev_sym_session_max_data_size() and
rte_cryptodev_sym_session_max_size() use global var nb_drivers.
Returns amount of space required to create a session that
can work with all attached at that moment drivers.
Planned to be used by in rte_cryptodev_sym_session_max_size().
While these 2 funcs return size of particular session object.
@@ -15,6 +15,7 @@ struct cperf_op_fns;
typedef void *(*cperf_constructor_t)(
struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
uint16_t qp_id,
const struct cperf_options *options,
@@ -469,6 +469,7 @@ cperf_set_ops_aead(struct rte_crypto_op **ops,
static struct rte_cryptodev_sym_session *
cperf_create_session(struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
@@ -505,7 +506,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
}
/* create crypto session */
rte_cryptodev_sym_session_init(dev_id, sess, &cipher_xform,
- sess_mp);
+ priv_mp);
/*
* auth only
*/
@@ -532,7 +533,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
}
/* create crypto session */
rte_cryptodev_sym_session_init(dev_id, sess, &auth_xform,
- sess_mp);
+ priv_mp);
/*
* cipher and auth
*/
@@ -589,12 +590,12 @@ cperf_create_session(struct rte_mempool *sess_mp,
cipher_xform.next = &auth_xform;
/* create crypto session */
rte_cryptodev_sym_session_init(dev_id,
- sess, &cipher_xform, sess_mp);
+ sess, &cipher_xform, priv_mp);
} else { /* auth then cipher */
auth_xform.next = &cipher_xform;
/* create crypto session */
rte_cryptodev_sym_session_init(dev_id,
- sess, &auth_xform, sess_mp);
+ sess, &auth_xform, priv_mp);
}
} else { /* options->op_type == CPERF_AEAD */
aead_xform.type = RTE_CRYPTO_SYM_XFORM_AEAD;
@@ -615,7 +616,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
/* Create crypto session */
rte_cryptodev_sym_session_init(dev_id,
- sess, &aead_xform, sess_mp);
+ sess, &aead_xform, priv_mp);
}
return sess;
@@ -13,7 +13,7 @@
typedef struct rte_cryptodev_sym_session *(*cperf_sessions_create_t)(
- struct rte_mempool *sess_mp,
+ struct rte_mempool *sess_mp, struct rte_mempool *priv_mp,
uint8_t dev_id, const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
uint16_t iv_offset);
@@ -62,6 +62,7 @@ cperf_latency_test_free(struct cperf_latency_ctx *ctx)
void *
cperf_latency_test_constructor(struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id, uint16_t qp_id,
const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
@@ -86,8 +87,8 @@ cperf_latency_test_constructor(struct rte_mempool *sess_mp,
sizeof(struct rte_crypto_sym_op) +
sizeof(struct cperf_op_result *);
- ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
- iv_offset);
+ ctx->sess = op_fns->sess_create(sess_mp, priv_mp, dev_id, options,
+ test_vector, iv_offset);
if (ctx->sess == NULL)
goto err;
@@ -17,6 +17,7 @@
void *
cperf_latency_test_constructor(
struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
uint16_t qp_id,
const struct cperf_options *options,
@@ -80,6 +80,7 @@ cperf_pmd_cyclecount_test_free(struct cperf_pmd_cyclecount_ctx *ctx)
void *
cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id, uint16_t qp_id,
const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
@@ -106,8 +107,8 @@ cperf_pmd_cyclecount_test_constructor(struct rte_mempool *sess_mp,
uint16_t iv_offset = sizeof(struct rte_crypto_op) +
sizeof(struct rte_crypto_sym_op);
- ctx->sess = op_fns->sess_create(
- sess_mp, dev_id, options, test_vector, iv_offset);
+ ctx->sess = op_fns->sess_create(sess_mp, priv_mp, dev_id, options,
+ test_vector, iv_offset);
if (ctx->sess == NULL)
goto err;
@@ -18,6 +18,7 @@
void *
cperf_pmd_cyclecount_test_constructor(
struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
uint16_t qp_id,
const struct cperf_options *options,
@@ -47,6 +47,7 @@ cperf_throughput_test_free(struct cperf_throughput_ctx *ctx)
void *
cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id, uint16_t qp_id,
const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
@@ -69,8 +70,8 @@ cperf_throughput_test_constructor(struct rte_mempool *sess_mp,
uint16_t iv_offset = sizeof(struct rte_crypto_op) +
sizeof(struct rte_crypto_sym_op);
- ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
- iv_offset);
+ ctx->sess = op_fns->sess_create(sess_mp, priv_mp, dev_id, options,
+ test_vector, iv_offset);
if (ctx->sess == NULL)
goto err;
@@ -18,6 +18,7 @@
void *
cperf_throughput_test_constructor(
struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
uint16_t qp_id,
const struct cperf_options *options,
@@ -51,6 +51,7 @@ cperf_verify_test_free(struct cperf_verify_ctx *ctx)
void *
cperf_verify_test_constructor(struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id, uint16_t qp_id,
const struct cperf_options *options,
const struct cperf_test_vector *test_vector,
@@ -73,8 +74,8 @@ cperf_verify_test_constructor(struct rte_mempool *sess_mp,
uint16_t iv_offset = sizeof(struct rte_crypto_op) +
sizeof(struct rte_crypto_sym_op);
- ctx->sess = op_fns->sess_create(sess_mp, dev_id, options, test_vector,
- iv_offset);
+ ctx->sess = op_fns->sess_create(sess_mp, priv_mp, dev_id, options,
+ test_vector, iv_offset);
if (ctx->sess == NULL)
goto err;
@@ -18,6 +18,7 @@
void *
cperf_verify_test_constructor(
struct rte_mempool *sess_mp,
+ struct rte_mempool *priv_mp,
uint8_t dev_id,
uint16_t qp_id,
const struct cperf_options *options,
@@ -22,6 +22,11 @@
#include "cperf_test_pmd_cyclecount.h"
+static struct {
+ struct rte_mempool *sess_mp;
+ struct rte_mempool *priv_mp;
+} session_pool_socket[RTE_MAX_NUMA_NODES];
+
const char *cperf_test_type_strs[] = {
[CPERF_TEST_TYPE_THROUGHPUT] = "throughput",
[CPERF_TEST_TYPE_LATENCY] = "latency",
@@ -61,8 +66,59 @@ const struct cperf_test cperf_testmap[] = {
};
static int
-cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs,
- struct rte_mempool *session_pool_socket[])
+fill_session_pool_socket(int32_t socket_id, uint32_t max_sess_size,
+ uint32_t nb_sessions)
+{
+ char mp_name[RTE_MEMPOOL_NAMESIZE];
+ struct rte_mempool *sess_mp;
+
+ if (session_pool_socket[socket_id].priv_mp == NULL) {
+
+ snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+ "priv_sess_mp_%u", socket_id);
+
+ sess_mp = rte_mempool_create(mp_name,
+ nb_sessions,
+ max_sess_size,
+ 0, 0, NULL, NULL, NULL,
+ NULL, socket_id,
+ 0);
+
+ if (sess_mp == NULL) {
+ printf("Cannot create pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ return -ENOMEM;
+ }
+
+ printf("Allocated pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ session_pool_socket[socket_id].priv_mp = sess_mp;
+ }
+
+ if (session_pool_socket[socket_id].sess_mp == NULL) {
+
+ snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+ "sess_mp_%u", socket_id);
+
+ sess_mp = rte_cryptodev_sym_session_pool_create(mp_name,
+ nb_sessions, 0, 0, socket_id);
+
+ if (sess_mp == NULL) {
+ printf("Cannot create pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ return -ENOMEM;
+ }
+
+ printf("Allocated pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ session_pool_socket[socket_id].sess_mp = sess_mp;
+ }
+
+ return 0;
+}
+
+static int
+cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs)
{
uint8_t enabled_cdev_count = 0, nb_lcores, cdev_id;
uint32_t sessions_needed = 0;
@@ -96,7 +152,7 @@ cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs,
uint32_t max_sess_size = 0, sess_size;
for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
- sess_size = rte_cryptodev_sym_get_private_session_size(cdev_id);
+ sess_size = rte_cryptodev_sym_private_session_size(cdev_id);
if (sess_size > max_sess_size)
max_sess_size = sess_size;
}
@@ -144,10 +200,6 @@ cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs,
.socket_id = socket_id
};
- struct rte_cryptodev_qp_conf qp_conf = {
- .nb_descriptors = opts->nb_descriptors
- };
-
/**
* Device info specifies the min headroom and tailroom
* requirement for the crypto PMD. This need to be honoured
@@ -194,29 +246,19 @@ cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs,
"%u sessions\n", opts->nb_qps);
return -ENOTSUP;
}
- if (session_pool_socket[socket_id] == NULL) {
- char mp_name[RTE_MEMPOOL_NAMESIZE];
- struct rte_mempool *sess_mp;
-
- snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
- "sess_mp_%u", socket_id);
- sess_mp = rte_mempool_create(mp_name,
- sessions_needed,
- max_sess_size,
- 0,
- 0, NULL, NULL, NULL,
- NULL, socket_id,
- 0);
-
- if (sess_mp == NULL) {
- printf("Cannot create session pool on socket %d\n",
- socket_id);
- return -ENOMEM;
- }
- printf("Allocated session pool on socket %d\n", socket_id);
- session_pool_socket[socket_id] = sess_mp;
- }
+ ret = fill_session_pool_socket(socket_id, max_sess_size,
+ sessions_needed);
+ if (ret < 0)
+ return ret;
+
+ struct rte_cryptodev_qp_conf qp_conf = {
+ .nb_descriptors = opts->nb_descriptors,
+ .sess_pool = session_pool_socket[socket_id].sess_mp,
+ .priv_sess_pool =
+ session_pool_socket[socket_id].priv_mp,
+ };
+
ret = rte_cryptodev_configure(cdev_id, &conf);
if (ret < 0) {
@@ -226,8 +268,7 @@ cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs,
for (j = 0; j < opts->nb_qps; j++) {
ret = rte_cryptodev_queue_pair_setup(cdev_id, j,
- &qp_conf, socket_id,
- session_pool_socket[socket_id]);
+ &qp_conf, socket_id);
if (ret < 0) {
printf("Failed to setup queue pair %u on "
"cryptodev %u", j, cdev_id);
@@ -445,7 +486,6 @@ main(int argc, char **argv)
struct cperf_op_fns op_fns;
void *ctx[RTE_MAX_LCORE] = { };
- struct rte_mempool *session_pool_socket[RTE_MAX_NUMA_NODES] = { 0 };
int nb_cryptodevs = 0;
uint16_t total_nb_qps = 0;
@@ -479,8 +519,7 @@ main(int argc, char **argv)
goto err;
}
- nb_cryptodevs = cperf_initialize_cryptodev(&opts, enabled_cdevs,
- session_pool_socket);
+ nb_cryptodevs = cperf_initialize_cryptodev(&opts, enabled_cdevs);
if (!opts.silent)
cperf_options_dump(&opts);
@@ -548,7 +587,9 @@ main(int argc, char **argv)
uint8_t socket_id = rte_cryptodev_socket_id(cdev_id);
ctx[i] = cperf_testmap[opts.test].constructor(
- session_pool_socket[socket_id], cdev_id, qp_id,
+ session_pool_socket[socket_id].sess_mp,
+ session_pool_socket[socket_id].priv_mp,
+ cdev_id, qp_id,
&opts, t_vec, &op_fns);
if (ctx[i] == NULL) {
RTE_LOG(ERR, USER1, "Test run constructor failed\n");
@@ -137,7 +137,7 @@ aesni_gcm_get_session(struct aesni_gcm_qp *qp, struct rte_crypto_op *op)
if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
return NULL;
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess_private_data))
+ if (rte_mempool_get(qp->priv_mp, (void **)&_sess_private_data))
return NULL;
sess = (struct aesni_gcm_session *)_sess_private_data;
@@ -145,7 +145,7 @@ aesni_gcm_get_session(struct aesni_gcm_qp *qp, struct rte_crypto_op *op)
if (unlikely(aesni_gcm_set_session_parameters(qp->ops,
sess, sym_op->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp, _sess_private_data);
+ rte_mempool_put(qp->priv_mp, _sess_private_data);
sess = NULL;
}
sym_op->session = (struct rte_cryptodev_sym_session *)_sess;
@@ -391,9 +391,9 @@ handle_completed_gcm_crypto_op(struct aesni_gcm_qp *qp,
/* Free session if a session-less crypto op */
if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
memset(sess, 0, sizeof(struct aesni_gcm_session));
- memset(op->sym->session, 0,
- rte_cryptodev_sym_get_header_session_size());
- rte_mempool_put(qp->sess_mp, sess);
+ set_sym_session_private_data(op->sym->session,
+ cryptodev_driver_id, NULL);
+ rte_mempool_put(qp->priv_mp, sess);
rte_mempool_put(qp->sess_mp, op->sym->session);
op->sym->session = NULL;
}
@@ -201,7 +201,7 @@ aesni_gcm_pmd_qp_create_processed_pkts_ring(struct aesni_gcm_qp *qp,
static int
aesni_gcm_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id, struct rte_mempool *session_pool)
+ int socket_id)
{
struct aesni_gcm_qp *qp = NULL;
struct aesni_gcm_private *internals = dev->data->dev_private;
@@ -229,7 +229,8 @@ aesni_gcm_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
if (qp->processed_pkts == NULL)
goto qp_setup_cleanup;
- qp->sess_mp = session_pool;
+ qp->sess_mp = qp_conf->sess_pool;
+ qp->priv_mp = qp_conf->priv_sess_pool;
memset(&qp->qp_stats, 0, sizeof(qp->qp_stats));
@@ -47,7 +47,9 @@ struct aesni_gcm_qp {
struct rte_cryptodev_stats qp_stats; /* 8 * 4 = 32 B */
/**< Queue pair statistics */
struct rte_mempool *sess_mp;
- /**< Session Mempool */
+ /**< crypto session Mempool */
+ struct rte_mempool *priv_mp;
+ /**< private session Mempool */
uint16_t id;
/**< Queue Pair Identifier */
char name[RTE_CRYPTODEV_NAME_MAX_LEN];
@@ -516,7 +516,7 @@ get_session(struct aesni_mb_qp *qp, struct rte_crypto_op *op)
if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
return NULL;
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess_private_data))
+ if (rte_mempool_get(qp->priv_mp, (void **)&_sess_private_data))
return NULL;
sess = (struct aesni_mb_session *)_sess_private_data;
@@ -524,7 +524,7 @@ get_session(struct aesni_mb_qp *qp, struct rte_crypto_op *op)
if (unlikely(aesni_mb_set_session_parameters(qp->op_fns,
sess, op->sym->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp, _sess_private_data);
+ rte_mempool_put(qp->priv_mp, _sess_private_data);
sess = NULL;
}
op->sym->session = (struct rte_cryptodev_sym_session *)_sess;
@@ -741,9 +741,9 @@ post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job)
/* Free session if a session-less crypto op */
if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
memset(sess, 0, sizeof(struct aesni_mb_session));
- memset(op->sym->session, 0,
- rte_cryptodev_sym_get_header_session_size());
- rte_mempool_put(qp->sess_mp, sess);
+ set_sym_session_private_data(op->sym->session,
+ cryptodev_driver_id, NULL);
+ rte_mempool_put(qp->priv_mp, sess);
rte_mempool_put(qp->sess_mp, op->sym->session);
op->sym->session = NULL;
}
@@ -482,7 +482,7 @@ aesni_mb_pmd_qp_create_processed_ops_ring(struct aesni_mb_qp *qp,
static int
aesni_mb_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id, struct rte_mempool *session_pool)
+ int socket_id)
{
struct aesni_mb_qp *qp = NULL;
struct aesni_mb_private *internals = dev->data->dev_private;
@@ -520,7 +520,8 @@ aesni_mb_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
goto qp_setup_cleanup;
}
- qp->sess_mp = session_pool;
+ qp->sess_mp = qp_conf->sess_pool;
+ qp->priv_mp = qp_conf->priv_sess_pool;
memset(&qp->stats, 0, sizeof(qp->stats));
@@ -137,7 +137,9 @@ struct aesni_mb_qp {
struct rte_ring *ingress_queue;
/**< Ring for placing operations ready for processing */
struct rte_mempool *sess_mp;
- /**< Session Mempool */
+ /**< crypto session Mempool */
+ struct rte_mempool *priv_mp;
+ /**< private Session Mempool */
struct rte_cryptodev_stats stats;
/**< Queue pair statistics */
uint8_t digest_idx;
@@ -1416,8 +1416,7 @@ dpaa2_sec_queue_pair_release(struct rte_cryptodev *dev, uint16_t queue_pair_id)
static int
dpaa2_sec_queue_pair_setup(struct rte_cryptodev *dev, uint16_t qp_id,
__rte_unused const struct rte_cryptodev_qp_conf *qp_conf,
- __rte_unused int socket_id,
- __rte_unused struct rte_mempool *session_pool)
+ __rte_unused int socket_id)
{
struct dpaa2_sec_dev_private *priv = dev->data->dev_private;
struct dpaa2_sec_qp *qp;
@@ -1576,8 +1576,7 @@ dpaa_sec_queue_pair_release(struct rte_cryptodev *dev,
static int
dpaa_sec_queue_pair_setup(struct rte_cryptodev *dev, uint16_t qp_id,
__rte_unused const struct rte_cryptodev_qp_conf *qp_conf,
- __rte_unused int socket_id,
- __rte_unused struct rte_mempool *session_pool)
+ __rte_unused int socket_id)
{
struct dpaa_sec_dev_private *internals;
struct dpaa_sec_qp *qp = NULL;
@@ -49,16 +49,18 @@ null_crypto_set_session_parameters(
/** Process crypto operation for mbuf */
static int
process_op(const struct null_crypto_qp *qp, struct rte_crypto_op *op,
- struct null_crypto_session *sess __rte_unused)
+ struct null_crypto_session *sess)
{
/* set status as successful by default */
op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
/* Free session if a session-less crypto op. */
if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
- memset(op->sym->session, 0,
- sizeof(struct null_crypto_session));
- rte_cryptodev_sym_session_free(op->sym->session);
+ memset(sess, 0, sizeof(*sess));
+ set_sym_session_private_data(op->sym->session,
+ cryptodev_driver_id, NULL);
+ rte_mempool_put(qp->priv_mp, sess);
+ rte_mempool_put(qp->sess_mp, op->sym->session);
op->sym->session = NULL;
}
@@ -87,7 +89,7 @@ get_session(struct null_crypto_qp *qp, struct rte_crypto_op *op)
if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
return NULL;
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess_private_data))
+ if (rte_mempool_get(qp->priv_mp, (void **)&_sess_private_data))
return NULL;
sess = (struct null_crypto_session *)_sess_private_data;
@@ -95,7 +97,7 @@ get_session(struct null_crypto_qp *qp, struct rte_crypto_op *op)
if (unlikely(null_crypto_set_session_parameters(sess,
sym_op->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp, _sess_private_data);
+ rte_mempool_put(qp->priv_mp, _sess_private_data);
sess = NULL;
}
sym_op->session = (struct rte_cryptodev_sym_session *)_sess;
@@ -184,7 +184,7 @@ null_crypto_pmd_qp_create_processed_pkts_ring(struct null_crypto_qp *qp,
static int
null_crypto_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id, struct rte_mempool *session_pool)
+ int socket_id)
{
struct null_crypto_private *internals = dev->data->dev_private;
struct null_crypto_qp *qp;
@@ -228,7 +228,8 @@ null_crypto_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
goto qp_setup_cleanup;
}
- qp->sess_mp = session_pool;
+ qp->sess_mp = qp_conf->sess_pool;
+ qp->priv_mp = qp_conf->priv_sess_pool;
memset(&qp->qp_stats, 0, sizeof(qp->qp_stats));
@@ -30,7 +30,9 @@ struct null_crypto_qp {
struct rte_ring *processed_pkts;
/**< Ring for placing process packets */
struct rte_mempool *sess_mp;
- /**< Session Mempool */
+ /**< crypto session Mempool */
+ struct rte_mempool *priv_mp;
+ /**< private session Mempool */
struct rte_cryptodev_stats qp_stats;
/**< Queue pair statistics */
} __rte_cache_aligned;
@@ -390,8 +390,7 @@ scheduler_pmd_qp_release(struct rte_cryptodev *dev, uint16_t qp_id)
/** Setup a queue pair */
static int
scheduler_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
- const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
- struct rte_mempool *session_pool)
+ const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
{
struct scheduler_ctx *sched_ctx = dev->data->dev_private;
struct scheduler_qp_ctx *qp_ctx;
@@ -419,7 +418,7 @@ scheduler_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
* must be big enough for all the drivers used.
*/
ret = rte_cryptodev_queue_pair_setup(slave_id, qp_id,
- qp_conf, socket_id, session_pool);
+ qp_conf, socket_id);
if (ret < 0)
return ret;
}
@@ -36,8 +36,7 @@ static void virtio_crypto_dev_stats_reset(struct rte_cryptodev *dev);
static int virtio_crypto_qp_setup(struct rte_cryptodev *dev,
uint16_t queue_pair_id,
const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id,
- struct rte_mempool *session_pool);
+ int socket_id);
static int virtio_crypto_qp_release(struct rte_cryptodev *dev,
uint16_t queue_pair_id);
static void virtio_crypto_dev_free_mbufs(struct rte_cryptodev *dev);
@@ -585,8 +584,7 @@ virtio_crypto_dev_stats_reset(struct rte_cryptodev *dev)
static int
virtio_crypto_qp_setup(struct rte_cryptodev *dev, uint16_t queue_pair_id,
const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id,
- struct rte_mempool *session_pool __rte_unused)
+ int socket_id)
{
int ret;
struct virtqueue *vq;
@@ -821,11 +821,15 @@ main_loop(__attribute__((unused)) void *dummy)
qconf->inbound.sa_ctx = socket_ctx[socket_id].sa_in;
qconf->inbound.cdev_map = cdev_map_in;
qconf->inbound.session_pool = socket_ctx[socket_id].session_pool;
+ qconf->inbound.priv_session_pool =
+ socket_ctx[socket_id].priv_session_pool;
qconf->outbound.sp4_ctx = socket_ctx[socket_id].sp_ip4_out;
qconf->outbound.sp6_ctx = socket_ctx[socket_id].sp_ip6_out;
qconf->outbound.sa_ctx = socket_ctx[socket_id].sa_out;
qconf->outbound.cdev_map = cdev_map_out;
qconf->outbound.session_pool = socket_ctx[socket_id].session_pool;
+ qconf->outbound.priv_session_pool =
+ socket_ctx[socket_id].priv_session_pool;
if (qconf->nb_rx_queue == 0) {
RTE_LOG(INFO, IPSEC, "lcore %u has nothing to do\n", lcore_id);
@@ -972,20 +976,19 @@ print_usage(const char *prgname)
}
static int32_t
-parse_portmask(const char *portmask)
+parse_portmask(const char *portmask, uint32_t *pmv)
{
- char *end = NULL;
+ char *end;
unsigned long pm;
/* parse hexadecimal string */
+ errno = 0;
pm = strtoul(portmask, &end, 16);
- if ((portmask[0] == '\0') || (end == NULL) || (*end != '\0'))
+ if (errno != 0 || *end != '\0' || pm > UINT32_MAX)
return -1;
- if ((pm == 0) && errno)
- return -1;
-
- return pm;
+ *pmv = pm;
+ return 0;
}
static int32_t
@@ -1063,6 +1066,7 @@ parse_args(int32_t argc, char **argv)
int32_t opt, ret;
char **argvopt;
int32_t option_index;
+ uint32_t v;
char *prgname = argv[0];
int32_t f_present = 0;
@@ -1073,8 +1077,8 @@ parse_args(int32_t argc, char **argv)
switch (opt) {
case 'p':
- enabled_port_mask = parse_portmask(optarg);
- if (enabled_port_mask == 0) {
+ ret = parse_portmask(optarg, &enabled_port_mask);
+ if (ret < 0 || enabled_port_mask == 0) {
printf("invalid portmask\n");
print_usage(prgname);
return -1;
@@ -1085,8 +1089,8 @@ parse_args(int32_t argc, char **argv)
promiscuous_on = 1;
break;
case 'u':
- unprotected_port_mask = parse_portmask(optarg);
- if (unprotected_port_mask == 0) {
+ ret = parse_portmask(optarg, &unprotected_port_mask);
+ if (ret < 0) {
printf("invalid unprotected portmask\n");
print_usage(prgname);
return -1;
@@ -1147,15 +1151,16 @@ parse_args(int32_t argc, char **argv)
single_sa_idx);
break;
case CMD_LINE_OPT_CRYPTODEV_MASK_NUM:
- ret = parse_portmask(optarg);
+ ret = parse_portmask(optarg, &v);
if (ret == -1) {
- printf("Invalid argument[portmask]\n");
+ printf("Invalid argument[%s]\n",
+ CMD_LINE_OPT_CRYPTODEV_MASK);
print_usage(prgname);
return -1;
}
/* else */
- enabled_cryptodev_mask = ret;
+ enabled_cryptodev_mask = v;
break;
default:
print_usage(prgname);
@@ -1360,6 +1365,54 @@ check_cryptodev_mask(uint8_t cdev_id)
return -1;
}
+static int
+fill_session_pool(int32_t socket_id, uint32_t max_sess_size)
+{
+ char mp_name[RTE_MEMPOOL_NAMESIZE];
+ struct rte_mempool *sess_mp;
+
+ if (!socket_ctx[socket_id].priv_session_pool) {
+
+ snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+ "priv_sess_mp_%u", socket_id);
+ sess_mp = rte_mempool_create(mp_name,
+ CDEV_MP_NB_OBJS,
+ max_sess_size,
+ CDEV_MP_CACHE_SZ,
+ 0, NULL, NULL, NULL,
+ NULL, socket_id,
+ 0);
+ if (sess_mp == NULL) {
+ printf("Cannot create pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ return -ENOMEM;
+ }
+
+ printf("Allocated pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ socket_ctx[socket_id].priv_session_pool = sess_mp;
+ }
+
+ if (!socket_ctx[socket_id].session_pool) {
+
+ snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+ "sess_mp_%u", socket_id);
+
+ sess_mp = rte_cryptodev_sym_session_pool_create(mp_name,
+ CDEV_MP_NB_OBJS, CDEV_MP_CACHE_SZ, 0, socket_id);
+ if (sess_mp == NULL) {
+ printf("Cannot create pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ return -ENOMEM;
+ }
+
+ printf("Allocated pool \"%s\" on socket %d\n",
+ mp_name, socket_id);
+ socket_ctx[socket_id].session_pool = sess_mp;
+ }
+
+ return 0;
+}
static int32_t
cryptodevs_init(void)
@@ -1392,7 +1445,7 @@ cryptodevs_init(void)
uint32_t max_sess_sz = 0, sess_sz;
for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
- sess_sz = rte_cryptodev_sym_get_private_session_size(cdev_id);
+ sess_sz = rte_cryptodev_sym_private_session_size(cdev_id);
if (sess_sz > max_sess_sz)
max_sess_sz = sess_sz;
}
@@ -1448,38 +1501,23 @@ cryptodevs_init(void)
"Device does not support at least %u "
"sessions", CDEV_MP_NB_OBJS / 2);
- if (!socket_ctx[dev_conf.socket_id].session_pool) {
- char mp_name[RTE_MEMPOOL_NAMESIZE];
- struct rte_mempool *sess_mp;
-
- snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
- "sess_mp_%u", dev_conf.socket_id);
- sess_mp = rte_mempool_create(mp_name,
- CDEV_MP_NB_OBJS,
- max_sess_sz,
- CDEV_MP_CACHE_SZ,
- 0, NULL, NULL, NULL,
- NULL, dev_conf.socket_id,
- 0);
- if (sess_mp == NULL)
- rte_exit(EXIT_FAILURE,
- "Cannot create session pool on socket %d\n",
- dev_conf.socket_id);
- else
- printf("Allocated session pool on socket %d\n",
- dev_conf.socket_id);
- socket_ctx[dev_conf.socket_id].session_pool = sess_mp;
- }
+ if (fill_session_pool(dev_conf.socket_id, max_sess_sz) != 0)
+ rte_exit(EXIT_FAILURE,
+ "Cannot create session pools on socket %d\n",
+ dev_conf.socket_id);
if (rte_cryptodev_configure(cdev_id, &dev_conf))
rte_panic("Failed to initialize cryptodev %u\n",
cdev_id);
qp_conf.nb_descriptors = CDEV_QUEUE_DESC;
+ qp_conf.sess_pool = socket_ctx[dev_conf.socket_id].session_pool;
+ qp_conf.priv_sess_pool =
+ socket_ctx[dev_conf.socket_id].priv_session_pool;
+
for (qp = 0; qp < dev_conf.nb_queue_pairs; qp++)
if (rte_cryptodev_queue_pair_setup(cdev_id, qp,
- &qp_conf, dev_conf.socket_id,
- socket_ctx[dev_conf.socket_id].session_pool))
+ &qp_conf, dev_conf.socket_id))
rte_panic("Failed to setup queue %u for "
"cdev_id %u\n", 0, cdev_id);
@@ -144,6 +144,7 @@ struct ipsec_ctx {
uint16_t last_qp;
struct cdev_qp tbl[MAX_QP_PER_LCORE];
struct rte_mempool *session_pool;
+ struct rte_mempool *priv_session_pool;
struct rte_mbuf *ol_pkts[MAX_PKT_BURST] __rte_aligned(sizeof(void *));
uint16_t ol_pkts_cnt;
};
@@ -166,6 +167,7 @@ struct socket_ctx {
struct rt_ctx *rt_ip6;
struct rte_mempool *mbuf_pool;
struct rte_mempool *session_pool;
+ struct rte_mempool *priv_session_pool;
};
struct cnt_blk {
@@ -943,8 +943,7 @@ rte_cryptodev_close(uint8_t dev_id)
int
rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
- const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
- struct rte_mempool *session_pool)
+ const struct rte_cryptodev_qp_conf *qp_conf, int socket_id)
{
struct rte_cryptodev *dev;
@@ -954,6 +953,12 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
return -EINVAL;
}
+ if (qp_conf == NULL || qp_conf->sess_pool == NULL ||
+ qp_conf->priv_sess_pool == NULL) {
+ CDEV_LOG_ERR("Invalid queue_pair config");
+ return -EINVAL;
+ }
+
dev = &rte_crypto_devices[dev_id];
if (queue_pair_id >= dev->data->nb_queue_pairs) {
CDEV_LOG_ERR("Invalid queue_pair_id=%d", queue_pair_id);
@@ -969,7 +974,7 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_pair_setup, -ENOTSUP);
return (*dev->dev_ops->queue_pair_setup)(dev, queue_pair_id, qp_conf,
- socket_id, session_pool);
+ socket_id);
}
@@ -1146,6 +1151,41 @@ rte_cryptodev_pmd_callback_process(struct rte_cryptodev *dev,
rte_spinlock_unlock(&rte_cryptodev_cb_lock);
}
+static void
+cryptodev_sym_session_init_elem(__rte_unused struct rte_mempool *pool,
+ void *arg, void *obj, __rte_unused uint32_t idx)
+{
+ struct rte_cryptodev_sym_session *ds;
+ const struct rte_cryptodev_sym_session *ss;
+
+ ds = obj;
+ ss = arg;
+
+ *ds = *ss;
+ memset(ds->sess_data, 0, rte_cryptodev_sym_session_data_size(ds));
+}
+
+struct rte_mempool *
+rte_cryptodev_sym_session_pool_create(const char *name,
+ uint32_t nb_elts, uint32_t cache_size, uint16_t priv_size,
+ int socket_id)
+{
+ struct rte_mempool *mp;
+ uint32_t elt_size;
+ struct rte_cryptodev_sym_session s = {
+ .nb_drivers = nb_drivers,
+ .priv_size = priv_size,
+ };
+
+ elt_size = rte_cryptodev_sym_session_max_size(priv_size);
+ mp = rte_mempool_create(name, nb_elts, elt_size, cache_size, 0,
+ NULL, NULL, cryptodev_sym_session_init_elem, &s,
+ socket_id, 0);
+ if (mp == NULL)
+ CDEV_LOG_ERR("%s(name=%s) failed, rte_errno=%d\n",
+ __func__, name, rte_errno);
+ return mp;
+}
int
rte_cryptodev_sym_session_init(uint8_t dev_id,
@@ -1163,12 +1203,15 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
return -EINVAL;
index = dev->driver_id;
+ if (index > sess->nb_drivers)
+ return -EINVAL;
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -ENOTSUP);
- if (sess->sess_private_data[index] == NULL) {
+ if (sess->sess_data[index].refcnt == 0) {
ret = dev->dev_ops->sym_session_configure(dev, xforms,
- sess, mp);
+ sess, mp);
+
if (ret < 0) {
CDEV_LOG_ERR(
"dev_id %d failed to configure session details",
@@ -1177,6 +1220,7 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
}
}
+ sess->sess_data[index].refcnt++;
return 0;
}
@@ -1229,8 +1273,7 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
/* Clear device session pointer.
* Include the flag indicating presence of user data
*/
- memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
-
+ memset(sess->sess_data, 0, rte_cryptodev_sym_session_data_size(sess));
return sess;
}
@@ -1258,16 +1301,20 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id,
struct rte_cryptodev_sym_session *sess)
{
struct rte_cryptodev *dev;
+ uint32_t idx;
dev = rte_cryptodev_pmd_get_dev(dev_id);
- if (dev == NULL || sess == NULL)
+ if (dev == NULL || sess == NULL || dev->driver_id > sess->nb_drivers)
return -EINVAL;
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_clear, -ENOTSUP);
- dev->dev_ops->sym_session_clear(dev, sess);
+ idx = dev->driver_id;
+ if (--sess->sess_data[idx].refcnt != 0)
+ return -EBUSY;
+ dev->dev_ops->sym_session_clear(dev, sess);
return 0;
}
@@ -1285,7 +1332,6 @@ rte_cryptodev_asym_session_clear(uint8_t dev_id,
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->asym_session_clear, -ENOTSUP);
dev->dev_ops->asym_session_clear(dev, sess);
-
return 0;
}
@@ -1293,7 +1339,6 @@ int
rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
{
uint8_t i;
- void *sess_priv;
struct rte_mempool *sess_mp;
if (sess == NULL)
@@ -1301,8 +1346,7 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
/* Check that all device private data has been freed */
for (i = 0; i < nb_drivers; i++) {
- sess_priv = get_sym_session_private_data(sess, i);
- if (sess_priv != NULL)
+ if (sess->sess_data[i].refcnt != 0)
return -EBUSY;
}
@@ -1313,6 +1357,23 @@ rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
return 0;
}
+unsigned int
+rte_cryptodev_sym_session_max_data_size(void)
+{
+ struct rte_cryptodev_sym_session *sess = NULL;
+
+ return (sizeof(sess->sess_data[0]) * nb_drivers);
+}
+
+size_t
+rte_cryptodev_sym_session_max_size(uint16_t priv_size)
+{
+ struct rte_cryptodev_sym_session *sess = NULL;
+
+ return (sizeof(*sess) + priv_size +
+ rte_cryptodev_sym_session_max_data_size());
+}
+
int __rte_experimental
rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
{
@@ -1337,18 +1398,6 @@ rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
return 0;
}
-
-unsigned int
-rte_cryptodev_sym_get_header_session_size(void)
-{
- /*
- * Header contains pointers to the private data
- * of all registered drivers, and a flag which
- * indicates presence of user data
- */
- return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
-}
-
unsigned int __rte_experimental
rte_cryptodev_asym_get_header_session_size(void)
{
@@ -1361,11 +1410,9 @@ rte_cryptodev_asym_get_header_session_size(void)
}
unsigned int
-rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
+rte_cryptodev_sym_private_session_size(uint8_t dev_id)
{
struct rte_cryptodev *dev;
- unsigned int header_size = sizeof(void *) * nb_drivers;
- unsigned int priv_sess_size;
if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
return 0;
@@ -1375,18 +1422,7 @@ rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
if (*dev->dev_ops->sym_session_get_size == NULL)
return 0;
- priv_sess_size = (*dev->dev_ops->sym_session_get_size)(dev);
-
- /*
- * If size is less than session header size,
- * return the latter, as this guarantees that
- * sessionless operations will work
- */
- if (priv_sess_size < header_size)
- return header_size;
-
- return priv_sess_size;
-
+ return (*dev->dev_ops->sym_session_get_size)(dev);
}
unsigned int __rte_experimental
@@ -1409,7 +1445,6 @@ rte_cryptodev_asym_get_private_session_size(uint8_t dev_id)
return header_size;
return priv_sess_size;
-
}
int __rte_experimental
@@ -1418,15 +1453,10 @@ rte_cryptodev_sym_session_set_user_data(
void *data,
uint16_t size)
{
- uint16_t off_set = sizeof(void *) * nb_drivers;
- uint8_t *user_data_present = (uint8_t *)sess + off_set;
-
- if (sess == NULL)
+ if (sess == NULL || sess->priv_size < size)
return -EINVAL;
- *user_data_present = 1;
- off_set += sizeof(uint8_t);
- rte_memcpy((uint8_t *)sess + off_set, data, size);
+ rte_memcpy(sess->sess_data + sess->nb_drivers, data, size);
return 0;
}
@@ -1434,14 +1464,10 @@ void * __rte_experimental
rte_cryptodev_sym_session_get_user_data(
struct rte_cryptodev_sym_session *sess)
{
- uint16_t off_set = sizeof(void *) * nb_drivers;
- uint8_t *user_data_present = (uint8_t *)sess + off_set;
-
- if (sess == NULL || !*user_data_present)
+ if (sess == NULL || sess->priv_size == 0)
return NULL;
- off_set += sizeof(uint8_t);
- return (uint8_t *)sess + off_set;
+ return (sess->sess_data + sess->nb_drivers);
}
/** Initialise rte_crypto_op mempool element */
@@ -495,6 +495,14 @@ enum rte_cryptodev_event_type {
/** Crypto device queue pair configuration structure. */
struct rte_cryptodev_qp_conf {
uint32_t nb_descriptors; /**< Number of descriptors per queue pair */
+ struct rte_mempool *sess_pool;
+ /**< Pointer to crypto sessions mempool,
+ * used for session-less operations.
+ */
+ struct rte_mempool *priv_sess_pool;
+ /**< Pointer to device specific sessions mempool,
+ * used for session-less operations.
+ */
};
/**
@@ -680,17 +688,13 @@ rte_cryptodev_close(uint8_t dev_id);
* *SOCKET_ID_ANY* if there is no NUMA constraint
* for the DMA memory allocated for the receive
* queue pair.
- * @param session_pool Pointer to device session mempool, used
- * for session-less operations.
- *
* @return
* - 0: Success, queue pair correctly set up.
* - <0: Queue pair configuration failed
*/
extern int
rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
- const struct rte_cryptodev_qp_conf *qp_conf, int socket_id,
- struct rte_mempool *session_pool);
+ const struct rte_cryptodev_qp_conf *qp_conf, int socket_id);
/**
* Get the number of queue pairs on a specific crypto device
@@ -954,10 +958,43 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
* has a fixed algo, key, op-type, digest_len etc.
*/
struct rte_cryptodev_sym_session {
- __extension__ void *sess_private_data[0];
- /**< Private symmetric session material */
+ uint64_t userdata;
+ /**< Can be used for external metadata */
+ uint16_t nb_drivers;
+ /**< number of elements in sess_data array */
+ uint16_t priv_size;
+ /**< session private data will be placed after sess_data */
+ __extension__ struct {
+ void *data;
+ uint16_t refcnt;
+ } sess_data[0];
+ /**< Driver specific session material, variable size */
};
+static inline size_t
+rte_cryptodev_sym_session_data_size(const struct rte_cryptodev_sym_session *s)
+{
+ return (sizeof(s->sess_data[0]) * s->nb_drivers);
+}
+
+static inline size_t
+rte_cryptodev_sym_session_size(const struct rte_cryptodev_sym_session *s)
+{
+ return (sizeof(*s) + (s)->priv_size +
+ rte_cryptodev_sym_session_data_size(s));
+}
+
+unsigned int
+rte_cryptodev_sym_session_max_data_size(void);
+
+size_t
+rte_cryptodev_sym_session_max_size(uint16_t priv_size);
+
+struct rte_mempool *
+rte_cryptodev_sym_session_pool_create(const char *name,
+ uint32_t nb_elts, uint32_t cache_size, uint16_t priv_size,
+ int socket_id);
+
/** Cryptodev asymmetric crypto session */
struct rte_cryptodev_asym_session {
__extension__ void *sess_private_data[0];
@@ -1123,7 +1160,7 @@ rte_cryptodev_asym_get_header_session_size(void);
* symmetric session
*/
unsigned int
-rte_cryptodev_sym_get_private_session_size(uint8_t dev_id);
+rte_cryptodev_sym_private_session_size(uint8_t dev_id);
/**
* Get the size of the private data for asymmetric session
@@ -191,13 +191,12 @@ typedef void (*cryptodev_info_get_t)(struct rte_cryptodev *dev,
* @param qp_id Queue Pair Index
* @param qp_conf Queue configuration structure
* @param socket_id Socket Index
- * @param session_pool Pointer to device session mempool
*
* @return Returns 0 on success.
*/
typedef int (*cryptodev_queue_pair_setup_t)(struct rte_cryptodev *dev,
uint16_t qp_id, const struct rte_cryptodev_qp_conf *qp_conf,
- int socket_id, struct rte_mempool *session_pool);
+ int socket_id);
/**
* Release memory resources allocated by given queue pair.
@@ -478,20 +477,25 @@ RTE_INIT(init_ ##driver_id)\
static inline void *
get_sym_session_private_data(const struct rte_cryptodev_sym_session *sess,
- uint8_t driver_id) {
- return sess->sess_private_data[driver_id];
+ uint8_t driver_id)
+{
+ if (driver_id < sess->nb_drivers)
+ return sess->sess_data[driver_id].data;
+ return NULL;
}
static inline void
set_sym_session_private_data(struct rte_cryptodev_sym_session *sess,
uint8_t driver_id, void *private_data)
{
- sess->sess_private_data[driver_id] = private_data;
+ if (driver_id < sess->nb_drivers)
+ sess->sess_data[driver_id].data = private_data;
}
static inline void *
get_asym_session_private_data(const struct rte_cryptodev_asym_session *sess,
- uint8_t driver_id) {
+ uint8_t driver_id)
+{
return sess->sess_private_data[driver_id];
}