Just to be clear, these are the changes that will fix v4:
diff --git a/app/test/test_security.c b/app/test/test_security.c
index 62e4991eb..060cf1ffa 100644
--- a/app/test/test_security.c
+++ b/app/test/test_security.c
@@ -395,7 +395,7 @@ mock_session_destroy(void *device, struct
rte_security_session *sess)
void *sess_priv = get_sec_session_private_data(sess);
mock_session_destroy_exp.called++;
- if ((mock_session_create_exp.ret == 0) && (sess_priv != NULL)) {
+ 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);
}
@@ -1501,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;
Best regards
Lukasz
W dniu 18.10.2020 o 11:30, Lukasz Wojciechowski pisze:
> Hi Akhil,
>
> If you replace mock_session_create_exp.ret with mock_session_destroy_exp.ret
> in mock_session_destroy() everything works fine and all test cases pass.
>
> W dniu 18.10.2020 o 10:47, Akhil Goyal pisze:
>> Hi Lukasz,
>>> Hi Akhil,
>>>
>>> just one more thing, rest looks good
>>>
>>> <snip>
>>>> diff --git a/app/test/test_security.c b/app/test/test_security.c
>>>> index 77fd5adc6..62e4991eb 100644
>>>> --- a/app/test/test_security.c
>>>> +++ b/app/test/test_security.c
>>>> @@ -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_create_exp.ret == 0) && (sess_priv != NULL)) {
>>> mock_session_destroy_exp.ret
> Here is the place you need to change. Sorry that my comment was not
> clear enough.
>>>> + 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_ex
>>> p, device);
>>> MOCK_TEST_ASSERT_POINTER_PARAMETER(mock_session_destroy_ex
>>> p, sess);
>>> <snip>
>>>> @@ -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,
>>>> @@ -1396,6 +1518,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,
>>>> ut_params->sess);
>>>>
>>> TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_session_destr
>>> oy,
>>>> ret, -1, "%d");
>>>> TEST_ASSERT_MOCK_CALLS(mock_session_destroy_exp, 1);
>>>> TEST_ASSERT_MEMPOOL_USAGE(1);
>>> nit: you can add and assertion here as well: TEST_ASSERT_PRIV_MP_USAGE(1);
>> Thanks for the review of the patch.
>>
>> This is causing the test to fail. And I cannot spend more time in debugging
>> this right now.
>> Since we are approaching RC1 deadline and the intent of the patch is to add
>> a new
>> Parameter in session create API and which is complete. The issue is with the
>> security
>> Tests which are trying to create mock driver APIs with no real usage.
>>
>> Hence, I reckon we can go ahead with this patch in RC1 and fix it later.
>>
>>
>> @tho...@monjalon.net, @ano...@marvell.com, @Ananyev, Konstantin.
>> Any thoughts from your side?
>>
>> 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
