diff mbox series

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

Message ID aaf5c1bd800bca00e4b3fe50fcb2f1b464fde1ee.1554114165.git.shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series DMA map anonymous memory to eth devices |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Shahaf Shuler April 1, 2019, 10:34 a.m. UTC 
  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(-)

Comments

Anatoly Burakov April 1, 2019, 1:50 p.m. UTC | #1 
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?
Shahaf Shuler April 2, 2019, 7:02 a.m. UTC | #2 
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
Anatoly Burakov April 2, 2019, 3:15 p.m. UTC | #3 
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
Shahaf Shuler April 3, 2019, 5:47 a.m. UTC | #4 
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
diff mbox series

Patch

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.