DPDK patches and discussions
 help / color / Atom feed
From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, dev@dpdk.org
Cc: thomas@monjalon.net, mdr@ashroe.eu, anoobj@marvell.com,
	hemant.agrawal@nxp.com, konstantin.ananyev@intel.com,
	declan.doherty@intel.com, radu.nicolau@intel.com,
	david.coyle@intel.com,
	"\"'Lukasz Wojciechowski'\"," <l.wojciechow@partner.samsung.com>
Subject: Re: [dpdk-dev] [PATCH v5] security: update session create API
Date: Sun, 18 Oct 2020 13:03:06 +0200
Message-ID: <857b9ccb-48fe-5653-6396-7ad2ec10d3db@partner.samsung.com> (raw)
In-Reply-To: <20201018094041.3848-1-akhil.goyal@nxp.com>

Hi Akhil,
Thanks for your patience.

I reviewed all files and run tests. IMO everything is OK now.

W dniu 18.10.2020 o 11:40, 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.goyal@nxp.com>
Reviewed-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
Tested-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
> Changes in v5:
> fixed security test.
>
> Changes in v4:
> rebase on TOT
> addressed comments from Lukasz on v3
>
> 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               | 167 ++++++++++++++++++++++---
>   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, 202 insertions(+), 55 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 1d4c46f08..3afc1d809 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c
> @@ -782,9 +782,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) {
> @@ -7762,7 +7768,8 @@ static int test_pdcp_proto(int i, int oop, enum rte_crypto_cipher_operation opc,
>   
>   	/* 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: ",
> @@ -8022,7 +8029,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: ",
> @@ -8488,6 +8496,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) {
> @@ -8663,6 +8672,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..060cf1ffa 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, "	\
> +			"but there are %u allocated objects",		\
> +			expected_priv_mp_usage, priv_mp_usage);		\
> +} while (0)
>   
>   /**
>    * Mockup structures and functions for rte_security_ops;
> @@ -237,26 +255,37 @@ 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);
> +
> +	if (mock_session_create_exp.ret == 0) {
> +		ret = rte_mempool_get(priv_mp, &sess_priv);
> +		TEST_ASSERT_EQUAL(0, ret,
> +			"priv mempool does not have enough objects");
>   
> -	mock_session_create_exp.sess = sess;
> +		set_sec_session_private_data(sess, sess_priv);
> +		mock_session_create_exp.sess = sess;
> +	}
>   
>   	return mock_session_create_exp.ret;
>   }
> @@ -363,8 +392,13 @@ 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++;
> +	if ((mock_session_destroy_exp.ret == 0) && (sess_priv != NULL)) {
> +		rte_mempool_put(rte_mempool_from_obj(sess_priv), sess_priv);
> +		set_sec_session_private_data(sess, NULL);
> +	}
>   	MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_destroy_exp, device);
>   	MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_destroy_exp, sess);
>   
> @@ -502,6 +536,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 +559,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 +577,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 +612,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 +713,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 +760,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 +786,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 +812,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,31 +835,59 @@ 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)
>   {
>   	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,
> -			NULL);
> +			NULL, 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 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 +903,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 +914,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 +960,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 +989,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 +1003,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 +1389,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 +1397,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 +1414,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 +1423,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 +1440,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 +1449,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 +1464,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 +1472,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 +1492,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,
> @@ -1379,6 +1501,7 @@ test_session_destroy_ops_failure(void)
>   			ret, -1, "%d");
>   	TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 1);
>   	TEST_ASSERT_MEMPOOL_USAGE(1);
> +	TEST_ASSERT_PRIV_MP_USAGE(1);
>   	TEST_ASSERT_SESSION_COUNT(1);
>   
>   	return TEST_SUCCESS;
> @@ -1396,6 +1519,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 +1528,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 +2495,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 41c95aa52..c64aef3de 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 6cbfd1184..000e82cce 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=c01a96eb-9dc88db3-c01b1da4-0cc47a31c8b4-b59bae941a26f568&q=1&e=bb91da3e-ca68-49fb-9578-d771844b2f86&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 48717ee53..03ef28b68 100644
> --- a/doc/guides/rel_notes/release_20_11.rst
> +++ b/doc/guides/rel_notes/release_20_11.rst
> @@ -404,6 +404,12 @@ API Changes
>     ``uint32_t`` to ``uint8_t`` so that a new field ``sdap_enabled`` can be added
>     to support SDAP.
>   
> +* 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.
> +
>   * ipsec: ``RTE_SATP_LOG2_NUM`` has been dropped from ``enum`` and
>     subsequently moved ``rte_ipsec`` lib from experimental to stable.
>   
> 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 c259b35e0..271531af1 100644
> --- a/lib/librte_security/rte_security.h
> +++ b/lib/librte_security/rte_security.h
> @@ -394,6 +394,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
> @@ -401,7 +402,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.wojciechow@partner.samsung.com


  reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200903201022eucas1p19a5154b9fb7063f72b18dea1114eeed3@eucas1p1.samsung.com>
2020-09-03 20:09 ` [dpdk-dev] [PATCH] " akhil.goyal
2020-09-04 16:04   ` Lukasz Wojciechowski
2020-10-10 22:09     ` Akhil Goyal
2020-09-24 16:22   ` Coyle, David
2020-10-10 22:06     ` Akhil Goyal
2020-10-10 22:11   ` [dpdk-dev] [PATCH v2] " Akhil Goyal
2020-10-12 17:46     ` Akhil Goyal
2020-10-13  2:12     ` Lukasz Wojciechowski
2020-10-14 19:00       ` Akhil Goyal
2020-10-15  1:14         ` Lukasz Wojciechowski
2020-10-14 18:56     ` [dpdk-dev] [PATCH v3] " Akhil Goyal
2020-10-15  1:11       ` Lukasz Wojciechowski
2020-10-17 11:50       ` [dpdk-dev] [PATCH v4] " Akhil Goyal
2020-10-17 13:13         ` Lukasz Wojciechowski
2020-10-18  8:47           ` Akhil Goyal
2020-10-18  9:30             ` Lukasz Wojciechowski
2020-10-18  9:37               ` Lukasz Wojciechowski
2020-10-18  9:42               ` Akhil Goyal
2020-10-18  9:40         ` [dpdk-dev] [PATCH v5] " Akhil Goyal
2020-10-18 11:03           ` Lukasz Wojciechowski [this message]
2020-10-19  7:51             ` Thomas Monjalon

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=857b9ccb-48fe-5653-6396-7ad2ec10d3db@partner.samsung.com \
    --to=l.wojciechow@partner.samsung.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=david.coyle@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=radu.nicolau@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox