[PATCH 0/3] DMA map anonymous memory to eth devices

[PATCH 0/3] DMA map anonymous memory to eth devices

[Shahaf Shuler]

Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

Shahaf Shuler (3):
  app/testpmd: fix mempool free on exit
  app/testpmd: support creation of no IOVA contig mempools
  app/testpmd: map anonymous memory for eth devices

 app/test-pmd/parameters.c             |  5 ++
 app/test-pmd/testpmd.c                | 87 +++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h                |  4 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 ++
 4 files changed, 94 insertions(+), 7 deletions(-)

-- 
2.12.0

[PATCH 1/3] app/testpmd: fix mempool free on exit

[Shahaf Shuler]

Allocated mempools were never free. it is bad practice.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++++++++++------
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 40c873b972..7ea6c1d7e0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@ struct fwd_engine * fwd_engines[] = {
 	NULL,
 };
 
+struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
 uint32_t retry_enabled;
@@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 /*
  * Configuration initialisation done once at init time.
  */
-static void
+static struct rte_mempool *
 mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
@@ -904,6 +906,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
 		}
 	}
+	return rte_mp;
 
 err:
 	if (rte_mp == NULL) {
@@ -913,6 +916,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
+	return NULL;
 }
 
 /*
@@ -1130,14 +1134,18 @@ init_config(void)
 		uint8_t i;
 
 		for (i = 0; i < num_sockets; i++)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-					 socket_ids[i]);
+			mempools[i] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool,
+						       socket_ids[i]);
 	} else {
 		if (socket_num == UMA_NO_CONFIG)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool, 0);
+			mempools[0] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool, 0);
 		else
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-						 socket_num);
+			mempools[socket_num] = mbuf_pool_create
+							(mbuf_data_size,
+							 nb_mbuf_per_pool,
+							 socket_num);
 	}
 
 	init_port_config();
@@ -2396,6 +2404,7 @@ pmd_test_exit(void)
 	struct rte_device *device;
 	portid_t pt_id;
 	int ret;
+	int i;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -2449,6 +2458,9 @@ pmd_test_exit(void)
 			return;
 		}
 	}
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++)
+		if (mempools[i])
+			rte_mempool_free(mempools[i]);
 
 	printf("\nBye...\n");
 }
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a45988ebc5..84ce8ffa2f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
 
 extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
+extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 /**
  * Forwarding Configuration
  *
-- 
2.12.0

[PATCH 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

providing a command line parameter to set the mempool flags accordingly.
This mode is relevant only when creating an empty mempool and then
populating with memory.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/parameters.c             | 5 +++++
 app/test-pmd/testpmd.c                | 3 ++-
 app/test-pmd/testpmd.h                | 2 ++
 doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7b6b60905d..ce0056b0bd 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -202,6 +202,8 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random writes per packet\n");
 	printf("  --noisy-lkup-num-reads=N: do N random reads per packet\n");
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
+	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
+	       "valid only with --mp-alloc=anon\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -651,6 +653,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-writes",	1, 0, 0 },
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
+		{ "no-iova-contig",             0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1242,6 +1245,8 @@ launch_args_parse(int argc, char** argv)
 					rte_exit(EXIT_FAILURE,
 						 "noisy-lkup-num-reads-writes must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
+				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7ea6c1d7e0..8c4ebc774c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,7 @@ struct fwd_engine * fwd_engines[] = {
 };
 
 struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+uint16_t mempool_flags;
 
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
@@ -867,7 +868,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_mp = rte_mempool_create_empty(pool_name, nb_mbuf,
 				mb_size, (unsigned int) mb_mempool_cache,
 				sizeof(struct rte_pktmbuf_pool_private),
-				socket_id, 0);
+				socket_id, mempool_flags);
 			if (rte_mp == NULL)
 				goto err;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 84ce8ffa2f..46d26181dd 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -266,6 +266,8 @@ extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
 extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
 
+extern uint16_t mempool_flags;
+
 /**
  * Forwarding Configuration
  *
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index b717b8c7b7..7b4341647f 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -427,3 +427,8 @@ The commandline options are:
 
     Set the number of r/w accesses to be done in noisy neighbour simulation memory buffer to N.
     Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--no-iova-contig``
+
+    Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
+    The default value is 0.
-- 
2.12.0

Re: [PATCH 2/3] app/testpmd: support creation of no IOVA contig mempools

[Burakov, Anatoly]

On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> providing a command line parameter to set the mempool flags accordingly.
> This mode is relevant only when creating an empty mempool and then
> populating with memory.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>   app/test-pmd/parameters.c             | 5 +++++
>   app/test-pmd/testpmd.c                | 3 ++-
>   app/test-pmd/testpmd.h                | 2 ++
>   doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
>   4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 7b6b60905d..ce0056b0bd 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -202,6 +202,8 @@ usage(char* progname)
>   	printf("  --noisy-lkup-num-writes=N: do N random writes per packet\n");
>   	printf("  --noisy-lkup-num-reads=N: do N random reads per packet\n");
>   	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
> +	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
> +	       "valid only with --mp-alloc=anon\n");

Maybe rather than spelling this out in the help message, it would be 
better to enforce it at option parse time, and document it in the user 
guide?

-- 
Thanks,
Anatoly

Re: [PATCH 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

Monday, April 1, 2019 4:50 PM, Burakov, Anatoly:
> Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: support creation of no
> IOVA contig mempools
> 
> On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> > providing a command line parameter to set the mempool flags accordingly.
> > This mode is relevant only when creating an empty mempool and then
> > populating with memory.
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> >   app/test-pmd/parameters.c             | 5 +++++
> >   app/test-pmd/testpmd.c                | 3 ++-
> >   app/test-pmd/testpmd.h                | 2 ++
> >   doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
> >   4 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 7b6b60905d..ce0056b0bd 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -202,6 +202,8 @@ usage(char* progname)
> >   	printf("  --noisy-lkup-num-writes=N: do N random writes per
> packet\n");
> >   	printf("  --noisy-lkup-num-reads=N: do N random reads per
> packet\n");
> >   	printf("  --noisy-lkup-num-writes=N: do N random reads and writes
> > per packet\n");
> > +	printf("  --no-iova-contig: mempool memory can be IOVA non
> contiguous. "
> > +	       "valid only with --mp-alloc=anon\n");
> 
> Maybe rather than spelling this out in the help message, it would be better
> to enforce it at option parse time, 

No problem, I can add.

>and document it in the user guide?

I think I did documents it. what is missing?



> 
> --
> Thanks,
> Anatoly

Re: [PATCH 2/3] app/testpmd: support creation of no IOVA contig mempools

[Burakov, Anatoly]

On 02-Apr-19 8:02 AM, Shahaf Shuler wrote:
> Monday, April 1, 2019 4:50 PM, Burakov, Anatoly:
>> Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: support creation of no
>> IOVA contig mempools
>>
>> On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
>>> providing a command line parameter to set the mempool flags accordingly.
>>> This mode is relevant only when creating an empty mempool and then
>>> populating with memory.
>>>
>>> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
>>> ---
>>>    app/test-pmd/parameters.c             | 5 +++++
>>>    app/test-pmd/testpmd.c                | 3 ++-
>>>    app/test-pmd/testpmd.h                | 2 ++
>>>    doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
>>>    4 files changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index 7b6b60905d..ce0056b0bd 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -202,6 +202,8 @@ usage(char* progname)
>>>    	printf("  --noisy-lkup-num-writes=N: do N random writes per
>> packet\n");
>>>    	printf("  --noisy-lkup-num-reads=N: do N random reads per
>> packet\n");
>>>    	printf("  --noisy-lkup-num-writes=N: do N random reads and writes
>>> per packet\n");
>>> +	printf("  --no-iova-contig: mempool memory can be IOVA non
>> contiguous. "
>>> +	       "valid only with --mp-alloc=anon\n");
>>
>> Maybe rather than spelling this out in the help message, it would be better
>> to enforce it at option parse time,
> 
> No problem, I can add.
> 
>> and document it in the user guide?
> 
> I think I did documents it. what is missing?

Nothing, as it seems :) The user guide is fine. I just don't think we 
should explicitly reference other parameters in the help message, as it 
makes it harder to track down any changes/usages that may be needed.

> 
> 
> 
>>
>> --
>> Thanks,
>> Anatoly


-- 
Thanks,
Anatoly

Re: [PATCH 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

Tuesday, April 2, 2019 6:16 PM, Burakov, Anatoly:
> Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: support creation of no
> IOVA contig mempools
> 
> On 02-Apr-19 8:02 AM, Shahaf Shuler wrote:
> > Monday, April 1, 2019 4:50 PM, Burakov, Anatoly:
> >> Subject: Re: [dpdk-dev] [PATCH 2/3] app/testpmd: support creation of
> >> no IOVA contig mempools
> >>
> >> On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> >>> providing a command line parameter to set the mempool flags
> accordingly.
> >>> This mode is relevant only when creating an empty mempool and then
> >>> populating with memory.
> >>>
> >>> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> >>> ---
> >>>    app/test-pmd/parameters.c             | 5 +++++
> >>>    app/test-pmd/testpmd.c                | 3 ++-
> >>>    app/test-pmd/testpmd.h                | 2 ++
> >>>    doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
> >>>    4 files changed, 14 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> >>> index 7b6b60905d..ce0056b0bd 100644
> >>> --- a/app/test-pmd/parameters.c
> >>> +++ b/app/test-pmd/parameters.c
> >>> @@ -202,6 +202,8 @@ usage(char* progname)
> >>>    	printf("  --noisy-lkup-num-writes=N: do N random writes per
> >> packet\n");
> >>>    	printf("  --noisy-lkup-num-reads=N: do N random reads per
> >> packet\n");
> >>>    	printf("  --noisy-lkup-num-writes=N: do N random reads and
> >>> writes per packet\n");
> >>> +	printf("  --no-iova-contig: mempool memory can be IOVA non
> >> contiguous. "
> >>> +	       "valid only with --mp-alloc=anon\n");
> >>
> >> Maybe rather than spelling this out in the help message, it would be
> >> better to enforce it at option parse time,
> >
> > No problem, I can add.
> >
> >> and document it in the user guide?
> >
> > I think I did documents it. what is missing?
> 
> Nothing, as it seems :) The user guide is fine. I just don't think we should
> explicitly reference other parameters in the help message, as it makes it
> harder to track down any changes/usages that may be needed.

I think when people run testpmd and want to reason on the use of some parameter they use the help message and not the user guide (it is more straight forward).
Also - usually when changes are done one do grep for the field name it change, so it will catch also the parameter in the help message. Moreover, we have maintainers for this app to make sure nothing is missed. 

So I suggest to keep it. I don't think it will that of a burden and will simplify the user experience. 


> 
> >
> >
> >
> >>
> >> --
> >> Thanks,
> >> Anatoly
> 
> 
> --
> Thanks,
> Anatoly

[PATCH 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler]

Mempools can be populated with anonymous memory when using command line
parameter --mp-alloc=anon.

Considering the mempools are going to be used by the net devices,
it is better to DMA map this memory.

This patch add such mapping now that we have the APIs in place[1].

[1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 8c4ebc774c..6729469c6b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -834,6 +834,61 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 
 	return 0;
 }
+static void
+dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	int ret;
+
+	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to un-register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
+					memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA unmap addr 0x%p\n",
+				    memhdr->addr);
+		}
+	}
+}
+
+static void
+dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	size_t page_size = sysconf(_SC_PAGESIZE);
+	int ret;
+
+	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
+				  page_size);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
+				      memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA map addr 0x%p\n",
+				    memhdr->addr);
+		}
+	}
+}
 
 /*
  * Configuration initialisation done once at init time.
@@ -879,6 +934,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			}
 			rte_pktmbuf_pool_init(rte_mp, NULL);
 			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
+			rte_mempool_mem_iter(rte_mp, dma_map_cb, NULL);
 			break;
 		}
 	case MP_ALLOC_XMEM:
@@ -2460,8 +2516,12 @@ pmd_test_exit(void)
 		}
 	}
 	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++)
-		if (mempools[i])
+		if (mempools[i]) {
+			if (mp_alloc_type == MP_ALLOC_ANON)
+				rte_mempool_mem_iter(mempools[i], dma_unmap_cb,
+						     NULL);
 			rte_mempool_free(mempools[i]);
+		}
 
 	printf("\nBye...\n");
 }
-- 
2.12.0

Re: [PATCH 3/3] app/testpmd: map anonymous memory for eth devices

[Burakov, Anatoly]

On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> Mempools can be populated with anonymous memory when using command line
> parameter --mp-alloc=anon.
> 
> Considering the mempools are going to be used by the net devices,
> it is better to DMA map this memory.
> 
> This patch add such mapping now that we have the APIs in place[1].
> 
> [1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>   app/test-pmd/testpmd.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 8c4ebc774c..6729469c6b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -834,6 +834,61 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
>   
>   	return 0;
>   }
> +static void
> +dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
> +	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
> +{
> +	uint16_t pid = 0;
> +	int ret;
> +
> +	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
> +	if (ret) {
> +		TESTPMD_LOG(DEBUG,
> +			    "unable to un-register addr 0x%p\n", memhdr->addr);
> +		return;
> +	}
> +	RTE_ETH_FOREACH_DEV(pid) {
> +		struct rte_eth_dev *dev =
> +			&rte_eth_devices[pid];
> +
> +		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
> +					memhdr->len);
> +		if (ret) {
> +			TESTPMD_LOG(DEBUG,
> +				    "unable to DMA unmap addr 0x%p\n",
> +				    memhdr->addr);
> +		}
> +	}

I would expect unmap *then* unregister.

> +}
> +
> +static void
> +dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
> +	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
> +{
> +	uint16_t pid = 0;
> +	size_t page_size = sysconf(_SC_PAGESIZE);
> +	int ret;
> +
> +	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
> +				  page_size);
> +	if (ret) {
> +		TESTPMD_LOG(DEBUG,
> +			    "unable to register addr 0x%p\n", memhdr->addr);
> +		return;
> +	}
> +	RTE_ETH_FOREACH_DEV(pid) {
> +		struct rte_eth_dev *dev =
> +			&rte_eth_devices[pid];
> +
> +		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
> +				      memhdr->len);
> +		if (ret) {
> +			TESTPMD_LOG(DEBUG,
> +				    "unable to DMA map addr 0x%p\n",
> +				    memhdr->addr);

Maybe mention the device/driver name as well? Otherwise it's hard to see 
which particular DMA map/unmap has failed, should one choose to 
debug-by-logs.

> +		}
> +	}
> +}
>   
>   /*
>    * Configuration initialisation done once at init time.
> @@ -879,6 +934,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>   			}
>   			rte_pktmbuf_pool_init(rte_mp, NULL);
>   			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
> +			rte_mempool_mem_iter(rte_mp, dma_map_cb, NULL);
>   			break;
>   		}
>   	case MP_ALLOC_XMEM:
> @@ -2460,8 +2516,12 @@ pmd_test_exit(void)
>   		}
>   	}
>   	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++)
> -		if (mempools[i])
> +		if (mempools[i]) {
> +			if (mp_alloc_type == MP_ALLOC_ANON)
> +				rte_mempool_mem_iter(mempools[i], dma_unmap_cb,
> +						     NULL);
>   			rte_mempool_free(mempools[i]);
> +		}
>   
>   	printf("\nBye...\n");
>   }
> 


-- 
Thanks,
Anatoly

Re: [PATCH 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler]

Monday, April 1, 2019 4:28 PM, Burakov, Anatoly:
> Subject: Re: [dpdk-dev] [PATCH 3/3] app/testpmd: map anonymous memory
> for eth devices
> 
> On 01-Apr-19 11:34 AM, Shahaf Shuler wrote:
> > Mempools can be populated with anonymous memory when using
> command
> > line parameter --mp-alloc=anon.
> >
> > Considering the mempools are going to be used by the net devices, it
> > is better to DMA map this memory.
> >
> > This patch add such mapping now that we have the APIs in place[1].
> >
> > [1] commit c33a675b6276 ("bus: introduce device level DMA memory
> > mapping")
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > ---
> >   app/test-pmd/testpmd.c | 62
> ++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 8c4ebc774c..6729469c6b 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -834,6 +834,61 @@ setup_extmem(uint32_t nb_mbufs, uint32_t
> mbuf_sz,
> > bool huge)
> >
> >   	return 0;
> >   }
> > +static void
> > +dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque
> __rte_unused,
> > +	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx
> > +__rte_unused) {
> > +	uint16_t pid = 0;
> > +	int ret;
> > +
> > +	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
> > +	if (ret) {
> > +		TESTPMD_LOG(DEBUG,
> > +			    "unable to un-register addr 0x%p\n", memhdr-
> >addr);
> > +		return;
> > +	}
> > +	RTE_ETH_FOREACH_DEV(pid) {
> > +		struct rte_eth_dev *dev =
> > +			&rte_eth_devices[pid];
> > +
> > +		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
> > +					memhdr->len);
> > +		if (ret) {
> > +			TESTPMD_LOG(DEBUG,
> > +				    "unable to DMA unmap addr 0x%p\n",
> > +				    memhdr->addr);
> > +		}
> > +	}
> 
> I would expect unmap *then* unregister.

Right, thanks. 

> 
> > +}
> > +
> > +static void
> > +dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque
> __rte_unused,
> > +	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx
> __rte_unused)
> > +{
> > +	uint16_t pid = 0;
> > +	size_t page_size = sysconf(_SC_PAGESIZE);
> > +	int ret;
> > +
> > +	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
> > +				  page_size);
> > +	if (ret) {
> > +		TESTPMD_LOG(DEBUG,
> > +			    "unable to register addr 0x%p\n", memhdr->addr);
> > +		return;
> > +	}
> > +	RTE_ETH_FOREACH_DEV(pid) {
> > +		struct rte_eth_dev *dev =
> > +			&rte_eth_devices[pid];
> > +
> > +		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
> > +				      memhdr->len);
> > +		if (ret) {
> > +			TESTPMD_LOG(DEBUG,
> > +				    "unable to DMA map addr 0x%p\n",
> > +				    memhdr->addr);
> 
> Maybe mention the device/driver name as well? Otherwise it's hard to see
> which particular DMA map/unmap has failed, should one choose to debug-
> by-logs.

Will add. 

> 
> > +		}
> > +	}
> > +}
> >
> >   /*
> >    * Configuration initialisation done once at init time.
> > @@ -879,6 +934,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size,
> unsigned nb_mbuf,
> >   			}
> >   			rte_pktmbuf_pool_init(rte_mp, NULL);
> >   			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init,
> NULL);
> > +			rte_mempool_mem_iter(rte_mp, dma_map_cb,
> NULL);
> >   			break;
> >   		}
> >   	case MP_ALLOC_XMEM:
> > @@ -2460,8 +2516,12 @@ pmd_test_exit(void)
> >   		}
> >   	}
> >   	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++)
> > -		if (mempools[i])
> > +		if (mempools[i]) {
> > +			if (mp_alloc_type == MP_ALLOC_ANON)
> > +				rte_mempool_mem_iter(mempools[i],
> dma_unmap_cb,
> > +						     NULL);
> >   			rte_mempool_free(mempools[i]);
> > +		}
> >
> >   	printf("\nBye...\n");
> >   }
> >
> 
> 
> --
> Thanks,
> Anatoly

[PATCH v2 0/3] DMA map anonymous memory to eth devices

[Shahaf Shuler]

Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

On v2:
 * enforcing parameter validity at option parse time.
 * precede unmap to be before unregister.
 * extended debug log verbosity to include eth device name.
 * precede unmap and unregister to happen before eth devices are detached.


Shahaf Shuler (3):
  app/testpmd: fix mempool free on exit
  app/testpmd: support creation of no IOVA contig mempools
  app/testpmd: map anonymous memory for eth devices

 app/test-pmd/parameters.c             | 13 +++++
 app/test-pmd/testpmd.c                | 93 +++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h                |  4 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 ++
 4 files changed, 108 insertions(+), 7 deletions(-)

-- 
2.12.0

[PATCH v2 1/3] app/testpmd: fix mempool free on exit

[Shahaf Shuler]

Allocated mempools were never free. it is bad practice.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 25 +++++++++++++++++++------
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 40c873b972..5c68eb9ec6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@ struct fwd_engine * fwd_engines[] = {
 	NULL,
 };
 
+struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
 uint32_t retry_enabled;
@@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 /*
  * Configuration initialisation done once at init time.
  */
