Re: [dpdk-dev] [PATCH] security: update session create API

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Lukasz,

Thanks for the review.

> > diff --git a/app/test/test_security.c b/app/test/test_security.c
> > index 77fd5adc6..ed7de348f 100644
> > --- a/app/test/test_security.c
> > +++ b/app/test/test_security.c
> > @@ -237,6 +237,7 @@ 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;
> >
> session_create op is now called with private mbuf, so you need also to
> update assert in mock session_create:

OK will be fixed in v2

> 
> @@ -248,13 +249,13 @@ 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)
>   {
>          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);
> 
>          mock_session_create_exp.sess = sess;
> 
> 
> 
> > @@ -502,6 +503,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 };
> >
> >   /**
> > @@ -525,6 +527,7 @@ static struct security_unittest_params {
> >   };
> >
> >   #define SECURITY_TEST_MEMPOOL_NAME "SecurityTestsMempoolName"
> > +#define SECURITY_TEST_PRIV_MEMPOOL_NAME
> "SecurityTestsPrivMempoolName"
> Please make the mempool name shorter, otherwise it causes tests to fail:
> 
> EAL: Test assert testsuite_setup line 558 failed: Cannot create priv
> mempool File name too long
> 

Fixed in v2

> >   #define SECURITY_TEST_MEMPOOL_SIZE 15
> >   #define SECURITY_TEST_SESSION_OBJECT_SIZE sizeof(struct
> rte_security_session)
> >
> > @@ -545,6 +548,17 @@ 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),
> > +			0, 0, NULL, NULL, NULL, NULL,
> > +			SOCKET_ID_ANY, 0);
> > +	TEST_ASSERT_NOT_NULL(ts_params->session_priv_mpool,
> > +			"Cannot create priv mempool %s\n",
> > +			rte_strerror(rte_errno));
> > +
> If creation of private data mpool fails, primary mempool need to be
> freed before function returns failure code.
This is an issue in whole of the file.
However, have fixed it in v2 for this particular case in v2.