Hi Akhil,
thank you for responding to review and for v3.
You patch currently does not apply:
dpdk$ git apply v3-security-update-session-create-API.patch
error: patch failed: doc/guides/rel_notes/deprecation.rst:164
error: doc/guides/rel_notes/deprecation.rst: patch does not apply
error: patch failed: doc/guides/rel_notes/release_20_11.rst:344
error: doc/guides/rel_notes/release_20_11.rst: patch does not apply
and I'm sorry but there are still few things - see inline comments
W dniu 14.10.2020 o 20:56, Akhil Goyal pisze:
> 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.go...@nxp.com>
> ---
> Changes in v3:
> fixed checkpatch issues.
> Added new test in test_security.c for priv_mempool
>
> Changes in V2:
> incorporated comments from Lukasz and David.
>
> app/test-crypto-perf/cperf_ops.c | 4 +-
> app/test-crypto-perf/main.c | 12 +-
> app/test/test_cryptodev.c | 18 ++-
> app/test/test_ipsec.c | 3 +-
> app/test/test_security.c | 160 ++++++++++++++++++++++---
> doc/guides/prog_guide/rte_security.rst | 8 +-
> 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 | 7 +-
> lib/librte_security/rte_security.h | 4 +-
> 12 files changed, 196 insertions(+), 54 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-crypto-perf/main.c b/app/test-crypto-perf/main.c
> index 62ae6048b..53864ffdd 100644
> --- a/app/test-crypto-perf/main.c
> +++ b/app/test-crypto-perf/main.c
> @@ -156,7 +156,14 @@ cperf_initialize_cryptodev(struct cperf_options *opts,
> uint8_t *enabled_cdevs)
> if (sess_size > max_sess_size)
> max_sess_size = sess_size;
> }
> -
> +#ifdef RTE_LIBRTE_SECURITY
> + for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
> + sess_size = rte_security_session_get_size(
> + rte_cryptodev_get_sec_ctx(cdev_id));
> + if (sess_size > max_sess_size)
> + max_sess_size = sess_size;
> + }
> +#endif
> /*
> * Calculate number of needed queue pairs, based on the amount
> * of available number of logical cores and crypto devices.
> @@ -247,8 +254,7 @@ cperf_initialize_cryptodev(struct cperf_options *opts,
> uint8_t *enabled_cdevs)
> opts->nb_qps * nb_slaves;
> #endif
> } else
> - sessions_needed = enabled_cdev_count *
> - opts->nb_qps * 2;
> + sessions_needed = enabled_cdev_count * opts->nb_qps;
>
> /*
> * A single session is required per queue pair
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index c7975ed01..9f1b92c51 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -773,9 +773,15 @@ testsuite_setup(void)
> unsigned int session_size =
> rte_cryptodev_sym_get_private_session_size(dev_id);
>
> +#ifdef RTE_LIBRTE_SECURITY
> + unsigned int security_session_size = rte_security_session_get_size(
> + rte_cryptodev_get_sec_ctx(dev_id));
> +
> + if (session_size < security_session_size)
> + session_size = security_session_size;
> +#endif
> /*
> - * Create mempool with maximum number of sessions * 2,
> - * to include the session headers
> + * Create mempool with maximum number of sessions.
> */
> if (info.sym.max_nb_sessions != 0 &&
> info.sym.max_nb_sessions < MAX_NB_SESSIONS) {
> @@ -7751,7 +7757,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: ",
> @@ -8011,7 +8018,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: ",
> @@ -8368,6 +8376,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) {
> @@ -8543,6 +8552,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..35ed6ff10 100644
> --- a/app/test/test_security.c
> +++ b/app/test/test_security.c
> @@ -200,6 +200,24 @@
> expected_mempool_usage, mempool_usage); \
> } while (0)
>
> +/**
> + * Verify usage of mempool by checking if number of allocated objects matches
> + * expectations. The mempool is used to manage objects for sessions priv
> data.
> + * A single object is acquired from mempool during session_create
> + * and put back in session_destroy.
> + *
> + * @param expected_priv_mp_usage expected number of used priv mp objects
> + */
> +#define TEST_ASSERT_PRIV_MP_USAGE(expected_priv_mp_usage) do {
> \
> + struct security_testsuite_params *ts_params = &testsuite_params;\
> + unsigned int priv_mp_usage; \
> + priv_mp_usage = rte_mempool_in_use_count( \
> + ts_params->session_priv_mpool); \
> + TEST_ASSERT_EQUAL(expected_priv_mp_usage, priv_mp_usage, \
> + "Expecting %u priv mempool allocations, "
> \
one tab less
> + "but there are %u allocated objects", \
> + expected_priv_mp_usage, priv_mp_usage); \
> +} while (0)
>
> /**
> * Mockup structures and functions for rte_security_ops;
> @@ -237,27 +255,38 @@ 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;
>
> int called;
> int failed;
> -} mock_session_create_exp = {NULL, NULL, NULL, NULL, 0, 0, 0};
> +} mock_session_create_exp = {NULL, NULL, NULL, NULL, NULL, 0, 0, 0};
>
> 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)
> {
> + void *sess_priv;
> + int ret;
> +
> 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);
> + ret = rte_mempool_get(priv_mp, &sess_priv);
> + TEST_ASSERT_EQUAL(0, ret,
> + "priv mempool does not have enough objects");
>
> + set_sec_session_private_data(sess, sess_priv);
if op function doesn't return 0, it shouldn't leave also sess_priv set
in sess.
Maybe put the code for getting sess_priv from mempool and setting it in
session inside:
if (mock_session_create_exp.ret == 0) {
...
}
> mock_session_create_exp.sess = sess;
>
> + if (mock_session_create_exp.ret != 0)
> + rte_mempool_put(priv_mp, sess_priv);
> +
> return mock_session_create_exp.ret;
> }
>
> @@ -363,8 +392,10 @@ static struct mock_session_destroy_data {
> static int
> mock_session_destroy(void *device, struct rte_security_session *sess)
> {
> - mock_session_destroy_exp.called++;
> + void *sess_priv = get_sec_session_private_data(sess);
>
> + mock_session_destroy_exp.called++;
> + rte_mempool_put(rte_mempool_from_obj(sess_priv), sess_priv);
sess_priv should be released only if op function is going to succeed.
You can check that in similar way as you did in create op by checking
mock_session_destroy_exp.ret
Otherwise testcase test_session_destroy_ops_failure might cause a
problem because your are putting same object twice into the mempool
(once in mock_session_destroy and 2nd time in ut_teardown when session
is destroyed)
> MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_destroy_exp, device);
> MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_destroy_exp, sess);
>
> @@ -502,6 +533,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 };
>
> /**
> @@ -524,9 +556,11 @@ static struct security_unittest_params {
> .sess = NULL,
> };
>
> -#define SECURITY_TEST_MEMPOOL_NAME "SecurityTestsMempoolName"
> +#define SECURITY_TEST_MEMPOOL_NAME "SecurityTestMp"
> +#define SECURITY_TEST_PRIV_MEMPOOL_NAME "SecurityTestPrivMp"
> #define SECURITY_TEST_MEMPOOL_SIZE 15
> -#define SECURITY_TEST_SESSION_OBJECT_SIZE sizeof(struct rte_security_session)
> +#define SECURITY_TEST_SESSION_OBJ_SZ sizeof(struct rte_security_session)
> +#define SECURITY_TEST_SESSION_PRIV_OBJ_SZ 64
>
> /**
> * testsuite_setup initializes whole test suite parameters.
> @@ -540,11 +574,27 @@ testsuite_setup(void)
> ts_params->session_mpool = rte_mempool_create(
> SECURITY_TEST_MEMPOOL_NAME,
> SECURITY_TEST_MEMPOOL_SIZE,
> - SECURITY_TEST_SESSION_OBJECT_SIZE,
> + SECURITY_TEST_SESSION_OBJ_SZ,
> 0, 0, NULL, NULL, NULL, NULL,
> 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,
> + SECURITY_TEST_SESSION_PRIV_OBJ_SZ,
> + 0, 0, NULL, NULL, NULL, NULL,
> + SOCKET_ID_ANY, 0);
> + if (ts_params->session_priv_mpool == NULL) {
> + RTE_LOG(ERR, USER1, "TestCase %s() line %d failed (null): "
> + "Cannot create priv mempool %s\n",
> + __func__, __LINE__, rte_strerror(rte_errno));
> + rte_mempool_free(ts_params->session_mpool);
> + ts_params->session_mpool = NULL;
> + return TEST_FAILED;
> + }
> +
> return TEST_SUCCESS;
> }
>
> @@ -559,6 +609,10 @@ testsuite_teardown(void)
> rte_mempool_free(ts_params->session_mpool);
> ts_params->session_mpool = NULL;
> }
> + if (ts_params->session_priv_mpool) {
> + rte_mempool_free(ts_params->session_priv_mpool);
> + ts_params->session_priv_mpool = NULL;
> + }
> }
>
> /**
> @@ -656,10 +710,12 @@ ut_setup_with_session(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;
>
> 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,11 +757,13 @@ 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);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> @@ -725,11 +783,13 @@ 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);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> @@ -749,11 +809,13 @@ 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);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> @@ -770,18 +832,21 @@ 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);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> }
>
> /**
> - * Test execution of rte_security_session_create with NULL mp parameter
> + * Test execution of rte_security_session_create with NULL session
> + * mempool
> */
> static int
> test_session_create_inv_mempool(void)
> @@ -790,11 +855,35 @@ 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);
...NULL, ts_params->session_priv_mpool); would be better as it would
test if making primary mempool NULL is the cause of session_create failure.
> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> sess, NULL, "%p");
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> + TEST_ASSERT_SESSION_COUNT(0);
> +
> + return TEST_SUCCESS;
> +}
> +
> +/**
> + * Test execution of rte_security_session_create with NULL session
> + * priv mempool
> + */
> +static int
> +test_session_create_inv_sess_priv_mempool(void)
> +{
> + struct security_unittest_params *ut_params = &unittest_params;
> + struct security_testsuite_params *ts_params = &testsuite_params;
> + struct rte_security_session *sess;
> +
> + sess = rte_security_session_create(&ut_params->ctx, &ut_params->conf,
> + ts_params->session_mpool, NULL);
> + TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_create,
> + sess, NULL, "%p");
> + TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0);
> + TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> @@ -810,6 +899,7 @@ test_session_create_mempool_empty(void)
> struct security_testsuite_params *ts_params = &testsuite_params;
> struct security_unittest_params *ut_params = &unittest_params;
> struct rte_security_session *tmp[SECURITY_TEST_MEMPOOL_SIZE];
> + void *tmp1[SECURITY_TEST_MEMPOOL_SIZE];
> struct rte_security_session *sess;
>
> /* Get all available objects from mempool. */
> @@ -820,21 +910,34 @@ test_session_create_mempool_empty(void)
> TEST_ASSERT_EQUAL(0, ret,
> "Expect getting %d object from mempool"
> " to succeed", i);
> + ret = rte_mempool_get(ts_params->session_priv_mpool,
> + (void **)(&tmp1[i]));
> + TEST_ASSERT_EQUAL(0, ret,
> + "Expect getting %d object from priv mempool"
> + " to succeed", i);
> }
> TEST_ASSERT_MEMPOOL_USAGE(SECURITY_TEST_MEMPOOL_SIZE);
> + TEST_ASSERT_PRIV_MP_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);
> TEST_ASSERT_MEMPOOL_USAGE(SECURITY_TEST_MEMPOOL_SIZE);
> + TEST_ASSERT_PRIV_MP_USAGE(SECURITY_TEST_MEMPOOL_SIZE);
> TEST_ASSERT_SESSION_COUNT(0);
>
> /* Put objects back to the pool. */
> - for (i = 0; i < SECURITY_TEST_MEMPOOL_SIZE; ++i)
> - rte_mempool_put(ts_params->session_mpool, (void *)(tmp[i]));
> + for (i = 0; i < SECURITY_TEST_MEMPOOL_SIZE; ++i) {
> + rte_mempool_put(ts_params->session_mpool,
> + (void *)(tmp[i]));
> + rte_mempool_put(ts_params->session_priv_mpool,
> + (tmp1[i]));
> + }
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
>
> return TEST_SUCCESS;
> }
> @@ -853,14 +956,17 @@ 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);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> return TEST_SUCCESS;
> @@ -879,10 +985,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,
> @@ -891,6 +999,7 @@ test_session_create_success(void)
> sess, mock_session_create_exp.sess);
> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 1);
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> /*
> @@ -1276,6 +1385,7 @@ test_session_destroy_inv_context(void)
> struct security_unittest_params *ut_params = &unittest_params;
>
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(NULL, ut_params->sess);
> @@ -1283,6 +1393,7 @@ test_session_destroy_inv_context(void)
> ret, -EINVAL, "%d");
> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 0);
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> return TEST_SUCCESS;
> @@ -1299,6 +1410,7 @@ test_session_destroy_inv_context_ops(void)
> ut_params->ctx.ops = NULL;
>
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(&ut_params->ctx,
> @@ -1307,6 +1419,7 @@ test_session_destroy_inv_context_ops(void)
> ret, -EINVAL, "%d");
> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 0);
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> return TEST_SUCCESS;
> @@ -1323,6 +1436,7 @@ test_session_destroy_inv_context_ops_fun(void)
> ut_params->ctx.ops = &empty_ops;
>
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(&ut_params->ctx,
> @@ -1331,6 +1445,7 @@ test_session_destroy_inv_context_ops_fun(void)
> ret, -ENOTSUP, "%d");
> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 0);
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> return TEST_SUCCESS;
> @@ -1345,6 +1460,7 @@ test_session_destroy_inv_session(void)
> struct security_unittest_params *ut_params = &unittest_params;
>
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(&ut_params->ctx, NULL);
> @@ -1352,6 +1468,7 @@ test_session_destroy_inv_session(void)
> ret, -EINVAL, "%d");
> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 0);
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> return TEST_SUCCESS;
> @@ -1371,6 +1488,7 @@ test_session_destroy_ops_failure(void)
> mock_session_destroy_exp.ret = -1;
>
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(&ut_params->ctx,
You can add also:
TEST_ASSERT_PRIV_MP_USAGE(1);
in line 1500 after rte_security_session_destroy() returned to verify
that private mempool usage stays on same level after failure of destroy op.
Currently adding it without fixing mock of session_destroy op, will
cause test failure:
EAL: Test assert test_session_destroy_ops_failure line 1500 failed:
Expecting 1 priv mempool allocations, but there are 0 allocated objects
EAL: in ../app/test/test_security.c:1500 test_session_destroy_ops_failure
> @@ -1396,6 +1514,7 @@ test_session_destroy_success(void)
> mock_session_destroy_exp.sess = ut_params->sess;
> mock_session_destroy_exp.ret = 0;
> TEST_ASSERT_MEMPOOL_USAGE(1);
> + TEST_ASSERT_PRIV_MP_USAGE(1);
> TEST_ASSERT_SESSION_COUNT(1);
>
> int ret = rte_security_session_destroy(&ut_params->ctx,
> @@ -1404,6 +1523,7 @@ test_session_destroy_success(void)
> ret, 0, "%d");
> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 1);
> TEST_ASSERT_MEMPOOL_USAGE(0);
> + TEST_ASSERT_PRIV_MP_USAGE(0);
> TEST_ASSERT_SESSION_COUNT(0);
>
> /*
> @@ -2370,6 +2490,8 @@ static struct unit_test_suite security_testsuite = {
> test_session_create_inv_configuration),
> TEST_CASE_ST(ut_setup, ut_teardown,
> test_session_create_inv_mempool),
> + TEST_CASE_ST(ut_setup, ut_teardown,
> + test_session_create_inv_sess_priv_mempool),
> TEST_CASE_ST(ut_setup, ut_teardown,
> test_session_create_mempool_empty),
> TEST_CASE_ST(ut_setup, ut_teardown,
> diff --git a/doc/guides/prog_guide/rte_security.rst
> b/doc/guides/prog_guide/rte_security.rst
> index 127da2e4f..d30a79576 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -533,8 +533,12 @@ 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 private session data 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``.
> +And the session mempool object size should be enough to accommodate
> +``rte_security_session``.
>
> 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 43cdd3c58..26be1b3de 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -164,13 +164,6 @@ Deprecation Notices
> following the IPv6 header, as proposed in RFC
>
> https://protect2.fireeye.com/v1/url?k=0ff8f153-529fb575-0ff97a1c-0cc47a31384a-da56d065c0f960ba&q=1&e=4b8cafbf-ec0f-4a52-9c77-e1c5a4efcfc5&u=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2020-August%2F177257.html.
>
> -* 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: support for using IV with all sizes is added, J0 still can
> be used but only when IV length in following structs
> ``rte_crypto_auth_xform``,
> ``rte_crypto_aead_xform`` is set to zero. When IV length is greater or
> equal
> diff --git a/doc/guides/rel_notes/release_20_11.rst
> b/doc/guides/rel_notes/release_20_11.rst
> index f1b9b4dfe..0fb1b20cb 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -344,6 +344,12 @@ API Changes
> * The structure ``rte_crypto_sym_vec`` is updated to support both
> cpu_crypto synchrounous operation and asynchronous raw data-path APIs.
>
> +* 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 60132c4bd..2326089bb 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -2348,12 +2348,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);
> @@ -2376,12 +2372,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..ee4666026 100644
> --- a/lib/librte_security/rte_security.c
> +++ b/lib/librte_security/rte_security.c
> @@ -26,18 +26,21 @@
> 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;
>
> RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, session_create, NULL, NULL);
> RTE_PTR_OR_ERR_RET(conf, NULL);
> RTE_PTR_OR_ERR_RET(mp, NULL);
> + RTE_PTR_OR_ERR_RET(priv_mp, NULL);
>
> 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
--
Lukasz Wojciechowski
Principal Software Engineer
Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciec...@partner.samsung.com