-static void
+static struct rte_mempool *
 mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
@@ -904,6 +906,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
 		}
 	}
+	return rte_mp;
 
 err:
 	if (rte_mp == NULL) {
@@ -913,6 +916,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
+	return NULL;
 }
 
 /*
@@ -1130,14 +1134,18 @@ init_config(void)
 		uint8_t i;
 
 		for (i = 0; i < num_sockets; i++)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-					 socket_ids[i]);
+			mempools[i] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool,
+						       socket_ids[i]);
 	} else {
 		if (socket_num == UMA_NO_CONFIG)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool, 0);
+			mempools[0] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool, 0);
 		else
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-						 socket_num);
+			mempools[socket_num] = mbuf_pool_create
+							(mbuf_data_size,
+							 nb_mbuf_per_pool,
+							 socket_num);
 	}
 
 	init_port_config();
@@ -2396,6 +2404,7 @@ pmd_test_exit(void)
 	struct rte_device *device;
 	portid_t pt_id;
 	int ret;
+	int i;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -2449,6 +2458,10 @@ pmd_test_exit(void)
 			return;
 		}
 	}
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i])
+			rte_mempool_free(mempools[i]);
+	}
 
 	printf("\nBye...\n");
 }
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a45988ebc5..84ce8ffa2f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
 
 extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
+extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 /**
  * Forwarding Configuration
  *
-- 
2.12.0

Re: [dpdk-dev] [PATCH v2 1/3] app/testpmd: fix mempool free on exit

[Iremonger, Bernard]

> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Thursday, April 4, 2019 6:15 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; rasland@mellanox.com; thomas@monjalon.net; Yigit,
> Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> Subject: [PATCH v2 1/3] app/testpmd: fix mempool free on exit
> 
> Allocated mempools were never free. it is bad practice.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com> 

[PATCH v2 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

providing a command line parameter to set the mempool flags accordingly.
This mode is relevant only when creating an empty mempool and then
populating with memory.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/parameters.c             | 13 +++++++++++++
 app/test-pmd/testpmd.c                |  3 ++-
 app/test-pmd/testpmd.h                |  2 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7b6b60905d..7f18b3e748 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -202,6 +202,8 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random writes per packet\n");
 	printf("  --noisy-lkup-num-reads=N: do N random reads per packet\n");
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
+	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
+	       "valid only with --mp-alloc=anon\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -651,6 +653,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-writes",	1, 0, 0 },
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
+		{ "no-iova-contig",             0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1242,6 +1245,8 @@ launch_args_parse(int argc, char** argv)
 					rte_exit(EXIT_FAILURE,
 						 "noisy-lkup-num-reads-writes must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
+				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
 			break;
 		case 'h':
 			usage(argv[0]);
@@ -1258,4 +1263,12 @@ launch_args_parse(int argc, char** argv)
 	/* Set offload configuration from command line parameters. */
 	rx_mode.offloads = rx_offloads;
 	tx_mode.offloads = tx_offloads;
