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 754A7A04DB; Thu, 15 Oct 2020 03:14:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5017A1BCD5; Thu, 15 Oct 2020 03:14:47 +0200 (CEST) Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id BFBF11BCC2 for ; Thu, 15 Oct 2020 03:14:44 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20201015011433euoutp014198fccead89607b5e8c40df0d96d3a6~_BWqqjeLl2537825378euoutp01r for ; Thu, 15 Oct 2020 01:14:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20201015011433euoutp014198fccead89607b5e8c40df0d96d3a6~_BWqqjeLl2537825378euoutp01r DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1602724473; bh=XwRe58vVBSUQvf0BahkqGYV85l87DyS7OMbmNLjUkqw=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=o8xdUp7n0IuTzjkcNNgVd3O9N3YGQjHaWKVygJAWzbGjYc5UG0Vz4SFUjsjhqLPDM NQVkwc60Wa8yjz2hgoScfFS/Mns+lquQbSRVcR1n1OgW66IyRVIH9tE/+vrQGnvk1b Yk4zLjGbntGGoH5dw6ykGOO64mEkcqLx5/DcUIh8= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20201015011427eucas1p26d2b8cee153f6c98ec72a08a2b8e5f8d~_BWlKKkpr1425214252eucas1p2K; Thu, 15 Oct 2020 01:14:27 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id A3.9C.05997.372A78F5; Thu, 15 Oct 2020 02:14:27 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20201015011426eucas1p1ad480d7c931286ee55b568ab22466a9d~_BWk3XLwn1184811848eucas1p1S; Thu, 15 Oct 2020 01:14:26 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20201015011426eusmtrp1502a579032bfc691726df7aaa2b1d306~_BWk2uTf10260502605eusmtrp18; Thu, 15 Oct 2020 01:14:26 +0000 (GMT) X-AuditID: cbfec7f4-65dff7000000176d-5a-5f87a273ce0b Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id D6.7E.06314.272A78F5; Thu, 15 Oct 2020 02:14:26 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20201015011425eusmtip2a0acec6ebe02bba876945c2ae5d1185f~_BWj7_OSf1901119011eusmtip2I; Thu, 15 Oct 2020 01:14:25 +0000 (GMT) To: Akhil Goyal , "dev@dpdk.org" Cc: "thomas@monjalon.net" , "mdr@ashroe.eu" , "anoobj@marvell.com" , Hemant Agrawal , "konstantin.ananyev@intel.com" , "declan.doherty@intel.com" , "radu.nicolau@intel.com" , "david.coyle@intel.com" , "\"'Lukasz Wojciechowski'\"," From: Lukasz Wojciechowski Message-ID: <2be7a00e-2033-fb46-5696-a60a0ff9317d@partner.samsung.com> Date: Thu, 15 Oct 2020 03:14:24 +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: Content-Transfer-Encoding: 8bit Content-Language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA02SbUhTcRTG++/ebdfV6t+W7aCFMBAqUFMjLizTYsStCAv6ZJitvKnpluw6 Tb/04vB1pU0qW1Tq0Ja9yHxLzF6cpemmEfRiQWaklSuNLA2dSF5vkt9+5znP4TkHDkUousQB VIohgzUadGlqiYxs7px6HsJV5SdsdBYTdJ3nGqJrGptEdMGrXER/HzxD0mPj90T0zU9OCf1j poqkP1vuIrqk6SlJ5xVhenzwGRmzlLnRVyJmpiuqxYy9bUTElFW+IJjOtxeljHOsRcS0j7WJ 9krjZFsS2bSUTNYYtvWQLHm2ITq9OORE/ps66Sl0MbgI+VGAN0F53QcpzwrsQND5OaoIyeb4 N4LK3NuE0PiFoP3niYUBS0+tVDDdQOCq7kZCMYrgT22xhHcpsQZe50+KeV6FtWB+d0fMmwjs IWC6/CviGxIcBU8uT8yb5HgH+EoHSJ5JHAyDLRfm2R8fhFKnmRQ8K6H78tAcU5QfjofJj3Je JnAQ5DZdIQRWwbuh6yI+C/CAFAqrOwlhbS2M15+RCKwEb1ejVOA14C6zkMJAM4JXvikkFI8Q vDnr+OfSQMesT8InE3g91LWGCfI2aC//gngZ8HLoH10pLLEcrM2XCEGWQ0GeQnCHwrDlAlqI nbkzRJYitW3RZbZF59gWnWP7n1uByFqkYk2cPonlIgxsViin03MmQ1LokeP6ejT3Xu7Zrt8t qHXmsAthCqmXyZf48hIUYl0ml613IaAI9Sr59l73QYU8UZedwxqPJxhNaSznQoEUqVbJI6tG 4hU4SZfBprJsOmtc6Ioov4BTKGun8sdtu8ehUG3Wni8sC9e8RJf8lSetx+w560MaDqCj+3cW uhV7I82Pg8IzPd611ppbD0fs0ZrVjZbHp3tjPPq1D/ree5MDx+IbGcc6TX3b1R5v6nXtjuF9 tnOVE7v2nP2WjvdVOK2mKUfDbsdEbExcf0ZHhOr+kZTNsSSzwqwmuWRd+AbCyOn+Aun4Yiha AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrFIsWRmVeSWpSXmKPExsVy+t/xe7pFi9rjDQ7dNrBYf2Yeo8WyLVuZ LDquNjNavHnQxGLx7tN2JouVjzeyWbz/s4jF4lnPOkaL/q1HWSzaugQsPj04weLA7bH8XD+r x68FS1k9Fu95yeQxeeFFZo9jN6exe2x8t4PJ4+C7PUwB7FF6NkX5pSWpChn5xSW2StGGFkZ6 hpYWekYmlnqGxuaxVkamSvp2NimpOZllqUX6dgl6Gf822xd061a0X1/P3sA4TbWLkZNDQsBE oufUKvYuRi4OIYGljBJT9x9n62LkAErISHy4JABRIyzx51oXG0TNa0aJ/f+62UASwgLWEtfa v7GC2CICLhItt9ayghQxC5xjlnj27RYjRMcpJolZK36xgFSxCdhKHJn5FayDV8BN4veEe2Bx FgFViQc7poLZogJxEj8m9rJB1AhKnJz5hAXkIk6BWIlvD3lBwswCZhLzNj9khrDlJZq3zoay xSVuPZnPNIFRaBaS7llIWmYhaZmFpGUBI8sqRpHU0uLc9NxiQ73ixNzi0rx0veT83E2MwHjd duzn5h2MlzYGH2IU4GBU4uFl+N0WL8SaWFZcmXuIUYKDWUmE1+ns6Tgh3pTEyqrUovz4otKc 1OJDjKZAv01klhJNzgemkrySeENTQ3MLS0NzY3NjMwslcd4OgYMxQgLpiSWp2ampBalFMH1M HJxSDYwt3LJ5N1b8XS6clZDNHbRJJ3/p8Uce134rvTRynlMezhIqzHx3wywTh8IOgfSzar+P FvCWzD1Yslfxx3Gh64vNm1le5xUvYEyx1FwmYJLlntUuV9Yx80F2+Z17xmmHBc7sYuVUPezZ e1prVqD+WZlH07a3+aa45qdd7d9mbvnqSeKeg+W5vUosxRmJhlrMRcWJAI/YBUPtAgAA X-CMS-MailID: 20201015011426eucas1p1ad480d7c931286ee55b568ab22466a9d X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20201010221145eucas1p12bd7152a21d291cc91a97fb7849e33d0 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20201010221145eucas1p12bd7152a21d291cc91a97fb7849e33d0 References: <20200903200958.28025-1-akhil.goyal@nxp.com> <20201010221124.6937-1-akhil.goyal@nxp.com> <2b1e8aed-e4fe-a9fe-98b7-e40755b04e94@partner.samsung.com> Subject: Re: [dpdk-dev] [PATCH v2] 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" W dniu 14.10.2020 o 21:00, Akhil Goyal pisze: > Hi Lukasz, > >> Hi Akhil, >>> -#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) >>> @@ -545,6 +548,22 @@ testsuite_setup(void) >>> SOCKET_ID_ANY, 0); >>> TEST_ASSERT_NOT_NULL(ts_params->session_mpool, >>> "Cannot create mempool %s\n", >> rte_strerror(rte_errno)); >>> + >>> + ts_params->session_priv_mpool = rte_mempool_create( >>> + SECURITY_TEST_PRIV_MEMPOOL_NAME, >>> + SECURITY_TEST_MEMPOOL_SIZE, >>> + rte_security_session_get_size(&unittest_params.ctx), >> Call to rte_security_session_get_size() will cause a mockup function >> mock_session_get_size() to be called, which will return 0. >> Why do you call this function instead of defining some value for private >> mempool element size? > Fixed in v3 > >>> + 0, 0, NULL, NULL, NULL, NULL, >>> + SOCKET_ID_ANY, 0); >>> + if (ts_params->session_priv_mpool == NULL) { >>> + printf("TestCase %s() line %d failed (null): " >>> + "Cannot create priv mempool %s\n", >>> + __func__, __LINE__, rte_strerror(rte_errno)); >> Instead of printf() use RTE_LOG(ERR, EAL,...). All other messages are >> printed this way. It allows control of error messages if required. > Fixed in v3, should be USER1 instead of EAL though. Can you explain me why, there should be USER1? All other errors are printed with EAL tag. > >>> + rte_mempool_free(ts_params->session_mpool); >>> + ts_params->session_mpool = NULL; >>> + return TEST_FAILED; >>> + } >>> + >>> return TEST_SUCCESS; >>> } >>> >>> @@ -559,6 +578,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; >>> + } >>> } >>> >>> /** >>> @@ -659,7 +682,8 @@ ut_setup_with_session(void) >>> mock_session_create_exp.ret = 0; >>> >>> sess = rte_security_session_create(&ut_params->ctx, &ut_params- >>> conf, >>> - ts_params->session_mpool); >>> + ts_params->session_mpool, >>> + ts_params->session_priv_mpool); >>> >> TEST_ASSERT_MOCK_FUNCTION_CALL_NOT_NULL(rte_security_sessio >> n_create, >>> sess); >>> TEST_ASSERT_EQUAL(sess, mock_session_create_exp.sess, >>> @@ -701,7 +725,8 @@ test_session_create_inv_context(void) >>> struct rte_security_session *sess; >>> >>> sess = rte_security_session_create(NULL, &ut_params->conf, >>> - ts_params->session_mpool); >>> + ts_params->session_mpool, >>> + ts_params->session_priv_mpool); >>> >> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_creat >> e, >>> sess, NULL, "%p"); >>> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0); >>> @@ -725,7 +750,8 @@ test_session_create_inv_context_ops(void) >>> ut_params->ctx.ops = NULL; >>> >>> sess = rte_security_session_create(&ut_params->ctx, &ut_params- >>> conf, >>> - ts_params->session_mpool); >>> + ts_params->session_mpool, >>> + ts_params->session_priv_mpool); >>> >> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_creat >> e, >>> sess, NULL, "%p"); >>> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0); >>> @@ -749,7 +775,8 @@ test_session_create_inv_context_ops_fun(void) >>> ut_params->ctx.ops = &empty_ops; >>> >>> sess = rte_security_session_create(&ut_params->ctx, &ut_params- >>> conf, >>> - ts_params->session_mpool); >>> + ts_params->session_mpool, >>> + ts_params->session_priv_mpool); >>> >> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_creat >> e, >>> sess, NULL, "%p"); >>> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0); >>> @@ -770,7 +797,8 @@ test_session_create_inv_configuration(void) >>> struct rte_security_session *sess; >>> >>> sess = rte_security_session_create(&ut_params->ctx, NULL, >>> - ts_params->session_mpool); >>> + ts_params->session_mpool, >>> + ts_params->session_priv_mpool); >>> >> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_creat >> e, >>> sess, NULL, "%p"); >>> TEST_ASSERT_MOCK_CALLS(mock_session_create_exp, 0); >>> @@ -781,7 +809,7 @@ test_session_create_inv_configuration(void) >>> } >>> >>> /** >>> - * Test execution of rte_security_session_create with NULL mp parameter >>> + * Test execution of rte_security_session_create with NULL mempools >>> */ >>> static int >>> test_session_create_inv_mempool(void) >>> @@ -790,7 +818,7 @@ test_session_create_inv_mempool(void) >>> struct rte_security_session *sess; >>> >>> sess = rte_security_session_create(&ut_params->ctx, &ut_params- >>> conf, >>> - NULL); >>> + NULL, NULL); >> It would be best to add a new testcase for verification of passing NULL >> private mempool. >> If you pass NULL as the primary mempool as in this testcase, the >> verification of priv mempool (rte_securitry.c:37) won't ever happen >> because rte_security_session_create() will return in line 36. > Added a new test. However that was really unnecessary and was an overkill > To add a new case for so many negative cases. > > Please have a look at v3 and ack it if no further comments. > > Regards, > Akhil > -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com