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.wojciec...@partner.samsung.com