+
+	if (mempool_flags & MEMPOOL_F_NO_IOVA_CONTIG &&
+	    mp_alloc_type != MP_ALLOC_ANON) {
+		TESTPMD_LOG(WARNING, "cannot use no-iova-contig without "
+				  "mp-alloc=anon. mempool no-iova-contig is "
+				  "ignored\n");
+		mempool_flags = 0;
+	}
 }
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5c68eb9ec6..dd449a9859 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,7 @@ struct fwd_engine * fwd_engines[] = {
 };
 
 struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+uint16_t mempool_flags;
 
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
@@ -867,7 +868,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_mp = rte_mempool_create_empty(pool_name, nb_mbuf,
 				mb_size, (unsigned int) mb_mempool_cache,
 				sizeof(struct rte_pktmbuf_pool_private),
-				socket_id, 0);
+				socket_id, mempool_flags);
 			if (rte_mp == NULL)
 				goto err;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 84ce8ffa2f..46d26181dd 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -266,6 +266,8 @@ extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
 extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
 
+extern uint16_t mempool_flags;
+
 /**
  * Forwarding Configuration
  *
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index b717b8c7b7..7b4341647f 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -427,3 +427,8 @@ The commandline options are:
 
     Set the number of r/w accesses to be done in noisy neighbour simulation memory buffer to N.
     Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--no-iova-contig``
+
+    Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
+    The default value is 0.
-- 
2.12.0

Re: [PATCH v2 2/3] app/testpmd: support creation of no IOVA contig mempools

[Burakov, Anatoly]

On 04-Apr-19 6:14 AM, Shahaf Shuler wrote:
> providing a command line parameter to set the mempool flags accordingly.
> This mode is relevant only when creating an empty mempool and then
> populating with memory.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

[PATCH v2 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler]

Mempools can be populated with anonymous memory when using command line
parameter --mp-alloc=anon.

Considering the mempools are going to be used by the net devices,
it is better to DMA map this memory.

This patch add such mapping now that we have the APIs in place[1].

[1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dd449a9859..3ad111a143 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -834,6 +834,63 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 
 	return 0;
 }
+static void
+dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	int ret;
+
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
+					memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA unmap addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to un-register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+}
+
+static void
+dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	size_t page_size = sysconf(_SC_PAGESIZE);
+	int ret;
+
+	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
+				  page_size);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
+				      memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA map addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+}
 
 /*
  * Configuration initialisation done once at init time.
@@ -879,6 +936,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			}
 			rte_pktmbuf_pool_init(rte_mp, NULL);
 			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
+			rte_mempool_mem_iter(rte_mp, dma_map_cb, NULL);
 			break;
 		}
 	case MP_ALLOC_XMEM:
@@ -2410,6 +2468,13 @@ pmd_test_exit(void)
 	if (test_done == 0)
 		stop_packet_forwarding();
 
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i]) {
+			if (mp_alloc_type == MP_ALLOC_ANON)
+				rte_mempool_mem_iter(mempools[i], dma_unmap_cb,
+						     NULL);
+		}
+	}
 	if (ports != NULL) {
 		no_link_check = 1;
 		RTE_ETH_FOREACH_DEV(pt_id) {
-- 
2.12.0

Re: [PATCH v2 3/3] app/testpmd: map anonymous memory for eth devices

[Burakov, Anatoly]

On 04-Apr-19 6:14 AM, Shahaf Shuler wrote:
> Mempools can be populated with anonymous memory when using command line
> parameter --mp-alloc=anon.
> 
> Considering the mempools are going to be used by the net devices,
> it is better to DMA map this memory.
> 
> This patch add such mapping now that we have the APIs in place[1].
> 
> [1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
>   app/test-pmd/testpmd.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 65 insertions(+)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index dd449a9859..3ad111a143 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -834,6 +834,63 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
>   
>   	return 0;
>   }
> +static void
> +dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
> +	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
> +{
> +	uint16_t pid = 0;
> +	int ret;
> +
> +	RTE_ETH_FOREACH_DEV(pid) {
> +		struct rte_eth_dev *dev =
> +			&rte_eth_devices[pid];
> +
> +		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
> +					memhdr->len);
> +		if (ret) {
> +			TESTPMD_LOG(DEBUG,
> +				    "unable to DMA unmap addr 0x%p "
> +				    "for device %s\n",
> +				    memhdr->addr, dev->data->name);
> +		}
> +	}
> +	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
> +	if (ret) {
> +		TESTPMD_LOG(DEBUG,
> +			    "unable to un-register addr 0x%p\n", memhdr->addr);
> +		return;

No need for a return here.

Otherwise,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

[PATCH v3 0/3] DMA map anonymous memory to eth devices

[Shahaf Shuler]

Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

On v3:
 * remove redundant return.

On v2:
 * enforcing parameter validity at option parse time.
 * precede unmap to be before unregister.
 * extended debug log verbosity to include eth device name.
 * precede unmap and unregister to happen before eth devices are detached.

Shahaf Shuler (3):
  app/testpmd: fix mempool free on exit
  app/testpmd: support creation of no IOVA contig mempools
  app/testpmd: map anonymous memory for eth devices

 app/test-pmd/parameters.c             | 13 +++++
 app/test-pmd/testpmd.c                | 92 +++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h                |  4 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 ++
 4 files changed, 107 insertions(+), 7 deletions(-)

-- 
2.12.0

[PATCH v3 1/3] app/testpmd: fix mempool free on exit

[Shahaf Shuler]

Allocated mempools were never free. it is bad practice.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 25 +++++++++++++++++++------
 app/test-pmd/testpmd.h |  2 ++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 40c873b972..5c68eb9ec6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@ struct fwd_engine * fwd_engines[] = {
 	NULL,
 };
 
+struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
 uint32_t retry_enabled;
@@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 /*
  * Configuration initialisation done once at init time.
  */
