From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 060B5A04DB; Thu, 15 Oct 2020 03:11:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 858C51DB4F; Thu, 15 Oct 2020 03:11:49 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 7D1B11DB4D for ; Thu, 15 Oct 2020 03:11:47 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20201015011135euoutp023160ace82847c21585808aeb6b9d423c~_BUFg41KS2372823728euoutp02b for ; Thu, 15 Oct 2020 01:11:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20201015011135euoutp023160ace82847c21585808aeb6b9d423c~_BUFg41KS2372823728euoutp02b DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1602724295; bh=fW4INFEOl3Rt3ieR0+3uAchieK18cEfogU3hSXCRPrs=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=Tw9Cl1T0x/HPtacAgh1KaWWhn4LsJsggyKfYhyqLwpo6BcHpuU6oQ/KpTfQ7m0Txh Qkro+wqCxTE19NueWXm6rg/UIqk/itdTljcjyKn4uedeIldg/uu/6iBFB0GF1GDr4H LSjMaAwCt6M4+04gg8zu/TQ/qHhtUtEGC+Qm0avc= Received: from eusmges1new.samsung.com (unknown [203.254.199.242]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20201015011130eucas1p1a94c00783bdc45b01b1031b572d1b6e2~_BUAsShce1181411814eucas1p1p; Thu, 15 Oct 2020 01:11:30 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges1new.samsung.com (EUCPMTA) with SMTP id 42.E7.06456.2C1A78F5; Thu, 15 Oct 2020 02:11:30 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20201015011129eucas1p2f7e6430824bfdf853890100a089ec05e~_BT-hM1YJ2357723577eucas1p2w; Thu, 15 Oct 2020 01:11:29 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20201015011129eusmtrp119c473f38ec65be2095d15acb1438010~_BT-gfcxu0260502605eusmtrp1t; Thu, 15 Oct 2020 01:11:29 +0000 (GMT) X-AuditID: cbfec7f2-809ff70000001938-14-5f87a1c2217a Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 14.7E.06314.1C1A78F5; Thu, 15 Oct 2020 02:11:29 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20201015011127eusmtip1478fb97ce18eb09e2747446cdb209bd2~_BT_Sux5o0176801768eusmtip1d; Thu, 15 Oct 2020 01:11:27 +0000 (GMT) To: Akhil Goyal , 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'\"," From: Lukasz Wojciechowski Message-ID: <9b76ef22-fb3b-f622-59be-e224d9604e35@partner.samsung.com> Date: Thu, 15 Oct 2020 03:11:26 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: <20201014185651.31924-1-akhil.goyal@nxp.com> Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA01SfyyUcRjf933fu3ups6+jeYY0t2xpnNDa21BprV3/6U9t6OIN4XCHoq2J /OwaMoRVfixklRgXpqwjuk7Epi7ZMkecH2fza0RWdy/Lf5/n83ye5/N8tocmRVqeIx0tT2IV clmsmG9Nqfs2hzw11TlhJ2pynJimgSeIqWttI5jc0UzELExkUIxp+Q3BPDc085ml7RqK+aV6 hZiCtg8Uk52PmeWJj9S5A9L6wQKe9HfVM560tstISIurh0lp3/dSgbTZ1E5I35u6iCDBFWv/ CDY2OoVVeJ25ah2VVfQTJTTmEbcmdfcF6eiHHuUjKxrwSZgp+sLPR9a0CDcg6FSreVyximB4 oUVgVonwCoKSat+9ic22dYIT1SNYmtETnGgRQcsnv3xE03bYD941njXT9tgXXneWW5aS+C4B hqw5nrnBxwHQW75mwUJ8Ef6UDlrMKOwGI4uVlvMO4VAobL5HcRpb0JZPUeb9Vvg01Kk8zTSJ j0BmWyXJYQcYm3pquQ3wpABmxsd2Y14Ag6aA4LAdzPW3CjjsDLpiFcUNqBGMbm0iruhG8O1B w67KD3p2tvhmZxK7Q1OnF0cHwgtDh4UGbAP6RVvuCBt4qC4jOVoIudkiTi2BaVUJ2rPdfjlF FSJxxb5kFfviVOyLU/HftwpRjciBTVbGRbJKbzl7U6KUxSmT5ZGS8Pi4FvTvwXQ7/cvtaG3k mgZhGokPChs2ssNEPFmKMjVOg4AmxfbC8591oSJhhCw1jVXEhymSY1mlBjnRlNhB6FtjDBHh SFkSG8OyCaxir0vQVo7pyCO9ptPl8mOBx9Hegbrr/vwbqjCXzbJAh0jdnfmOwWMh9k7INVHu 0631M2p9qk4ZwwNqbb206p7iGMPQRrBk8bDT2NpI3iWT6+3QNEfk7pZSEJS3MmhcSEywk62t fjVJ9NPjOZAwOz376Khz9tvijN7g2G29LT2vL2yud1lvElPKKJn3cVKhlP0FGPq3IlwDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNIsWRmVeSWpSXmKPExsVy+t/xu7oHF7bHG7y7wW+x/sw8RotlW7Yy WXRcbWa0ePOgicXi3aftTBYrH29ks3j/ZxGLxbOedYwW/VuPsli0dQlYfHpwgsWB22P5uX5W j18LlrJ6LN7zkslj8sKLzB7Hbk5j99j4bgeTx8F3e5gC2KP0bIryS0tSFTLyi0tslaINLYz0 DC0t9IxMLPUMjc1jrYxMlfTtbFJSczLLUov07RL0Mlon3mcsWNXJVPHodDd7A+PtG4xdjJwc EgImEj+3fmMCsYUEljJKfJgn38XIARSXkfhwSQCiRFjiz7Uuti5GLqCS14wS/esfs4HUCAtY S+xbZQ9SIyJgLLFh10xWkBpmgUYmicZDRxkhGnYxSjy+sQpsGZuArcSRmV9ZQWxeATeJv9PO sYPYLAKqEpfezgarERWIk/gxsZcNokZQ4uTMJywgyzgFLCWW9eiChJkFzCTmbX7IDGHLSzRv nQ1li0vcejKfaQKj0Cwk3bOQtMxC0jILScsCRpZVjCKppcW56bnFhnrFibnFpXnpesn5uZsY gRG77djPzTsYL20MPsQowMGoxMPL8LstXog1say4MvcQowQHs5IIr9PZ03FCvCmJlVWpRfnx RaU5qcWHGE2BfpvILCWanA9MJnkl8YamhuYWlobmxubGZhZK4rwdAgdjhATSE0tSs1NTC1KL YPqYODilGhinFore4POZ2ROjl+HJ/NZhvkqGeLpUkw3H84Ve/yzmbTAoeywsEhiV9EBXRrIg SXZq/aF3Uhk+Uib8S4/8X2Xplqro8N7LT+vqjMs8Z7emyqusvvxL+LfQY5lMSb0T/PdDWc88 etORsy9c4D/37Hfyjw9FvXo2YYW7zwWuVs2w67eM4/sbgpRYijMSDbWYi4oTAWjABtbuAgAA X-CMS-MailID: 20201015011129eucas1p2f7e6430824bfdf853890100a089ec05e X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20201014185722eucas1p211cf7e15d223502bd78dfd2ae2dd5892 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20201014185722eucas1p211cf7e15d223502bd78dfd2ae2dd5892 References: <20201010221124.6937-1-akhil.goyal@nxp.com> <20201014185651.31924-1-akhil.goyal@nxp.com> Subject: Re: [dpdk-dev] [PATCH v3] security: update session create API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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.wojciechow@partner.samsung.com