app/test: add test for mbuf with pinned external buffer

  This patch adds unit test for the mbufs allocated from
the special pool with pinned external data buffers.

The pinned buffer mbufs are tested in the same way as
regular ones with taking into account some specifics
of cloning/attaching.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 app/test/test_mbuf.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 150 insertions(+), 15 deletions(-)
  

verify_mbuf_check_panics() is left commented out to test the patch
(verify_mbuf_check_panics() fails on my setup), will be removed in v2.

With best regards, Slava

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Viacheslav Ovsiienko
> Sent: Friday, January 24, 2020 22:25
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; olivier.matz@6wind.com;
> stephen@networkplumber.org; thomas@mellanox.net
> Subject: [dpdk-dev] [PATCH] app/test: add test for mbuf with pinned external
> buffer
> 
> This patch adds unit test for the mbufs allocated from the special pool with
> pinned external data buffers.
> 
> The pinned buffer mbufs are tested in the same way as regular ones with
> taking into account some specifics of cloning/attaching.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  app/test/test_mbuf.c | 165
> ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 150 insertions(+), 15 deletions(-)
> 
> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index
> 61ecffc..ee2f2f0 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -310,8 +310,17 @@
>  	return -1;
>  }
> 
> +static uint16_t
> +testclone_refcnt_read(struct rte_mbuf *m) {
> +	return RTE_MBUF_HAS_PINNED_EXTBUF(m) ?
> +	       rte_mbuf_ext_refcnt_read(m->shinfo) :
> +	       rte_mbuf_refcnt_read(m);
> +}
> +
>  static int
> -testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool)
> +testclone_testupdate_testdetach(struct rte_mempool *pktmbuf_pool,
> +				struct rte_mempool *clone_pool)
>  {
>  	struct rte_mbuf *m = NULL;
>  	struct rte_mbuf *clone = NULL;
> @@ -331,7 +340,7 @@
>  	*data = MAGIC_DATA;
> 
>  	/* clone the allocated mbuf */
> -	clone = rte_pktmbuf_clone(m, pktmbuf_pool);
> +	clone = rte_pktmbuf_clone(m, clone_pool);
>  	if (clone == NULL)
>  		GOTO_FAIL("cannot clone data\n");
> 
> @@ -339,7 +348,7 @@
>  	if (*data != MAGIC_DATA)
>  		GOTO_FAIL("invalid data in clone\n");
> 
> -	if (rte_mbuf_refcnt_read(m) != 2)
> +	if (testclone_refcnt_read(m) != 2)
>  		GOTO_FAIL("invalid refcnt in m\n");
> 
>  	/* free the clone */
> @@ -358,7 +367,7 @@
>  	data = rte_pktmbuf_mtod(m->next, unaligned_uint32_t *);
>  	*data = MAGIC_DATA;
> 
> -	clone = rte_pktmbuf_clone(m, pktmbuf_pool);
> +	clone = rte_pktmbuf_clone(m, clone_pool);
>  	if (clone == NULL)
>  		GOTO_FAIL("cannot clone data\n");
> 
> @@ -370,15 +379,15 @@
>  	if (*data != MAGIC_DATA)
>  		GOTO_FAIL("invalid data in clone->next\n");
> 
> -	if (rte_mbuf_refcnt_read(m) != 2)
> +	if (testclone_refcnt_read(m) != 2)
>  		GOTO_FAIL("invalid refcnt in m\n");
> 
> -	if (rte_mbuf_refcnt_read(m->next) != 2)
> +	if (testclone_refcnt_read(m->next) != 2)
>  		GOTO_FAIL("invalid refcnt in m->next\n");
> 
>  	/* try to clone the clone */
> 
> -	clone2 = rte_pktmbuf_clone(clone, pktmbuf_pool);
> +	clone2 = rte_pktmbuf_clone(clone, clone_pool);
>  	if (clone2 == NULL)
>  		GOTO_FAIL("cannot clone the clone\n");
> 
> @@ -390,10 +399,10 @@
>  	if (*data != MAGIC_DATA)
>  		GOTO_FAIL("invalid data in clone2->next\n");
> 
> -	if (rte_mbuf_refcnt_read(m) != 3)
> +	if (testclone_refcnt_read(m) != 3)
>  		GOTO_FAIL("invalid refcnt in m\n");
> 
> -	if (rte_mbuf_refcnt_read(m->next) != 3)
> +	if (testclone_refcnt_read(m->next) != 3)
>  		GOTO_FAIL("invalid refcnt in m->next\n");
> 
>  	/* free mbuf */
> @@ -418,7 +427,8 @@
>  }
> 
>  static int
> -test_pktmbuf_copy(struct rte_mempool *pktmbuf_pool)
> +test_pktmbuf_copy(struct rte_mempool *pktmbuf_pool,
> +		  struct rte_mempool *clone_pool)
>  {
>  	struct rte_mbuf *m = NULL;
>  	struct rte_mbuf *copy = NULL;
> @@ -458,11 +468,14 @@
>  	copy = NULL;
> 
>  	/* same test with a cloned mbuf */
> -	clone = rte_pktmbuf_clone(m, pktmbuf_pool);
> +	clone = rte_pktmbuf_clone(m, clone_pool);
>  	if (clone == NULL)
>  		GOTO_FAIL("cannot clone data\n");
> 
> -	if (!RTE_MBUF_CLONED(clone))
> +	if ((!RTE_MBUF_HAS_PINNED_EXTBUF(m) &&
> +	     !RTE_MBUF_CLONED(clone)) ||
> +	    (RTE_MBUF_HAS_PINNED_EXTBUF(m) &&
> +	     !RTE_MBUF_HAS_EXTBUF(clone)))
>  		GOTO_FAIL("clone did not give a cloned mbuf\n");
> 
>  	copy = rte_pktmbuf_copy(clone, pktmbuf_pool, 0, UINT32_MAX); @@
> -1199,10 +1212,11 @@
>  	buf = rte_pktmbuf_alloc(pktmbuf_pool);
>  	if (buf == NULL)
>  		return -1;
> +	/*
>  	printf("Checking good mbuf initially\n");
>  	if (verify_mbuf_check_panics(buf) != -1)
>  		return -1;
> -
> +	*/
>  	printf("Now checking for error conditions\n");
> 
>  	if (verify_mbuf_check_panics(NULL)) {
> @@ -2411,6 +2425,120 @@ struct test_case {
>  	return -1;
>  }
> 
> +/*
> + * Test the mbuf pool with pinned external data buffers
> + *  - Allocate memory zone for external buffer
> + *  - Create the mbuf pool with pinned external buffer
> + *  - Check the created pool with relevant mbuf pool unit tests  */
> +static int test_pktmbuf_ext_pinned_buffer(struct rte_mempool *std_pool)
> +{
> +
> +	struct rte_pktmbuf_extmem ext_mem;
> +	struct rte_mempool *pinned_pool = NULL;
> +	const struct rte_memzone *mz = NULL;
> +
> +	printf("Test mbuf pool with external pinned data buffers\n");
> +
> +	/* Allocate memzone for the external data buffer */
> +	mz = rte_memzone_reserve("pinned_pool",
> +				 NB_MBUF * MBUF_DATA_SIZE,
> +				 SOCKET_ID_ANY,
> +				 RTE_MEMZONE_2MB |
> RTE_MEMZONE_SIZE_HINT_ONLY);
> +	if (mz == NULL)
> +		GOTO_FAIL("%s: Memzone allocation failed\n", __func__);
> +
> +	/* Create the mbuf pool with pinned external data buffer */
> +	ext_mem.buf_ptr = mz->addr;
> +	ext_mem.buf_iova = mz->iova;
> +	ext_mem.buf_len = mz->len;
> +	ext_mem.elt_size = MBUF_DATA_SIZE;
> +
> +	pinned_pool = rte_pktmbuf_pool_create_extbuf("test_pinned_pool",
> +				NB_MBUF, MEMPOOL_CACHE_SIZE, 0,
> +				MBUF_DATA_SIZE,	SOCKET_ID_ANY,
> +				&ext_mem, 1);
> +	if (pinned_pool == NULL)
> +		GOTO_FAIL("%s: Mbuf pool with pinned external"
> +			  " buffer creation failed\n", __func__);
> +	/* test multiple mbuf alloc */
> +	if (test_pktmbuf_pool(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_mbuf_pool(pinned) failed\n",
> +			  __func__);
> +
> +	/* do it another time to check that all mbufs were freed */
> +	if (test_pktmbuf_pool(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_mbuf_pool(pinned) failed (2)\n",
> +			  __func__);
> +
> +	/* test that the data pointer on a packet mbuf is set properly */
> +	if (test_pktmbuf_pool_ptr(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_pktmbuf_pool_ptr(pinned) failed\n",
> +			  __func__);
> +
> +	/* test data manipulation in mbuf with non-ascii data */
> +	if (test_pktmbuf_with_non_ascii_data(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_pktmbuf_with_non_ascii_data(pinned)"
> +			  " failed\n", __func__);
> +
> +	/* test free pktmbuf segment one by one */
> +	if (test_pktmbuf_free_segment(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_pktmbuf_free_segment(pinned)
> failed\n",
> +			  __func__);
> +
> +	if (testclone_testupdate_testdetach(pinned_pool, std_pool) < 0)
> +		GOTO_FAIL("%s: testclone_and_testupdate(pinned) failed\n",
> +			  __func__);
> +
> +	if (test_pktmbuf_copy(pinned_pool, std_pool) < 0)
> +		GOTO_FAIL("%s: test_pktmbuf_copy(pinned) failed\n",
> +			  __func__);
> +
> +	if (test_failing_mbuf_sanity_check(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_failing_mbuf_sanity_check(pinned)"
> +			  " failed\n", __func__);
> +
> +	if (test_mbuf_linearize_check(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_mbuf_linearize_check(pinned) failed\n",
> +			  __func__);
> +
> +	/* test for allocating a bulk of mbufs with various sizes */
> +	if (test_pktmbuf_alloc_bulk(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_rte_pktmbuf_alloc_bulk(pinned)
> failed\n",
> +			  __func__);
> +
> +	/* test for allocating a bulk of mbufs with various sizes */
> +	if (test_neg_pktmbuf_alloc_bulk(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_neg_rte_pktmbuf_alloc_bulk(pinned)"
> +			  " failed\n", __func__);
> +
> +	/* test to read mbuf packet */
> +	if (test_pktmbuf_read(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_rte_pktmbuf_read(pinned) failed\n",
> +			  __func__);
> +
> +	/* test to read mbuf packet from offset */
> +	if (test_pktmbuf_read_from_offset(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_rte_pktmbuf_read_from_offset(pinned)"
> +			  " failed\n", __func__);
> +
> +	/* test to read data from chain of mbufs with data segments */
> +	if (test_pktmbuf_read_from_chain(pinned_pool) < 0)
> +		GOTO_FAIL("%s: test_rte_pktmbuf_read_from_chain(pinned)"
> +			  " failed\n", __func__);
> +
> +	RTE_SET_USED(std_pool);
> +	rte_mempool_free(pinned_pool);
> +	rte_memzone_free(mz);
> +	return 0;
> +
> +fail:
> +	rte_mempool_free(pinned_pool);
> +	rte_memzone_free(mz);
> +	return -1;
> +}
> +
>  static int
>  test_mbuf_dyn(struct rte_mempool *pktmbuf_pool)  { @@ -2635,12
> +2763,12 @@ struct test_case {
>  		goto err;
>  	}
> 
> -	if (testclone_testupdate_testdetach(pktmbuf_pool) < 0) {
> +	if (testclone_testupdate_testdetach(pktmbuf_pool, pktmbuf_pool) <
> 0) {
>  		printf("testclone_and_testupdate() failed \n");
>  		goto err;
>  	}
> 
> -	if (test_pktmbuf_copy(pktmbuf_pool) < 0) {
> +	if (test_pktmbuf_copy(pktmbuf_pool, pktmbuf_pool) < 0) {
>  		printf("test_pktmbuf_copy() failed\n");
>  		goto err;
>  	}
> @@ -2731,6 +2859,13 @@ struct test_case {
>  		goto err;
>  	}
> 
> +	/* test the mbuf pool with pinned external data buffers */
> +	if (test_pktmbuf_ext_pinned_buffer(pktmbuf_pool) < 0) {
> +		printf("test_pktmbuf_ext_pinned_buffer() failed\n");
> +		goto err;
> +	}
> +
> +
>  	ret = 0;
>  err:
>  	rte_mempool_free(pktmbuf_pool);
> --
> 1.8.3.1
  

Hi,

On Fri, Jan 24, 2020 at 08:25:18PM +0000, Viacheslav Ovsiienko wrote:
> This patch adds unit test for the mbufs allocated from
> the special pool with pinned external data buffers.
> 
> The pinned buffer mbufs are tested in the same way as
> regular ones with taking into account some specifics
> of cloning/attaching.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Looks good to me, you can add my ack in the v2, once we understand
the issue with verify_mbuf_check_panics().

> @@ -1199,10 +1212,11 @@
>  	buf = rte_pktmbuf_alloc(pktmbuf_pool);
>  	if (buf == NULL)
>  		return -1;
> +	/*
>  	printf("Checking good mbuf initially\n");
>  	if (verify_mbuf_check_panics(buf) != -1)
>  		return -1;
> -
> +	*/
>  	printf("Now checking for error conditions\n");
>  
>  	if (verify_mbuf_check_panics(NULL)) {
> @@ -2411,6 +2425,120 @@ struct test_case {

Note: on my platform, it still works if I remove this comment.

Regards,
Olivier
  

Olivier, thanks for the reviewing.
I'll remove the comment and send the v2.
I use 1G huge pages, will retest over 2M
and continue finding why my host fails.

With best regards, Slava

> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, February 6, 2020 10:17
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; stephen@networkplumber.org;
> thomas@mellanox.net
> Subject: Re: [PATCH] app/test: add test for mbuf with pinned external buffer
> 
> Hi,
> 
> On Fri, Jan 24, 2020 at 08:25:18PM +0000, Viacheslav Ovsiienko wrote:
> > This patch adds unit test for the mbufs allocated from the special
> > pool with pinned external data buffers.
> >
> > The pinned buffer mbufs are tested in the same way as regular ones
> > with taking into account some specifics of cloning/attaching.
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> 
> Looks good to me, you can add my ack in the v2, once we understand the
> issue with verify_mbuf_check_panics().
> 
> > @@ -1199,10 +1212,11 @@
> >  	buf = rte_pktmbuf_alloc(pktmbuf_pool);
> >  	if (buf == NULL)
> >  		return -1;
> > +	/*
> >  	printf("Checking good mbuf initially\n");
> >  	if (verify_mbuf_check_panics(buf) != -1)
> >  		return -1;
> > -
> > +	*/
> >  	printf("Now checking for error conditions\n");
> >
> >  	if (verify_mbuf_check_panics(NULL)) { @@ -2411,6 +2425,120 @@
> struct
> > test_case {
> 
> Note: on my platform, it still works if I remove this comment.
> 
> Regards,
> Olivier
  

I checked with 2M huge pages - fork() works OK on my host, does not work over 1G.
RH7.2/3.10.327. Will try other kernels.

With best regards, Slava

> -----Original Message-----
> From: Slava Ovsiienko
> Sent: Thursday, February 6, 2020 10:25
> To: Olivier Matz <olivier.matz@6wind.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; stephen@networkplumber.org;
> thomas@mellanox.net
> Subject: RE: [PATCH] app/test: add test for mbuf with pinned external buffer
> 
> Olivier, thanks for the reviewing.
> I'll remove the comment and send the v2.
> I use 1G huge pages, will retest over 2M and continue finding why my host
> fails.
> 
> With best regards, Slava
> 
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, February 6, 2020 10:17
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh
> > <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; Shahaf Shuler
> > <shahafs@mellanox.com>; stephen@networkplumber.org;
> > thomas@mellanox.net
> > Subject: Re: [PATCH] app/test: add test for mbuf with pinned external
> > buffer
> >
> > Hi,
> >
> > On Fri, Jan 24, 2020 at 08:25:18PM +0000, Viacheslav Ovsiienko wrote:
> > > This patch adds unit test for the mbufs allocated from the special
> > > pool with pinned external data buffers.
> > >
> > > The pinned buffer mbufs are tested in the same way as regular ones
> > > with taking into account some specifics of cloning/attaching.
> > >
> > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >
> > Looks good to me, you can add my ack in the v2, once we understand the
> > issue with verify_mbuf_check_panics().
> >
> > > @@ -1199,10 +1212,11 @@
> > >  	buf = rte_pktmbuf_alloc(pktmbuf_pool);
> > >  	if (buf == NULL)
> > >  		return -1;
> > > +	/*
> > >  	printf("Checking good mbuf initially\n");
> > >  	if (verify_mbuf_check_panics(buf) != -1)
> > >  		return -1;
> > > -
> > > +	*/
> > >  	printf("Now checking for error conditions\n");
> > >
> > >  	if (verify_mbuf_check_panics(NULL)) { @@ -2411,6 +2425,120 @@
> > struct
> > > test_case {
> >
> > Note: on my platform, it still works if I remove this comment.
> >
> > Regards,
> > Olivier