-static void
+static struct rte_mempool *
 mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
@@ -904,6 +906,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
 		}
 	}
+	return rte_mp;
 
 err:
 	if (rte_mp == NULL) {
@@ -913,6 +916,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
+	return NULL;
 }
 
 /*
@@ -1130,14 +1134,18 @@ init_config(void)
 		uint8_t i;
 
 		for (i = 0; i < num_sockets; i++)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-					 socket_ids[i]);
+			mempools[i] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool,
+						       socket_ids[i]);
 	} else {
 		if (socket_num == UMA_NO_CONFIG)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool, 0);
+			mempools[0] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool, 0);
 		else
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-						 socket_num);
+			mempools[socket_num] = mbuf_pool_create
+							(mbuf_data_size,
+							 nb_mbuf_per_pool,
+							 socket_num);
 	}
 
 	init_port_config();
@@ -2396,6 +2404,7 @@ pmd_test_exit(void)
 	struct rte_device *device;
 	portid_t pt_id;
 	int ret;
+	int i;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -2449,6 +2458,10 @@ pmd_test_exit(void)
 			return;
 		}
 	}
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i])
+			rte_mempool_free(mempools[i]);
+	}
 
 	printf("\nBye...\n");
 }
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a45988ebc5..84ce8ffa2f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
 
 extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
+extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 /**
  * Forwarding Configuration
  *
-- 
2.12.0

Re: [dpdk-dev] [PATCH v3 1/3] app/testpmd: fix mempool free on exit

[Ferruh Yigit]

On 4/4/2019 8:34 PM, Shahaf Shuler wrote:
> Allocated mempools were never free. it is bad practice.

+1

> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
<...>

> @@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
>  /*
>   * Configuration initialisation done once at init time.
>   */
> -static void
> +static struct rte_mempool *
>  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  		 unsigned int socket_id)
>  {
> @@ -904,6 +906,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
>  		}
>  	}
> +	return rte_mp;
>  
>  err:
>  	if (rte_mp == NULL) {
> @@ -913,6 +916,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>  	} else if (verbose_level > 0) {
>  		rte_mempool_dump(stdout, rte_mp);
>  	}
> +	return NULL;

