security: update session create API
Checks
Commit Message
From: Akhil Goyal <akhil.goyal@nxp.com>
The API ``rte_security_session_create`` takes only single
mempool for session and session private data. So the
application need to create mempool for twice the number of
sessions needed and will also lead to wastage of memory as
session private data need more memory compared to session.
Hence the API is modified to take two mempool pointers
- one for session and one for private data.
This is very similar to crypto based session create APIs.
Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
app/test-crypto-perf/cperf_ops.c | 4 +--
app/test/test_cryptodev.c | 8 +++--
app/test/test_ipsec.c | 3 +-
app/test/test_security.c | 42 ++++++++++++++++++++------
doc/guides/prog_guide/rte_security.rst | 6 ++--
doc/guides/rel_notes/deprecation.rst | 7 -----
doc/guides/rel_notes/release_20_11.rst | 6 ++++
examples/ipsec-secgw/ipsec-secgw.c | 12 ++------
examples/ipsec-secgw/ipsec.c | 9 ++++--
lib/librte_security/rte_security.c | 6 ++--
lib/librte_security/rte_security.h | 4 ++-
11 files changed, 68 insertions(+), 39 deletions(-)
Comments
W dniu 03.09.2020 o 22:09, akhil.goyal@nxp.com pisze:
> From: Akhil Goyal <akhil.goyal@nxp.com>
>
> The API ``rte_security_session_create`` takes only single
> mempool for session and session private data. So the
> application need to create mempool for twice the number of
> sessions needed and will also lead to wastage of memory as
> session private data need more memory compared to session.
> Hence the API is modified to take two mempool pointers
> - one for session and one for private data.
> This is very similar to crypto based session create APIs.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---
> app/test-crypto-perf/cperf_ops.c | 4 +--
> app/test/test_cryptodev.c | 8 +++--
> app/test/test_ipsec.c | 3 +-
> app/test/test_security.c | 42 ++++++++++++++++++++------
> doc/guides/prog_guide/rte_security.rst | 6 ++--
> doc/guides/rel_notes/deprecation.rst | 7 -----
> doc/guides/rel_notes/release_20_11.rst | 6 ++++
> examples/ipsec-secgw/ipsec-secgw.c | 12 ++------
> examples/ipsec-secgw/ipsec.c | 9 ++++--
> lib/librte_security/rte_security.c | 6 ++--
> lib/librte_security/rte_security.h | 4 ++-
> 11 files changed, 68 insertions(+), 39 deletions(-)
>
> diff --git a/app/test-crypto-perf/cperf_ops.c b/app/test-crypto-perf/cperf_ops.c
> index 3da835a9c..3a64a2c34 100644
> --- a/app/test-crypto-perf/cperf_ops.c
> +++ b/app/test-crypto-perf/cperf_ops.c
> @@ -621,7 +621,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
>
> /* Create security session */
> return (void *)rte_security_session_create(ctx,
> - &sess_conf, sess_mp);
> + &sess_conf, sess_mp, priv_mp);
> }
> if (options->op_type == CPERF_DOCSIS) {
> enum rte_security_docsis_direction direction;
> @@ -664,7 +664,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
>
> /* Create security session */
> return (void *)rte_security_session_create(ctx,
> - &sess_conf, priv_mp);
> + &sess_conf, sess_mp, priv_mp);
> }
> #endif
> sess = rte_cryptodev_sym_session_create(sess_mp);
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 70bf6fe2c..6d7da1408 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -7219,7 +7219,8 @@ test_pdcp_proto(int i, int oop,
>
> /* Create security session */
> ut_params->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ts_params->session_priv_mpool);
> + &sess_conf, ts_params->session_mpool,
> + ts_params->session_priv_mpool);
>
> if (!ut_params->sec_session) {
> printf("TestCase %s()-%d line %d failed %s: ",
> @@ -7479,7 +7480,8 @@ test_pdcp_proto_SGL(int i, int oop,
>
> /* Create security session */
> ut_params->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ts_params->session_priv_mpool);
> + &sess_conf, ts_params->session_mpool,
> + ts_params->session_priv_mpool);
>
> if (!ut_params->sec_session) {
> printf("TestCase %s()-%d line %d failed %s: ",
> @@ -7836,6 +7838,7 @@ test_docsis_proto_uplink(int i, struct docsis_test_data *d_td)
>
> /* Create security session */
> ut_params->sec_session = rte_security_session_create(ctx, &sess_conf,
> + ts_params->session_mpool,
> ts_params->session_priv_mpool);
>
> if (!ut_params->sec_session) {
> @@ -8011,6 +8014,7 @@ test_docsis_proto_downlink(int i, struct docsis_test_data *d_td)
>
> /* Create security session */
> ut_params->sec_session = rte_security_session_create(ctx, &sess_conf,
> + ts_params->session_mpool,
> ts_params->session_priv_mpool);
>
> if (!ut_params->sec_session) {
> diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
> index 79d00d7e0..9ad07a179 100644
> --- a/app/test/test_ipsec.c
> +++ b/app/test/test_ipsec.c
> @@ -632,7 +632,8 @@ create_dummy_sec_session(struct ipsec_unitest_params *ut,
> static struct rte_security_session_conf conf;
>
> ut->ss[j].security.ses = rte_security_session_create(&dummy_sec_ctx,
> - &conf, qp->mp_session_private);
> + &conf, qp->mp_session,
> + qp->mp_session_private);
>
> if (ut->ss[j].security.ses == NULL)
> return -ENOMEM;
> diff --git a/app/test/test_security.c b/app/test/test_security.c
> index 77fd5adc6..ed7de348f 100644
> --- a/app/test/test_security.c
> +++ b/app/test/test_security.c
> @@ -237,6 +237,7 @@ static struct mock_session_create_data {
> struct rte_security_session_conf *conf;
> struct rte_security_session *sess;
> struct rte_mempool *mp;
> + struct rte_mempool *priv_mp;
>
> int ret;
>
session_create op is now called with private mbuf, so you need also to
update assert in mock session_create:
@@ -248,13 +249,13 @@ static int
mock_session_create(void *device,
struct rte_security_session_conf *conf,
struct rte_security_session *sess,
- struct rte_mempool *mp)
+ struct rte_mempool *priv_mp)
{
mock_session_create_exp.called++;
MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
device);
MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp, conf);
- MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp, mp);
+ MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
priv_mp);
mock_session_create_exp.sess = sess;
> @@ -502,6 +503,7 @@ struct rte_security_ops mock_ops = {
> */
> static struct security_testsuite_params {
> struct rte_mempool *session_mpool;
> + struct rte_mempool *session_priv_mpool;
> } testsuite_params = { NULL };
>
> /**
> @@ -525,6 +527,7 @@ static struct security_unittest_params {
> };
>
> #define SECURITY_TEST_MEMPOOL_NAME "SecurityTestsMempoolName"
> +#define SECURITY_TEST_PRIV_MEMPOOL_NAME "SecurityTestsPrivMempoolName"
Please make the mempool name shorter, otherwise it causes tests to fail:
EAL: Test assert testsuite_setup line 558 failed: Cannot create priv
mempool File name too long
> #define SECURITY_TEST_MEMPOOL_SIZE 15
> #define SECURITY_TEST_SESSION_OBJECT_SIZE sizeof(struct rte_security_session)
>
> @@ -545,6 +548,17 @@ testsuite_setup(void)
> SOCKET_ID_ANY, 0);
> TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
> "Cannot create mempool %s\n", rte_strerror(rte_errno));
> +
> + ts_params->session_priv_mpool = rte_mempool_create(
> + SECURITY_TEST_PRIV_MEMPOOL_NAME,
> + SECURITY_TEST_MEMPOOL_SIZE,
> + rte_security_session_get_size(&unittest_params.ctx),
> + 0, 0, NULL, NULL, NULL, NULL,
> + SOCKET_ID_ANY, 0);
> + TEST_ASSERT_NOT_NULL(ts_params->session_priv_mpool,
> + "Cannot create priv mempool %s\n",
> + rte_strerror(rte_errno));
> +
If creation of private data mpool fails, primary mempool need to be
freed before function returns failure code.
> return TEST_SUCCESS;
> }
>
> @@ -659,7 +673,8 @@ ut_setup_with_session(void)
> mock_session_create_exp.ret = 0;
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_NOT_NULL(rte_security_session_create,
> sess);
> TEST_ASSERT_EQUAL(sess, mock_session_create_exp.sess,
> @@ -701,7 +716,8 @@ test_session_create_inv_context(void)
> struct rte_security_session *sess;
>
> sess = rte_security_session_create(NULL, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -725,7 +741,8 @@ test_session_create_inv_context_ops(void)
> ut_params->ctx.ops = NULL;
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -749,7 +766,8 @@ test_session_create_inv_context_ops_fun(void)
> ut_params->ctx.ops = &empty_ops;
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -770,7 +788,8 @@ test_session_create_inv_configuration(void)
> struct rte_security_session *sess;
>
> sess = rte_security_session_create(&ut_params->ctx, NULL,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -790,7 +809,7 @@ test_session_create_inv_mempool(void)
> struct rte_security_session *sess;
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - NULL);
> + NULL, NULL);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -824,7 +843,8 @@ test_session_create_mempool_empty(void)
> TEST_ASSERT_MEMPOOL_USAGE(SECURITY_TEST_MEMPOOL_SIZE);
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> @@ -853,10 +873,12 @@ test_session_create_ops_failure(void)
> mock_session_create_exp.device = NULL;
> mock_session_create_exp.conf = &ut_params->conf;
> mock_session_create_exp.mp = ts_params->session_mpool;
> + mock_session_create_exp.priv_mp = ts_params->session_priv_mpool;
> mock_session_create_exp.ret = -1; /* Return failure status. */
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 1);
> @@ -879,10 +901,12 @@ test_session_create_success(void)
> mock_session_create_exp.device = NULL;
> mock_session_create_exp.conf = &ut_params->conf;
> mock_session_create_exp.mp = ts_params->session_mpool;
> + mock_session_create_exp.priv_mp = ts_params->session_priv_mpool;
> mock_session_create_exp.ret = 0; /* Return success status. */
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> - ts_params->session_mpool);
> + ts_params->session_mpool,
> + ts_params->session_priv_mpool);
> TEST_ASSERT_MOCK_FUNCTION_CALL_NOT_NULL(rte_security_session_create,
> sess);
> TEST_ASSERT_EQUAL(sess, mock_session_create_exp.sess,
> diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
> index 127da2e4f..cff0653f5 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -533,8 +533,10 @@ and this allows further acceleration of the offload of Crypto workloads.
>
> The Security framework provides APIs to create and free sessions for crypto/ethernet
> devices, where sessions are mempool objects. It is the application's responsibility
> -to create and manage the session mempools. The mempool object size should be able to
> -accommodate the driver's private data of security session.
> +to create and manage two session mempools - one for session and other for session
> +private data. The mempool object size should be able to accommodate the driver's
> +private data of security session. The application can get the size of session private
> +data using API ``rte_security_session_get_size``.
>
> Once the session mempools have been created, ``rte_security_session_create()``
> is used to allocate and initialize a session for the required crypto/ethernet device.
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 345c38d5b..84be88a13 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -263,13 +263,6 @@ Deprecation Notices
> This feature faces reliability issues and is often conflicting with
> new features being implemented.
>
> -* security: The API ``rte_security_session_create`` takes only single mempool
> - for session and session private data. So the application need to create
> - mempool for twice the number of sessions needed and will also lead to
> - wastage of memory as session private data need more memory compared to session.
> - Hence the API will be modified to take two mempool pointers - one for session
> - and one for private data.
> -
> * cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algorithm``,
> ``RTE_CRYPTO_CIPHER_LIST_END`` from ``enum rte_crypto_cipher_algorithm`` and
> ``RTE_CRYPTO_AUTH_LIST_END`` from ``enum rte_crypto_auth_algorithm``
> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
> index df227a177..04c1a1b81 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -84,6 +84,12 @@ API Changes
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> +* security: The API ``rte_security_session_create`` is updated to take two
> + mempool objects one for session and other for session private data.
> + So the application need to create two mempools and get the size of session
> + private data using API ``rte_security_session_get_size`` for private session
> + mempool.
> +
>
> ABI Changes
> -----------
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 8ba15d23c..55a5ea9f4 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -2351,12 +2351,8 @@ session_pool_init(struct socket_ctx *ctx, int32_t socket_id, size_t sess_sz)
>
> snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> "sess_mp_%u", socket_id);
> - /*
> - * Doubled due to rte_security_session_create() uses one mempool for
> - * session and for session private data.
> - */
> nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
> - rte_lcore_count()) * 2;
> + rte_lcore_count());
> sess_mp = rte_cryptodev_sym_session_pool_create(
> mp_name, nb_sess, sess_sz, CDEV_MP_CACHE_SZ, 0,
> socket_id);
> @@ -2379,12 +2375,8 @@ session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
>
> snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> "sess_mp_priv_%u", socket_id);
> - /*
> - * Doubled due to rte_security_session_create() uses one mempool for
> - * session and for session private data.
> - */
> nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
> - rte_lcore_count()) * 2;
> + rte_lcore_count());
> sess_mp = rte_mempool_create(mp_name,
> nb_sess,
> sess_sz,
> diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
> index 01faa7ac7..6baeeb342 100644
> --- a/examples/ipsec-secgw/ipsec.c
> +++ b/examples/ipsec-secgw/ipsec.c
> @@ -117,7 +117,8 @@ create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa,
> set_ipsec_conf(sa, &(sess_conf.ipsec));
>
> ips->security.ses = rte_security_session_create(ctx,
> - &sess_conf, ipsec_ctx->session_priv_pool);
> + &sess_conf, ipsec_ctx->session_pool,
> + ipsec_ctx->session_priv_pool);
> if (ips->security.ses == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> @@ -198,7 +199,8 @@ create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa,
> }
>
> ips->security.ses = rte_security_session_create(sec_ctx,
> - &sess_conf, skt_ctx->session_pool);
> + &sess_conf, skt_ctx->session_pool,
> + skt_ctx->session_priv_pool);
> if (ips->security.ses == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> @@ -378,7 +380,8 @@ create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa,
> sess_conf.userdata = (void *) sa;
>
> ips->security.ses = rte_security_session_create(sec_ctx,
> - &sess_conf, skt_ctx->session_pool);
> + &sess_conf, skt_ctx->session_pool,
> + skt_ctx->session_priv_pool);
> if (ips->security.ses == NULL) {
> RTE_LOG(ERR, IPSEC,
> "SEC Session init failed: err: %d\n", ret);
> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> index 515c29e04..293ca747d 100644
> --- a/lib/librte_security/rte_security.c
> +++ b/lib/librte_security/rte_security.c
> @@ -26,7 +26,8 @@
> struct rte_security_session *
> rte_security_session_create(struct rte_security_ctx *instance,
> struct rte_security_session_conf *conf,
> - struct rte_mempool *mp)
> + struct rte_mempool *mp,
> + struct rte_mempool *priv_mp)
> {
> struct rte_security_session *sess = NULL;
>
> @@ -37,7 +38,8 @@ rte_security_session_create(struct rte_security_ctx *instance,
> if (rte_mempool_get(mp, (void **)&sess))
> return NULL;
>
> - if (instance->ops->session_create(instance->device, conf, sess, mp)) {
> + if (instance->ops->session_create(instance->device, conf,
> + sess, priv_mp)) {
> rte_mempool_put(mp, (void *)sess);
> return NULL;
> }
> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> index 16839e539..1710cdd6a 100644
> --- a/lib/librte_security/rte_security.h
> +++ b/lib/librte_security/rte_security.h
> @@ -386,6 +386,7 @@ struct rte_security_session {
> * @param instance security instance
> * @param conf session configuration parameters
> * @param mp mempool to allocate session objects from
> + * @param priv_mp mempool to allocate session private data objects from
> * @return
> * - On success, pointer to session
> * - On failure, NULL
> @@ -393,7 +394,8 @@ struct rte_security_session {
> struct rte_security_session *
> rte_security_session_create(struct rte_security_ctx *instance,
> struct rte_security_session_conf *conf,
> - struct rte_mempool *mp);
> + struct rte_mempool *mp,
> + struct rte_mempool *priv_mp);
>
> /**
> * Update security session as specified by the session configuration
Hi Akhil
> -----Original Message-----
> From: akhil.goyal@nxp.com <akhil.goyal@nxp.com>
> Sent: Thursday, September 3, 2020 9:10 PM
<snip>
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> 70bf6fe2c..6d7da1408 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -7219,7 +7219,8 @@ test_pdcp_proto(int i, int oop,
>
> /* Create security session */
> ut_params->sec_session = rte_security_session_create(ctx,
> - &sess_conf, ts_params-
> >session_priv_mpool);
> + &sess_conf, ts_params->session_mpool,
> + ts_params->session_priv_mpool);
[DC] ts_params->session_mpool is a cryptodev sym session pool. The assumption then in these security tests is that
security sessions are smaller than cryptodev sym sessions. This is currently true, but may not always be.
There should possibly be a new mempool created for security sessions.
Or at least an assert somewhere to check a security session is smaller than a cryptodev sym session, so that this doesn't
catch someone out in the future if security session grows in size.
The same comment applies to the crypto-perf-test and test_ipsec too
<snip>
> diff --git a/app/test/test_security.c b/app/test/test_security.c index
> 77fd5adc6..ed7de348f 100644
> --- a/app/test/test_security.c
> +++ b/app/test/test_security.c
> @@ -237,6 +237,7 @@ static struct mock_session_create_data {
> struct rte_security_session_conf *conf;
> struct rte_security_session *sess;
> struct rte_mempool *mp;
> + struct rte_mempool *priv_mp;
>
<snip>
> 790,7 +809,7 @@ test_session_create_inv_mempool(void)
> struct rte_security_session *sess;
>
> sess = rte_security_session_create(&ut_params->ctx, &ut_params-
> >conf,
> - NULL);
> + NULL, NULL);
[DC] This test test_session_create_inv_mempool() should have the priv_mp set to a valid
value (i.e. ts_params->session_priv_mpool), and a new test function should be added where
mp is valid, but priv_mp is NULL - this way we test for validity of both mempools independently.
<snip>
> a/doc/guides/prog_guide/rte_security.rst
> b/doc/guides/prog_guide/rte_security.rst
> index 127da2e4f..cff0653f5 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -533,8 +533,10 @@ and this allows further acceleration of the offload of
> Crypto workloads.
>
> The Security framework provides APIs to create and free sessions for
> crypto/ethernet devices, where sessions are mempool objects. It is the
> application's responsibility -to create and manage the session mempools. The
> mempool object size should be able to -accommodate the driver's private
> data of security session.
> +to create and manage two session mempools - one for session and other
> +for session private data. The mempool object size should be able to
> +accommodate the driver's private data of security session. The
> +application can get the size of session private data using API
> ``rte_security_session_get_size``.
[DC] This sentence should be updated to specify it's the private session data mempool that is being referred to
"The mempool object size should be able to accommodate the driver's private data of security session."
=>
"The private session data mempool object size should be able to accommodate the driver's private data of security
session."
Also, a sentence about the required size of the session mempool should also be added.
<snip>
> diff --git a/doc/guides/rel_notes/release_20_11.rst
> b/doc/guides/rel_notes/release_20_11.rst
> index df227a177..04c1a1b81 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -84,6 +84,12 @@ API Changes
> Also, make sure to start the actual text at the margin.
> =======================================================
>
> +* security: The API ``rte_security_session_create`` is updated to take
> +two
> + mempool objects one for session and other for session private data.
> + So the application need to create two mempools and get the size of
> +session
> + private data using API ``rte_security_session_get_size`` for private
> +session
> + mempool.
> +
[DC] Many of the PMDs which support security don't implement the session_get_size
callback. There's probably a job here for each PMD owner to add support for this callback.
>
> ABI Changes
> -----------
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> secgw/ipsec-secgw.c
> index 8ba15d23c..55a5ea9f4 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
<snip>
> @@ -2379,12 +2375,8 @@ session_priv_pool_init(struct socket_ctx *ctx,
> int32_t socket_id,
>
> snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> "sess_mp_priv_%u", socket_id);
> - /*
> - * Doubled due to rte_security_session_create() uses one mempool
> for
> - * session and for session private data.
> - */
> nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
> - rte_lcore_count()) * 2;
> + rte_lcore_count());
[DC] A change to double the number of sessions was made in test-crypto-perf when adding DOCSIS security protocol to this tester.
It was needed as both session and private session data was pulled from same mempool.
This change can now be reverted like this...
diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
index 8f8e580e4..6a71aff5f 100644
--- a/app/test-crypto-perf/main.c
+++ b/app/test-crypto-perf/main.c
@@ -248,7 +248,7 @@ cperf_initialize_cryptodev(struct cperf_options *opts, uint8_t *enabled_cdevs)
#endif
} else
sessions_needed = enabled_cdev_count *
- opts->nb_qps * 2;
+ opts->nb_qps;
<snip>
> git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> index 515c29e04..293ca747d 100644
> --- a/lib/librte_security/rte_security.c
> +++ b/lib/librte_security/rte_security.c
> @@ -26,7 +26,8 @@
> struct rte_security_session *
> rte_security_session_create(struct rte_security_ctx *instance,
> struct rte_security_session_conf *conf,
> - struct rte_mempool *mp)
> + struct rte_mempool *mp,
> + struct rte_mempool *priv_mp)
> {
> struct rte_security_session *sess = NULL;
[DC] Need to add a validity check for priv_mp to rte_security_session_create().
The cryptodev API checks both mp and priv_mp are not NULL, so security should do the same
RTE_PTR_OR_ERR_RET(priv_mp, NULL);
>
<snip>
> --
> 2.17.1
[DC] This API change has highlighted a bug in the security callbacks in the AESNi-MB PMD, specifically in
aesni_mb_pmd_sec_sess_destroy() in drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
Before putting the private session data back to the mempool, this function clears the data with a memset.
But the bug is that it cleared the security session struct instead of the private aesni_mb_session struct.
This didn't show up previously because the elements of the mempool were large, because both security session and private session
data came from the same mempool with large objects . But now that the security session mempool object are much smaller, this causes
a seg fault
The fix is as follows:
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index 2362f0c3c..b11d7f12b 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -911,7 +911,7 @@ aesni_mb_pmd_sec_sess_destroy(void *dev __rte_unused,
if (sess_priv) {
struct rte_mempool *sess_mp = rte_mempool_from_obj(sess_priv);
- memset(sess, 0, sizeof(struct aesni_mb_session));
+ memset(sess_priv, 0, sizeof(struct aesni_mb_session));
set_sec_session_private_data(sess, NULL);
rte_mempool_put(sess_mp, sess_priv);
}
Can this be fixed as part of this patchset or separate fix needed?
Hi David,
> Hi Akhil
>
> > -----Original Message-----
> > From: akhil.goyal@nxp.com <akhil.goyal@nxp.com>
> > Sent: Thursday, September 3, 2020 9:10 PM
>
> <snip>
>
> > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index
> > 70bf6fe2c..6d7da1408 100644
> > --- a/app/test/test_cryptodev.c
> > +++ b/app/test/test_cryptodev.c
> > @@ -7219,7 +7219,8 @@ test_pdcp_proto(int i, int oop,
> >
> > /* Create security session */
> > ut_params->sec_session = rte_security_session_create(ctx,
> > - &sess_conf, ts_params-
> > >session_priv_mpool);
> > + &sess_conf, ts_params->session_mpool,
> > + ts_params->session_priv_mpool);
>
> [DC] ts_params->session_mpool is a cryptodev sym session pool. The
> assumption then in these security tests is that
> security sessions are smaller than cryptodev sym sessions. This is currently true,
> but may not always be.
>
> There should possibly be a new mempool created for security sessions.
> Or at least an assert somewhere to check a security session is smaller than a
> cryptodev sym session, so that this doesn't
> catch someone out in the future if security session grows in size.
>
> The same comment applies to the crypto-perf-test and test_ipsec too
Fixed for test and crypto-perf. Test_ipsec is not exactly using a security session.
Fixing that is out of scope of this patch.
>
> <snip>
>
> > diff --git a/app/test/test_security.c b/app/test/test_security.c index
> > 77fd5adc6..ed7de348f 100644
> > --- a/app/test/test_security.c
> > +++ b/app/test/test_security.c
> > @@ -237,6 +237,7 @@ static struct mock_session_create_data {
> > struct rte_security_session_conf *conf;
> > struct rte_security_session *sess;
> > struct rte_mempool *mp;
> > + struct rte_mempool *priv_mp;
> >
>
> <snip>
>
> > 790,7 +809,7 @@ test_session_create_inv_mempool(void)
> > struct rte_security_session *sess;
> >
> > sess = rte_security_session_create(&ut_params->ctx, &ut_params-
> > >conf,
> > - NULL);
> > + NULL, NULL);
>
> [DC] This test test_session_create_inv_mempool() should have the priv_mp set
> to a valid
> value (i.e. ts_params->session_priv_mpool), and a new test function should be
> added where
> mp is valid, but priv_mp is NULL - this way we test for validity of both mempools
> independently.
I would say that would be an overkill with not much gain.
Both mempool should be created before session is created. That is quite obvious. Isn't it?
>
> <snip>
>
> > a/doc/guides/prog_guide/rte_security.rst
> > b/doc/guides/prog_guide/rte_security.rst
> > index 127da2e4f..cff0653f5 100644
> > --- a/doc/guides/prog_guide/rte_security.rst
> > +++ b/doc/guides/prog_guide/rte_security.rst
> > @@ -533,8 +533,10 @@ and this allows further acceleration of the offload of
> > Crypto workloads.
> >
> > The Security framework provides APIs to create and free sessions for
> > crypto/ethernet devices, where sessions are mempool objects. It is the
> > application's responsibility -to create and manage the session mempools. The
> > mempool object size should be able to -accommodate the driver's private
> > data of security session.
> > +to create and manage two session mempools - one for session and other
> > +for session private data. The mempool object size should be able to
> > +accommodate the driver's private data of security session. The
> > +application can get the size of session private data using API
> > ``rte_security_session_get_size``.
>
> [DC] This sentence should be updated to specify it's the private session data
> mempool that is being referred to
>
> "The mempool object size should be able to accommodate the driver's private
> data of security session."
> =>
> "The private session data mempool object size should be able to accommodate
> the driver's private data of security
> session."
>
> Also, a sentence about the required size of the session mempool should also be
> added.
Fixed in v2
>
> <snip>
>
> > diff --git a/doc/guides/rel_notes/release_20_11.rst
> > b/doc/guides/rel_notes/release_20_11.rst
> > index df227a177..04c1a1b81 100644
> > --- a/doc/guides/rel_notes/release_20_11.rst
> > +++ b/doc/guides/rel_notes/release_20_11.rst
> > @@ -84,6 +84,12 @@ API Changes
> > Also, make sure to start the actual text at the margin.
> > =======================================================
> >
> > +* security: The API ``rte_security_session_create`` is updated to take
> > +two
> > + mempool objects one for session and other for session private data.
> > + So the application need to create two mempools and get the size of
> > +session
> > + private data using API ``rte_security_session_get_size`` for private
> > +session
> > + mempool.
> > +
>
> [DC] Many of the PMDs which support security don't implement the
> session_get_size
> callback. There's probably a job here for each PMD owner to add support for this
> callback.
>
If a PMD is supporting rte_security, then it should comply with the APIs which are required.
> >
> > ABI Changes
> > -----------
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> > secgw/ipsec-secgw.c
> > index 8ba15d23c..55a5ea9f4 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
>
> <snip>
>
> > @@ -2379,12 +2375,8 @@ session_priv_pool_init(struct socket_ctx *ctx,
> > int32_t socket_id,
> >
> > snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> > "sess_mp_priv_%u", socket_id);
> > - /*
> > - * Doubled due to rte_security_session_create() uses one mempool
> > for
> > - * session and for session private data.
> > - */
> > nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
> > - rte_lcore_count()) * 2;
> > + rte_lcore_count());
>
> [DC] A change to double the number of sessions was made in test-crypto-perf
> when adding DOCSIS security protocol to this tester.
> It was needed as both session and private session data was pulled from same
> mempool.
> This change can now be reverted like this...
Fixed in v2
>
> diff --git a/app/test-crypto-perf/main.c b/app/test-crypto-perf/main.c
> index 8f8e580e4..6a71aff5f 100644
> --- a/app/test-crypto-perf/main.c
> +++ b/app/test-crypto-perf/main.c
> @@ -248,7 +248,7 @@ cperf_initialize_cryptodev(struct cperf_options *opts,
> uint8_t *enabled_cdevs)
> #endif
> } else
> sessions_needed = enabled_cdev_count *
> - opts->nb_qps * 2;
> + opts->nb_qps;
>
> <snip>
>
> > git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> > index 515c29e04..293ca747d 100644
> > --- a/lib/librte_security/rte_security.c
> > +++ b/lib/librte_security/rte_security.c
> > @@ -26,7 +26,8 @@
> > struct rte_security_session *
> > rte_security_session_create(struct rte_security_ctx *instance,
> > struct rte_security_session_conf *conf,
> > - struct rte_mempool *mp)
> > + struct rte_mempool *mp,
> > + struct rte_mempool *priv_mp)
> > {
> > struct rte_security_session *sess = NULL;
>
> [DC] Need to add a validity check for priv_mp to rte_security_session_create().
> The cryptodev API checks both mp and priv_mp are not NULL, so security should
> do the same
>
> RTE_PTR_OR_ERR_RET(priv_mp, NULL);
Fixed in v2
>
> >
>
> <snip>
>
> > --
> > 2.17.1
>
> [DC] This API change has highlighted a bug in the security callbacks in the AESNi-
> MB PMD, specifically in
> aesni_mb_pmd_sec_sess_destroy() in
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
>
> Before putting the private session data back to the mempool, this function
> clears the data with a memset.
> But the bug is that it cleared the security session struct instead of the private
> aesni_mb_session struct.
> This didn't show up previously because the elements of the mempool were large,
> because both security session and private session
> data came from the same mempool with large objects . But now that the
> security session mempool object are much smaller, this causes
> a seg fault
>
> The fix is as follows:
>
> diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> index 2362f0c3c..b11d7f12b 100644
> --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
> @@ -911,7 +911,7 @@ aesni_mb_pmd_sec_sess_destroy(void *dev
> __rte_unused,
>
> if (sess_priv) {
> struct rte_mempool *sess_mp = rte_mempool_from_obj(sess_priv);
> - memset(sess, 0, sizeof(struct aesni_mb_session));
> + memset(sess_priv, 0, sizeof(struct aesni_mb_session));
> set_sec_session_private_data(sess, NULL);
> rte_mempool_put(sess_mp, sess_priv);
> }
>
> Can this be fixed as part of this patchset or separate fix needed?
This patch is already applied on the tree now.
Hi Lukasz,
Thanks for the review.
> > diff --git a/app/test/test_security.c b/app/test/test_security.c
> > index 77fd5adc6..ed7de348f 100644
> > --- a/app/test/test_security.c
> > +++ b/app/test/test_security.c
> > @@ -237,6 +237,7 @@ static struct mock_session_create_data {
> > struct rte_security_session_conf *conf;
> > struct rte_security_session *sess;
> > struct rte_mempool *mp;
> > + struct rte_mempool *priv_mp;
> >
> > int ret;
> >
> session_create op is now called with private mbuf, so you need also to
> update assert in mock session_create:
OK will be fixed in v2
>
> @@ -248,13 +249,13 @@ static int
> mock_session_create(void *device,
> struct rte_security_session_conf *conf,
> struct rte_security_session *sess,
> - struct rte_mempool *mp)
> + struct rte_mempool *priv_mp)
> {
> mock_session_create_exp.called++;
>
> MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
> device);
> MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
> conf);
> - MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
> mp);
> + MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_create_exp,
> priv_mp);
>
> mock_session_create_exp.sess = sess;
>
>
>
> > @@ -502,6 +503,7 @@ struct rte_security_ops mock_ops = {
> > */
> > static struct security_testsuite_params {
> > struct rte_mempool *session_mpool;
> > + struct rte_mempool *session_priv_mpool;
> > } testsuite_params = { NULL };
> >
> > /**
> > @@ -525,6 +527,7 @@ static struct security_unittest_params {
> > };
> >
> > #define SECURITY_TEST_MEMPOOL_NAME "SecurityTestsMempoolName"
> > +#define SECURITY_TEST_PRIV_MEMPOOL_NAME
> "SecurityTestsPrivMempoolName"
> Please make the mempool name shorter, otherwise it causes tests to fail:
>
> EAL: Test assert testsuite_setup line 558 failed: Cannot create priv
> mempool File name too long
>
Fixed in v2
> > #define SECURITY_TEST_MEMPOOL_SIZE 15
> > #define SECURITY_TEST_SESSION_OBJECT_SIZE sizeof(struct
> rte_security_session)
> >
> > @@ -545,6 +548,17 @@ testsuite_setup(void)
> > SOCKET_ID_ANY, 0);
> > TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
> > "Cannot create mempool %s\n",
> rte_strerror(rte_errno));
> > +
> > + ts_params->session_priv_mpool = rte_mempool_create(
> > + SECURITY_TEST_PRIV_MEMPOOL_NAME,
> > + SECURITY_TEST_MEMPOOL_SIZE,
> > + rte_security_session_get_size(&unittest_params.ctx),
> > + 0, 0, NULL, NULL, NULL, NULL,
> > + SOCKET_ID_ANY, 0);
> > + TEST_ASSERT_NOT_NULL(ts_params->session_priv_mpool,
> > + "Cannot create priv mempool %s\n",
> > + rte_strerror(rte_errno));
> > +
> If creation of private data mpool fails, primary mempool need to be
> freed before function returns failure code.
This is an issue in whole of the file.
However, have fixed it in v2 for this particular case in v2.
@@ -621,7 +621,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
/* Create security session */
return (void *)rte_security_session_create(ctx,
- &sess_conf, sess_mp);
+ &sess_conf, sess_mp, priv_mp);
}
if (options->op_type == CPERF_DOCSIS) {
enum rte_security_docsis_direction direction;
@@ -664,7 +664,7 @@ cperf_create_session(struct rte_mempool *sess_mp,
/* Create security session */
return (void *)rte_security_session_create(ctx,
- &sess_conf, priv_mp);
+ &sess_conf, sess_mp, priv_mp);
}
#endif
sess = rte_cryptodev_sym_session_create(sess_mp);
@@ -7219,7 +7219,8 @@ test_pdcp_proto(int i, int oop,
/* Create security session */
ut_params->sec_session = rte_security_session_create(ctx,
- &sess_conf, ts_params->session_priv_mpool);
+ &sess_conf, ts_params->session_mpool,
+ ts_params->session_priv_mpool);
if (!ut_params->sec_session) {
printf("TestCase %s()-%d line %d failed %s: ",
@@ -7479,7 +7480,8 @@ test_pdcp_proto_SGL(int i, int oop,
/* Create security session */
ut_params->sec_session = rte_security_session_create(ctx,
- &sess_conf, ts_params->session_priv_mpool);
+ &sess_conf, ts_params->session_mpool,
+ ts_params->session_priv_mpool);
if (!ut_params->sec_session) {
printf("TestCase %s()-%d line %d failed %s: ",
@@ -7836,6 +7838,7 @@ test_docsis_proto_uplink(int i, struct docsis_test_data *d_td)
/* Create security session */
ut_params->sec_session = rte_security_session_create(ctx, &sess_conf,
+ ts_params->session_mpool,
ts_params->session_priv_mpool);
if (!ut_params->sec_session) {
@@ -8011,6 +8014,7 @@ test_docsis_proto_downlink(int i, struct docsis_test_data *d_td)
/* Create security session */
ut_params->sec_session = rte_security_session_create(ctx, &sess_conf,
+ ts_params->session_mpool,
ts_params->session_priv_mpool);
if (!ut_params->sec_session) {
@@ -632,7 +632,8 @@ create_dummy_sec_session(struct ipsec_unitest_params *ut,
static struct rte_security_session_conf conf;
ut->ss[j].security.ses = rte_security_session_create(&dummy_sec_ctx,
- &conf, qp->mp_session_private);
+ &conf, qp->mp_session,
+ qp->mp_session_private);
if (ut->ss[j].security.ses == NULL)
return -ENOMEM;
@@ -237,6 +237,7 @@ static struct mock_session_create_data {
struct rte_security_session_conf *conf;
struct rte_security_session *sess;
struct rte_mempool *mp;
+ struct rte_mempool *priv_mp;
int ret;
@@ -502,6 +503,7 @@ struct rte_security_ops mock_ops = {
*/
static struct security_testsuite_params {
struct rte_mempool *session_mpool;
+ struct rte_mempool *session_priv_mpool;
} testsuite_params = { NULL };
/**
@@ -525,6 +527,7 @@ static struct security_unittest_params {
};
#define SECURITY_TEST_MEMPOOL_NAME "SecurityTestsMempoolName"
+#define SECURITY_TEST_PRIV_MEMPOOL_NAME "SecurityTestsPrivMempoolName"
#define SECURITY_TEST_MEMPOOL_SIZE 15
#define SECURITY_TEST_SESSION_OBJECT_SIZE sizeof(struct rte_security_session)
@@ -545,6 +548,17 @@ testsuite_setup(void)
SOCKET_ID_ANY, 0);
TEST_ASSERT_NOT_NULL(ts_params->session_mpool,
"Cannot create mempool %s\n", rte_strerror(rte_errno));
+
+ ts_params->session_priv_mpool = rte_mempool_create(
+ SECURITY_TEST_PRIV_MEMPOOL_NAME,
+ SECURITY_TEST_MEMPOOL_SIZE,
+ rte_security_session_get_size(&unittest_params.ctx),
+ 0, 0, NULL, NULL, NULL, NULL,
+ SOCKET_ID_ANY, 0);
+ TEST_ASSERT_NOT_NULL(ts_params->session_priv_mpool,
+ "Cannot create priv mempool %s\n",
+ rte_strerror(rte_errno));
+
return TEST_SUCCESS;
}
@@ -659,7 +673,8 @@ ut_setup_with_session(void)
mock_session_create_exp.ret = 0;
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_NOT_NULL(rte_security_session_create,
sess);
TEST_ASSERT_EQUAL(sess, mock_session_create_exp.sess,
@@ -701,7 +716,8 @@ test_session_create_inv_context(void)
struct rte_security_session *sess;
sess = rte_security_session_create(NULL, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -725,7 +741,8 @@ test_session_create_inv_context_ops(void)
ut_params->ctx.ops = NULL;
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -749,7 +766,8 @@ test_session_create_inv_context_ops_fun(void)
ut_params->ctx.ops = &empty_ops;
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -770,7 +788,8 @@ test_session_create_inv_configuration(void)
struct rte_security_session *sess;
sess = rte_security_session_create(&ut_params->ctx, NULL,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -790,7 +809,7 @@ test_session_create_inv_mempool(void)
struct rte_security_session *sess;
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- NULL);
+ NULL, NULL);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -824,7 +843,8 @@ test_session_create_mempool_empty(void)
TEST_ASSERT_MEMPOOL_USAGE(SECURITY_TEST_MEMPOOL_SIZE);
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
@@ -853,10 +873,12 @@ test_session_create_ops_failure(void)
mock_session_create_exp.device = NULL;
mock_session_create_exp.conf = &ut_params->conf;
mock_session_create_exp.mp = ts_params->session_mpool;
+ mock_session_create_exp.priv_mp = ts_params->session_priv_mpool;
mock_session_create_exp.ret = -1; /* Return failure status. */
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
sess, NULL, "%p");
TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 1);
@@ -879,10 +901,12 @@ test_session_create_success(void)
mock_session_create_exp.device = NULL;
mock_session_create_exp.conf = &ut_params->conf;
mock_session_create_exp.mp = ts_params->session_mpool;
+ mock_session_create_exp.priv_mp = ts_params->session_priv_mpool;
mock_session_create_exp.ret = 0; /* Return success status. */
sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
- ts_params->session_mpool);
+ ts_params->session_mpool,
+ ts_params->session_priv_mpool);
TEST_ASSERT_MOCK_FUNCTION_CALL_NOT_NULL(rte_security_session_create,
sess);
TEST_ASSERT_EQUAL(sess, mock_session_create_exp.sess,
@@ -533,8 +533,10 @@ and this allows further acceleration of the offload of Crypto workloads.
The Security framework provides APIs to create and free sessions for crypto/ethernet
devices, where sessions are mempool objects. It is the application's responsibility
-to create and manage the session mempools. The mempool object size should be able to
-accommodate the driver's private data of security session.
+to create and manage two session mempools - one for session and other for session
+private data. The mempool object size should be able to accommodate the driver's
+private data of security session. The application can get the size of session private
+data using API ``rte_security_session_get_size``.
Once the session mempools have been created, ``rte_security_session_create()``
is used to allocate and initialize a session for the required crypto/ethernet device.
@@ -263,13 +263,6 @@ Deprecation Notices
This feature faces reliability issues and is often conflicting with
new features being implemented.
-* security: The API ``rte_security_session_create`` takes only single mempool
- for session and session private data. So the application need to create
- mempool for twice the number of sessions needed and will also lead to
- wastage of memory as session private data need more memory compared to session.
- Hence the API will be modified to take two mempool pointers - one for session
- and one for private data.
-
* cryptodev: ``RTE_CRYPTO_AEAD_LIST_END`` from ``enum rte_crypto_aead_algorithm``,
``RTE_CRYPTO_CIPHER_LIST_END`` from ``enum rte_crypto_cipher_algorithm`` and
``RTE_CRYPTO_AUTH_LIST_END`` from ``enum rte_crypto_auth_algorithm``
@@ -84,6 +84,12 @@ API Changes
Also, make sure to start the actual text at the margin.
=======================================================
+* security: The API ``rte_security_session_create`` is updated to take two
+ mempool objects one for session and other for session private data.
+ So the application need to create two mempools and get the size of session
+ private data using API ``rte_security_session_get_size`` for private session
+ mempool.
+
ABI Changes
-----------
@@ -2351,12 +2351,8 @@ session_pool_init(struct socket_ctx *ctx, int32_t socket_id, size_t sess_sz)
snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
"sess_mp_%u", socket_id);
- /*
- * Doubled due to rte_security_session_create() uses one mempool for
- * session and for session private data.
- */
nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
- rte_lcore_count()) * 2;
+ rte_lcore_count());
sess_mp = rte_cryptodev_sym_session_pool_create(
mp_name, nb_sess, sess_sz, CDEV_MP_CACHE_SZ, 0,
socket_id);
@@ -2379,12 +2375,8 @@ session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
"sess_mp_priv_%u", socket_id);
- /*
- * Doubled due to rte_security_session_create() uses one mempool for
- * session and for session private data.
- */
nb_sess = (get_nb_crypto_sessions() + CDEV_MP_CACHE_SZ *
- rte_lcore_count()) * 2;
+ rte_lcore_count());
sess_mp = rte_mempool_create(mp_name,
nb_sess,
sess_sz,
@@ -117,7 +117,8 @@ create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa,
set_ipsec_conf(sa, &(sess_conf.ipsec));
ips->security.ses = rte_security_session_create(ctx,
- &sess_conf, ipsec_ctx->session_priv_pool);
+ &sess_conf, ipsec_ctx->session_pool,
+ ipsec_ctx->session_priv_pool);
if (ips->security.ses == NULL) {
RTE_LOG(ERR, IPSEC,
"SEC Session init failed: err: %d\n", ret);
@@ -198,7 +199,8 @@ create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa,
}
ips->security.ses = rte_security_session_create(sec_ctx,
- &sess_conf, skt_ctx->session_pool);
+ &sess_conf, skt_ctx->session_pool,
+ skt_ctx->session_priv_pool);
if (ips->security.ses == NULL) {
RTE_LOG(ERR, IPSEC,
"SEC Session init failed: err: %d\n", ret);
@@ -378,7 +380,8 @@ create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa,
sess_conf.userdata = (void *) sa;
ips->security.ses = rte_security_session_create(sec_ctx,
- &sess_conf, skt_ctx->session_pool);
+ &sess_conf, skt_ctx->session_pool,
+ skt_ctx->session_priv_pool);
if (ips->security.ses == NULL) {
RTE_LOG(ERR, IPSEC,
"SEC Session init failed: err: %d\n", ret);
@@ -26,7 +26,8 @@
struct rte_security_session *
rte_security_session_create(struct rte_security_ctx *instance,
struct rte_security_session_conf *conf,
- struct rte_mempool *mp)
+ struct rte_mempool *mp,
+ struct rte_mempool *priv_mp)
{
struct rte_security_session *sess = NULL;
@@ -37,7 +38,8 @@ rte_security_session_create(struct rte_security_ctx *instance,
if (rte_mempool_get(mp, (void **)&sess))
return NULL;
- if (instance->ops->session_create(instance->device, conf, sess, mp)) {
+ if (instance->ops->session_create(instance->device, conf,
+ sess, priv_mp)) {
rte_mempool_put(mp, (void *)sess);
return NULL;
}
@@ -386,6 +386,7 @@ struct rte_security_session {
* @param instance security instance
* @param conf session configuration parameters
* @param mp mempool to allocate session objects from
+ * @param priv_mp mempool to allocate session private data objects from
* @return
* - On success, pointer to session
* - On failure, NULL
@@ -393,7 +394,8 @@ struct rte_security_session {
struct rte_security_session *
rte_security_session_create(struct rte_security_ctx *instance,
struct rte_security_session_conf *conf,
- struct rte_mempool *mp);
+ struct rte_mempool *mp,
+ struct rte_mempool *priv_mp);
/**
* Update security session as specified by the session configuration