This return never reached, because a few lines above there is:
  if (rte_mp == NULL) {
     rte_exit(EXIT_FAILURE ...

And for above "return rte_mp;" case, it skips "if (verbose_level > 0)" checks
because of an early return.

So what do you think remove above "return rte_mp;", and move here, instead of
NULL return?

<...>

> @@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
>  
>  extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
>  
> +extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];

There is no other .c file using 'mempools', can drop the extern.

[PATCH v3 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

providing a command line parameter to set the mempool flags accordingly.
This mode is relevant only when creating an empty mempool and then
populating with memory.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/parameters.c             | 13 +++++++++++++
 app/test-pmd/testpmd.c                |  3 ++-
 app/test-pmd/testpmd.h                |  2 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7b6b60905d..7f18b3e748 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -202,6 +202,8 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random writes per packet\n");
 	printf("  --noisy-lkup-num-reads=N: do N random reads per packet\n");
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
+	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
+	       "valid only with --mp-alloc=anon\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -651,6 +653,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-writes",	1, 0, 0 },
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
+		{ "no-iova-contig",             0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1242,6 +1245,8 @@ launch_args_parse(int argc, char** argv)
 					rte_exit(EXIT_FAILURE,
 						 "noisy-lkup-num-reads-writes must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
+				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
 			break;
 		case 'h':
 			usage(argv[0]);
@@ -1258,4 +1263,12 @@ launch_args_parse(int argc, char** argv)
 	/* Set offload configuration from command line parameters. */
 	rx_mode.offloads = rx_offloads;
 	tx_mode.offloads = tx_offloads;
+
+	if (mempool_flags & MEMPOOL_F_NO_IOVA_CONTIG &&
+	    mp_alloc_type != MP_ALLOC_ANON) {
+		TESTPMD_LOG(WARNING, "cannot use no-iova-contig without "
+				  "mp-alloc=anon. mempool no-iova-contig is "
+				  "ignored\n");
+		mempool_flags = 0;
+	}
 }
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5c68eb9ec6..dd449a9859 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,7 @@ struct fwd_engine * fwd_engines[] = {
 };
 
 struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+uint16_t mempool_flags;
 
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
@@ -867,7 +868,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_mp = rte_mempool_create_empty(pool_name, nb_mbuf,
 				mb_size, (unsigned int) mb_mempool_cache,
 				sizeof(struct rte_pktmbuf_pool_private),
-				socket_id, 0);
+				socket_id, mempool_flags);
 			if (rte_mp == NULL)
 				goto err;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 84ce8ffa2f..46d26181dd 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -266,6 +266,8 @@ extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
 extern struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
 
+extern uint16_t mempool_flags;
+
 /**
  * Forwarding Configuration
  *
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index b717b8c7b7..7b4341647f 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -427,3 +427,8 @@ The commandline options are:
 
     Set the number of r/w accesses to be done in noisy neighbour simulation memory buffer to N.
     Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--no-iova-contig``
+
+    Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
+    The default value is 0.
-- 
2.12.0

[PATCH v3 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler]

Mempools can be populated with anonymous memory when using command line
parameter --mp-alloc=anon.

Considering the mempools are going to be used by the net devices,
it is better to DMA map this memory.

This patch add such mapping now that we have the APIs in place[1].

[1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/testpmd.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index dd449a9859..3d1c855e52 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -834,6 +834,62 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 
 	return 0;
 }
+static void
+dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	int ret;
+
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
+					memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA unmap addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to un-register addr 0x%p\n", memhdr->addr);
+	}
+}
+
+static void
+dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	size_t page_size = sysconf(_SC_PAGESIZE);
+	int ret;
+
+	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
+				  page_size);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
+				      memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA map addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+}
 
 /*
  * Configuration initialisation done once at init time.
@@ -879,6 +935,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			}
 			rte_pktmbuf_pool_init(rte_mp, NULL);
 			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
+			rte_mempool_mem_iter(rte_mp, dma_map_cb, NULL);
 			break;
 		}
 	case MP_ALLOC_XMEM:
@@ -2410,6 +2467,13 @@ pmd_test_exit(void)
 	if (test_done == 0)
 		stop_packet_forwarding();
 
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i]) {
+			if (mp_alloc_type == MP_ALLOC_ANON)
+				rte_mempool_mem_iter(mempools[i], dma_unmap_cb,
+						     NULL);
+		}
+	}
 	if (ports != NULL) {
 		no_link_check = 1;
 		RTE_ETH_FOREACH_DEV(pt_id) {
-- 
2.12.0

Re: [dpdk-dev] [PATCH v3 0/3] DMA map anonymous memory to eth devices

[Ferruh Yigit]

On 4/4/2019 8:34 PM, Shahaf Shuler wrote:
> Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")
> 
> On v3:
>  * remove redundant return.
> 
> On v2:
>  * enforcing parameter validity at option parse time.
>  * precede unmap to be before unregister.
>  * extended debug log verbosity to include eth device name.
>  * precede unmap and unregister to happen before eth devices are detached.
> 
> Shahaf Shuler (3):
>   app/testpmd: fix mempool free on exit
>   app/testpmd: support creation of no IOVA contig mempools
>   app/testpmd: map anonymous memory for eth devices
> 

Hi Wenzhuo, Jingjing, Bernard,

Can you please check this patch?

Thanks,
ferruh

[dpdk-dev] [PATCH v4 0/3] DMA map anonymous memory to eth devices

[Shahaf Shuler]

Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

On v4:
 * simplify return logic from mbuf_pool_create.
 * remove extern from mempools structure.

On v3:
 * remove redundant return.

On v2:
 * enforcing parameter validity at option parse time.
 * precede unmap to be before unregister.
 * extended debug log verbosity to include eth device name.
 * precede unmap and unregister to happen before eth devices are detached.

Shahaf Shuler (3):
  app/testpmd: fix mempool free on exit
  app/testpmd: support creation of no IOVA contig mempools
  app/testpmd: map anonymous memory for eth devices

 app/test-pmd/parameters.c             | 13 +++++
 app/test-pmd/testpmd.c                | 91 +++++++++++++++++++++++++++---
 app/test-pmd/testpmd.h                |  2 +
 doc/guides/testpmd_app_ug/run_app.rst |  5 ++
 4 files changed, 104 insertions(+), 7 deletions(-)

-- 
2.12.0

[dpdk-dev] [PATCH v4 1/3] app/testpmd: fix mempool free on exit

[Shahaf Shuler]

Allocated mempools were never free. it is bad practice.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/testpmd.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index aeaa74c989..b7f70b0c47 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@ struct fwd_engine * fwd_engines[] = {
 	NULL,
 };
 
+struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
 uint32_t retry_enabled;
@@ -835,7 +837,7 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 /*
  * Configuration initialisation done once at init time.
  */
-static void
+static struct rte_mempool *
 mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 		 unsigned int socket_id)
 {
@@ -913,6 +915,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
+	return rte_mp;
 }
 
 /*
@@ -1130,14 +1133,18 @@ init_config(void)
 		uint8_t i;
 
 		for (i = 0; i < num_sockets; i++)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-					 socket_ids[i]);
+			mempools[i] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool,
+						       socket_ids[i]);
 	} else {
 		if (socket_num == UMA_NO_CONFIG)
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool, 0);
+			mempools[0] = mbuf_pool_create(mbuf_data_size,
+						       nb_mbuf_per_pool, 0);
 		else
-			mbuf_pool_create(mbuf_data_size, nb_mbuf_per_pool,
-						 socket_num);
+			mempools[socket_num] = mbuf_pool_create
+							(mbuf_data_size,
+							 nb_mbuf_per_pool,
+							 socket_num);
 	}
 
 	init_port_config();
@@ -2394,6 +2401,7 @@ pmd_test_exit(void)
 	struct rte_device *device;
 	portid_t pt_id;
 	int ret;
+	int i;
 
 	if (test_done == 0)
 		stop_packet_forwarding();
@@ -2447,6 +2455,10 @@ pmd_test_exit(void)
 			return;
 		}
 	}
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i])
+			rte_mempool_free(mempools[i]);
+	}
 
 	printf("\nBye...\n");
 }
-- 
2.12.0

Re: [dpdk-dev] [PATCH v4 1/3] app/testpmd: fix mempool free on exit

[Iremonger, Bernard]

> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Sunday, April 7, 2019 6:02 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; rasland@mellanox.com; thomas@monjalon.net; Yigit,
> Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> Subject: [PATCH v4 1/3] app/testpmd: fix mempool free on exit
> 
> Allocated mempools were never free. it is bad practice.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>

Acked-by:  Bernard Iremonger <bernard.iremonger@intel.com>

[dpdk-dev] [PATCH v4 2/3] app/testpmd: support creation of no IOVA contig mempools

[Shahaf Shuler]

providing a command line parameter to set the mempool flags accordingly.
This mode is relevant only when creating an empty mempool and then
populating with memory.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/parameters.c             | 13 +++++++++++++
 app/test-pmd/testpmd.c                |  3 ++-
 app/test-pmd/testpmd.h                |  2 ++
 doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7b6b60905d..7f18b3e748 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -202,6 +202,8 @@ usage(char* progname)
 	printf("  --noisy-lkup-num-writes=N: do N random writes per packet\n");
 	printf("  --noisy-lkup-num-reads=N: do N random reads per packet\n");
 	printf("  --noisy-lkup-num-writes=N: do N random reads and writes per packet\n");
+	printf("  --no-iova-contig: mempool memory can be IOVA non contiguous. "
+	       "valid only with --mp-alloc=anon\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -651,6 +653,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-writes",	1, 0, 0 },
 		{ "noisy-lkup-num-reads",	1, 0, 0 },
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
+		{ "no-iova-contig",             0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1242,6 +1245,8 @@ launch_args_parse(int argc, char** argv)
 					rte_exit(EXIT_FAILURE,
 						 "noisy-lkup-num-reads-writes must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "no-iova-contig"))
+				mempool_flags = MEMPOOL_F_NO_IOVA_CONTIG;
 			break;
 		case 'h':
 			usage(argv[0]);
@@ -1258,4 +1263,12 @@ launch_args_parse(int argc, char** argv)
 	/* Set offload configuration from command line parameters. */
 	rx_mode.offloads = rx_offloads;
 	tx_mode.offloads = tx_offloads;
+
+	if (mempool_flags & MEMPOOL_F_NO_IOVA_CONTIG &&
+	    mp_alloc_type != MP_ALLOC_ANON) {
+		TESTPMD_LOG(WARNING, "cannot use no-iova-contig without "
+				  "mp-alloc=anon. mempool no-iova-contig is "
+				  "ignored\n");
+		mempool_flags = 0;
+	}
 }
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b7f70b0c47..3b5ef0b6c1 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,7 @@ struct fwd_engine * fwd_engines[] = {
 };
 
 struct rte_mempool *mempools[RTE_MAX_NUMA_NODES];
+uint16_t mempool_flags;
 
 struct fwd_config cur_fwd_config;
 struct fwd_engine *cur_fwd_eng = &io_fwd_engine; /**< IO mode by default. */
@@ -867,7 +868,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			rte_mp = rte_mempool_create_empty(pool_name, nb_mbuf,
 				mb_size, (unsigned int) mb_mempool_cache,
 				sizeof(struct rte_pktmbuf_pool_private),
-				socket_id, 0);
+				socket_id, mempool_flags);
 			if (rte_mp == NULL)
 				goto err;
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a45988ebc5..e2117e66ff 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -264,6 +264,8 @@ extern struct fwd_engine ieee1588_fwd_engine;
 
 extern struct fwd_engine * fwd_engines[]; /**< NULL terminated array. */
 
+extern uint16_t mempool_flags;
+
 /**
  * Forwarding Configuration
  *
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index b717b8c7b7..7b4341647f 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -427,3 +427,8 @@ The commandline options are:
 
     Set the number of r/w accesses to be done in noisy neighbour simulation memory buffer to N.
     Only available with the noisy forwarding mode. The default value is 0.
+
+*   ``--no-iova-contig``
+
+    Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon.
+    The default value is 0.
-- 
2.12.0

[dpdk-dev] [PATCH v4 3/3] app/testpmd: map anonymous memory for eth devices

[Shahaf Shuler]

Mempools can be populated with anonymous memory when using command line
parameter --mp-alloc=anon.

Considering the mempools are going to be used by the net devices,
it is better to DMA map this memory.

This patch add such mapping now that we have the APIs in place[1].

[1] commit c33a675b6276 ("bus: introduce device level DMA memory mapping")

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 app/test-pmd/testpmd.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3b5ef0b6c1..2e99d99e12 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -834,6 +834,62 @@ setup_extmem(uint32_t nb_mbufs, uint32_t mbuf_sz, bool huge)
 
 	return 0;
 }
+static void
+dma_unmap_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	     struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	int ret;
+
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_unmap(dev->device, memhdr->addr, 0,
+					memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA unmap addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+	ret = rte_extmem_unregister(memhdr->addr, memhdr->len);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to un-register addr 0x%p\n", memhdr->addr);
+	}
+}
+
+static void
+dma_map_cb(struct rte_mempool *mp __rte_unused, void *opaque __rte_unused,
+	   struct rte_mempool_memhdr *memhdr, unsigned mem_idx __rte_unused)
+{
+	uint16_t pid = 0;
+	size_t page_size = sysconf(_SC_PAGESIZE);
+	int ret;
+
+	ret = rte_extmem_register(memhdr->addr, memhdr->len, NULL, 0,
+				  page_size);
+	if (ret) {
+		TESTPMD_LOG(DEBUG,
+			    "unable to register addr 0x%p\n", memhdr->addr);
+		return;
+	}
+	RTE_ETH_FOREACH_DEV(pid) {
+		struct rte_eth_dev *dev =
+			&rte_eth_devices[pid];
+
+		ret = rte_dev_dma_map(dev->device, memhdr->addr, 0,
+				      memhdr->len);
+		if (ret) {
+			TESTPMD_LOG(DEBUG,
+				    "unable to DMA map addr 0x%p "
+				    "for device %s\n",
+				    memhdr->addr, dev->data->name);
+		}
+	}
+}
 
 /*
  * Configuration initialisation done once at init time.
@@ -879,6 +935,7 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 			}
 			rte_pktmbuf_pool_init(rte_mp, NULL);
 			rte_mempool_obj_iter(rte_mp, rte_pktmbuf_init, NULL);
+			rte_mempool_mem_iter(rte_mp, dma_map_cb, NULL);
 			break;
 		}
 	case MP_ALLOC_XMEM:
@@ -2407,6 +2464,13 @@ pmd_test_exit(void)
 	if (test_done == 0)
 		stop_packet_forwarding();
 
+	for (i = 0 ; i < RTE_MAX_NUMA_NODES ; i++) {
+		if (mempools[i]) {
+			if (mp_alloc_type == MP_ALLOC_ANON)
+				rte_mempool_mem_iter(mempools[i], dma_unmap_cb,
+						     NULL);
+		}
+	}
 	if (ports != NULL) {
 		no_link_check = 1;
 		RTE_ETH_FOREACH_DEV(pt_id) {
-- 
2.12.0

Re: [dpdk-dev] [PATCH v4 0/3] DMA map anonymous memory to eth devices

[Ferruh Yigit]

On 4/7/2019 6:02 AM, Shahaf Shuler wrote:
> Small series to use the APIs introduced by commit c33a675b6276 ("bus: introduce device level DMA memory mapping")
> 
> On v4:
>  * simplify return logic from mbuf_pool_create.
>  * remove extern from mempools structure.
> 
> On v3:
>  * remove redundant return.
> 
> On v2:
>  * enforcing parameter validity at option parse time.
>  * precede unmap to be before unregister.
>  * extended debug log verbosity to include eth device name.
>  * precede unmap and unregister to happen before eth devices are detached.
> 
> Shahaf Shuler (3):
>   app/testpmd: fix mempool free on exit
>   app/testpmd: support creation of no IOVA contig mempools
>   app/testpmd: map anonymous memory for eth devices

Series applied to dpdk-next-net/master, thanks.