[v8] app/testpmd: support multi-process

Message ID 1617068905-5364-1-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v8] app/testpmd: support multi-process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS

Commit Message

humin (Q) March 30, 2021, 1:48 a.m. UTC
  From: Lijun Ou <oulijun@huawei.com>

This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0

the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
v8:
* Added warning info about queue numbers and process numbers.

v7:
* Fixed compiling error for unexpected unindent.

v6:
* Add rte flow description for multiple process.

v5:
* Fixed run_app.rst for multiple process description.
* Fix compiling error.

v4:
* Fixed minimum vlaue of Rxq or Txq in doc.

v3:
* Fixed compiling error using gcc10.0.

v2:
* Added document for this patch.
---
 app/test-pmd/cmdline.c                |  12 +++-
 app/test-pmd/config.c                 |  14 +++-
 app/test-pmd/parameters.c             |  11 +++
 app/test-pmd/testpmd.c                | 127 ++++++++++++++++++++++------------
 app/test-pmd/testpmd.h                |   7 ++
 doc/guides/testpmd_app_ug/run_app.rst | 101 +++++++++++++++++++++++++++
 6 files changed, 226 insertions(+), 46 deletions(-)
  

Comments

Li, Xiaoyun March 30, 2021, 2:17 a.m. UTC | #1
Hi

> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Tuesday, March 30, 2021 09:48
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; ajit.khaparde@broadcom.com; Li,
> Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v8] app/testpmd: support multi-process
> 
> From: Lijun Ou <oulijun@huawei.com>
> 
> This patch adds multi-process support for testpmd.
> The test cmd example as follows:
> the primary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
> 
> the secondary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
> v8:
> * Added warning info about queue numbers and process numbers.
> 
> v7:
> * Fixed compiling error for unexpected unindent.
> 
> v6:
> * Add rte flow description for multiple process.
> 
> v5:
> * Fixed run_app.rst for multiple process description.
> * Fix compiling error.
> 
> v4:
> * Fixed minimum vlaue of Rxq or Txq in doc.
> 
> v3:
> * Fixed compiling error using gcc10.0.
> 
> v2:
> * Added document for this patch.
> ---
>  app/test-pmd/cmdline.c                |  12 +++-
>  app/test-pmd/config.c                 |  14 +++-
>  app/test-pmd/parameters.c             |  11 +++
>  app/test-pmd/testpmd.c                | 127 ++++++++++++++++++++++------------
>  app/test-pmd/testpmd.h                |   7 ++
>  doc/guides/testpmd_app_ug/run_app.rst | 101
> +++++++++++++++++++++++++++
>  6 files changed, 226 insertions(+), 46 deletions(-)
> 2.7.4

Many commands shouldn't be allowed but now only have a brief guide in doc. This can be done in the future I think.
Overall, it looks good to me.

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
  
Ajit Khaparde March 30, 2021, 3:11 a.m. UTC | #2
On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> From: Lijun Ou <oulijun@huawei.com>
>
> This patch adds multi-process support for testpmd.
> The test cmd example as follows:
> the primary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>
> the secondary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
Some minor nits below. Otherwise looks fine to me.
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
> v8:
> * Added warning info about queue numbers and process numbers.
>
:::snip::::

> +*   ``--rxq=N``
> +
> +    Set the number of Rx queues per port to N. N is the sum of queues used by primary
> +    and secondary process. Primary process and secondary process should have separate
> +    queues, and each should occupy at least one queue. Where N should be the multiple
> +    of number of processes.
of the number of processes.

> +
> +*   ``--txq=N``
> +
> +    Set the number of Tx queues per port to N. N is the sum of queues used by primary
> +    and secondary process. Primary process and secondary process should have separate
> +    queues, and each should occupy at least one queue. Where N should be the multiple
> +    of number of processes.
of the number of processes.

> +
> +*   ``--num-procs=N``
> +
> +    The number of processes which will be used.
> +
:::: snip ::::
> +The number of rings should be a multiple of the number of processes. If not,
> +redundant queues will exist after queues are allocated to processes. After RSS is
> +enabled, packet loss occurs when traffic is sent to all processes at the same time.
> +Some traffic enters redundant queues and cannot be forwarded.
> +
> +Most dev ops is supported in primary and secondary process. While secondary process
Most dev ops are supported in the primary and secondary process. While....

> +is not permitted to allocate or release shared memory, so some ops are not supported
> +as follows:
> +``dev_configure``
> +``dev_start``
> +``dev_stop``
> +``rx_queue_setup``
> +``tx_queue_setup``
> +``rx_queue_release``
> +``tx_queue_release``
:::: snip:::
  
humin (Q) March 30, 2021, 6:36 a.m. UTC | #3
在 2021/3/30 10:17, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> Sent: Tuesday, March 30, 2021 09:48
>> To: dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; ajit.khaparde@broadcom.com; Li,
>> Xiaoyun <xiaoyun.li@intel.com>
>> Subject: [PATCH v8] app/testpmd: support multi-process
>>
>> From: Lijun Ou <oulijun@huawei.com>
>>
>> This patch adds multi-process support for testpmd.
>> The test cmd example as follows:
>> the primary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>
>> the secondary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>> v8:
>> * Added warning info about queue numbers and process numbers.
>>
>> v7:
>> * Fixed compiling error for unexpected unindent.
>>
>> v6:
>> * Add rte flow description for multiple process.
>>
>> v5:
>> * Fixed run_app.rst for multiple process description.
>> * Fix compiling error.
>>
>> v4:
>> * Fixed minimum vlaue of Rxq or Txq in doc.
>>
>> v3:
>> * Fixed compiling error using gcc10.0.
>>
>> v2:
>> * Added document for this patch.
>> ---
>>   app/test-pmd/cmdline.c                |  12 +++-
>>   app/test-pmd/config.c                 |  14 +++-
>>   app/test-pmd/parameters.c             |  11 +++
>>   app/test-pmd/testpmd.c                | 127 ++++++++++++++++++++++------------
>>   app/test-pmd/testpmd.h                |   7 ++
>>   doc/guides/testpmd_app_ug/run_app.rst | 101
>> +++++++++++++++++++++++++++
>>   6 files changed, 226 insertions(+), 46 deletions(-)
>> 2.7.4
> 
> Many commands shouldn't be allowed but now only have a brief guide in doc. This can be done in the future I think.
> Overall, it looks good to me.
> 
Thanks xiaoyun.
You are right, Maybe there are more things to do about
supporting multiple process for testpmd. We will perfect
it continuously in future.
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>
  
humin (Q) March 30, 2021, 6:41 a.m. UTC | #4
在 2021/3/30 11:11, Ajit Khaparde 写道:
> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>>
>> From: Lijun Ou <oulijun@huawei.com>
>>
>> This patch adds multi-process support for testpmd.
>> The test cmd example as follows:
>> the primary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>
>> the secondary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Some minor nits below. Otherwise looks fine to me.
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
Thanks Ajit.

Hi, Ferruh,
	Should I send v9 to fix the grammar bugs in doc which Ajit point
out or fix it in future?

>> ---
>> v8:
>> * Added warning info about queue numbers and process numbers.
>>
> :::snip::::
> 
>> +*   ``--rxq=N``
>> +
>> +    Set the number of Rx queues per port to N. N is the sum of queues used by primary
>> +    and secondary process. Primary process and secondary process should have separate
>> +    queues, and each should occupy at least one queue. Where N should be the multiple
>> +    of number of processes.
> of the number of processes.
> 
>> +
>> +*   ``--txq=N``
>> +
>> +    Set the number of Tx queues per port to N. N is the sum of queues used by primary
>> +    and secondary process. Primary process and secondary process should have separate
>> +    queues, and each should occupy at least one queue. Where N should be the multiple
>> +    of number of processes.
> of the number of processes.
> 
>> +
>> +*   ``--num-procs=N``
>> +
>> +    The number of processes which will be used.
>> +
> :::: snip ::::
>> +The number of rings should be a multiple of the number of processes. If not,
>> +redundant queues will exist after queues are allocated to processes. After RSS is
>> +enabled, packet loss occurs when traffic is sent to all processes at the same time.
>> +Some traffic enters redundant queues and cannot be forwarded.
>> +
>> +Most dev ops is supported in primary and secondary process. While secondary process
> Most dev ops are supported in the primary and secondary process. While....
> 
>> +is not permitted to allocate or release shared memory, so some ops are not supported
>> +as follows:
>> +``dev_configure``
>> +``dev_start``
>> +``dev_stop``
>> +``rx_queue_setup``
>> +``tx_queue_setup``
>> +``rx_queue_release``
>> +``tx_queue_release``
> :::: snip:::
>
  
Ferruh Yigit March 30, 2021, 10:19 a.m. UTC | #5
On 3/30/2021 7:41 AM, Min Hu (Connor) wrote:
> 
> 
> 在 2021/3/30 11:11, Ajit Khaparde 写道:
>> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>>>
>>> From: Lijun Ou <oulijun@huawei.com>
>>>
>>> This patch adds multi-process support for testpmd.
>>> The test cmd example as follows:
>>> the primary cmd:
>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>
>>> the secondary cmd:
>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> Some minor nits below. Otherwise looks fine to me.
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>
> Thanks Ajit.
> 
> Hi, Ferruh,
>      Should I send v9 to fix the grammar bugs in doc which Ajit point
> out or fix it in future?
> 

Hi Connor, if they are only outstanding issues, I can fix them while merging.

>>> ---
>>> v8:
>>> * Added warning info about queue numbers and process numbers.
>>>
>> :::snip::::
>>
>>> +*   ``--rxq=N``
>>> +
>>> +    Set the number of Rx queues per port to N. N is the sum of queues used 
>>> by primary
>>> +    and secondary process. Primary process and secondary process should have 
>>> separate
>>> +    queues, and each should occupy at least one queue. Where N should be the 
>>> multiple
>>> +    of number of processes.
>> of the number of processes.
>>
>>> +
>>> +*   ``--txq=N``
>>> +
>>> +    Set the number of Tx queues per port to N. N is the sum of queues used 
>>> by primary
>>> +    and secondary process. Primary process and secondary process should have 
>>> separate
>>> +    queues, and each should occupy at least one queue. Where N should be the 
>>> multiple
>>> +    of number of processes.
>> of the number of processes.
>>
>>> +
>>> +*   ``--num-procs=N``
>>> +
>>> +    The number of processes which will be used.
>>> +
>> :::: snip ::::
>>> +The number of rings should be a multiple of the number of processes. If not,
>>> +redundant queues will exist after queues are allocated to processes. After 
>>> RSS is
>>> +enabled, packet loss occurs when traffic is sent to all processes at the 
>>> same time.
>>> +Some traffic enters redundant queues and cannot be forwarded.
>>> +
>>> +Most dev ops is supported in primary and secondary process. While secondary 
>>> process
>> Most dev ops are supported in the primary and secondary process. While....
>>
>>> +is not permitted to allocate or release shared memory, so some ops are not 
>>> supported
>>> +as follows:
>>> +``dev_configure``
>>> +``dev_start``
>>> +``dev_stop``
>>> +``rx_queue_setup``
>>> +``tx_queue_setup``
>>> +``rx_queue_release``
>>> +``tx_queue_release``
>> :::: snip:::
>>
  
humin (Q) March 30, 2021, 10:43 a.m. UTC | #6
在 2021/3/30 18:19, Ferruh Yigit 写道:
> On 3/30/2021 7:41 AM, Min Hu (Connor) wrote:
>>
>>
>> 在 2021/3/30 11:11, Ajit Khaparde 写道:
>>> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> 
>>> wrote:
>>>>
>>>> From: Lijun Ou <oulijun@huawei.com>
>>>>
>>>> This patch adds multi-process support for testpmd.
>>>> The test cmd example as follows:
>>>> the primary cmd:
>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>>
>>>> the secondary cmd:
>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>> Some minor nits below. Otherwise looks fine to me.
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>
>> Thanks Ajit.
>>
>> Hi, Ferruh,
>>      Should I send v9 to fix the grammar bugs in doc which Ajit point
>> out or fix it in future?
>>
> 
> Hi Connor, if they are only outstanding issues, I can fix them while 
> merging.
> 
Thanks Ferruh.
>>>> ---
>>>> v8:
>>>> * Added warning info about queue numbers and process numbers.
>>>>
>>> :::snip::::
>>>
>>>> +*   ``--rxq=N``
>>>> +
>>>> +    Set the number of Rx queues per port to N. N is the sum of 
>>>> queues used by primary
>>>> +    and secondary process. Primary process and secondary process 
>>>> should have separate
>>>> +    queues, and each should occupy at least one queue. Where N 
>>>> should be the multiple
>>>> +    of number of processes.
>>> of the number of processes.
>>>
>>>> +
>>>> +*   ``--txq=N``
>>>> +
>>>> +    Set the number of Tx queues per port to N. N is the sum of 
>>>> queues used by primary
>>>> +    and secondary process. Primary process and secondary process 
>>>> should have separate
>>>> +    queues, and each should occupy at least one queue. Where N 
>>>> should be the multiple
>>>> +    of number of processes.
>>> of the number of processes.
>>>
>>>> +
>>>> +*   ``--num-procs=N``
>>>> +
>>>> +    The number of processes which will be used.
>>>> +
>>> :::: snip ::::
>>>> +The number of rings should be a multiple of the number of 
>>>> processes. If not,
>>>> +redundant queues will exist after queues are allocated to 
>>>> processes. After RSS is
>>>> +enabled, packet loss occurs when traffic is sent to all processes 
>>>> at the same time.
>>>> +Some traffic enters redundant queues and cannot be forwarded.
>>>> +
>>>> +Most dev ops is supported in primary and secondary process. While 
>>>> secondary process
>>> Most dev ops are supported in the primary and secondary process. 
>>> While....
>>>
>>>> +is not permitted to allocate or release shared memory, so some ops 
>>>> are not supported
>>>> +as follows:
>>>> +``dev_configure``
>>>> +``dev_start``
>>>> +``dev_stop``
>>>> +``rx_queue_setup``
>>>> +``tx_queue_setup``
>>>> +``rx_queue_release``
>>>> +``tx_queue_release``
>>> :::: snip:::
>>>
> 
> .
  
humin (Q) April 8, 2021, 10:32 a.m. UTC | #7
Hi, Ferry and all,
	This patch has been acked:
	Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
	Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
	
	Are there any other comments about that patch?


在 2021/3/30 18:43, Min Hu (Connor) 写道:
> 
> 
> 在 2021/3/30 18:19, Ferruh Yigit 写道:
>> On 3/30/2021 7:41 AM, Min Hu (Connor) wrote:
>>>
>>>
>>> 在 2021/3/30 11:11, Ajit Khaparde 写道:
>>>> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> 
>>>> wrote:
>>>>>
>>>>> From: Lijun Ou <oulijun@huawei.com>
>>>>>
>>>>> This patch adds multi-process support for testpmd.
>>>>> The test cmd example as follows:
>>>>> the primary cmd:
>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>>>
>>>>> the secondary cmd:
>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>>>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>> Some minor nits below. Otherwise looks fine to me.
>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>
>>> Thanks Ajit.
>>>
>>> Hi, Ferruh,
>>>      Should I send v9 to fix the grammar bugs in doc which Ajit point
>>> out or fix it in future?
>>>
>>
>> Hi Connor, if they are only outstanding issues, I can fix them while 
>> merging.
>>
> Thanks Ferruh.
>>>>> ---
>>>>> v8:
>>>>> * Added warning info about queue numbers and process numbers.
>>>>>
>>>> :::snip::::
>>>>
>>>>> +*   ``--rxq=N``
>>>>> +
>>>>> +    Set the number of Rx queues per port to N. N is the sum of 
>>>>> queues used by primary
>>>>> +    and secondary process. Primary process and secondary process 
>>>>> should have separate
>>>>> +    queues, and each should occupy at least one queue. Where N 
>>>>> should be the multiple
>>>>> +    of number of processes.
>>>> of the number of processes.
>>>>
>>>>> +
>>>>> +*   ``--txq=N``
>>>>> +
>>>>> +    Set the number of Tx queues per port to N. N is the sum of 
>>>>> queues used by primary
>>>>> +    and secondary process. Primary process and secondary process 
>>>>> should have separate
>>>>> +    queues, and each should occupy at least one queue. Where N 
>>>>> should be the multiple
>>>>> +    of number of processes.
>>>> of the number of processes.
>>>>
>>>>> +
>>>>> +*   ``--num-procs=N``
>>>>> +
>>>>> +    The number of processes which will be used.
>>>>> +
>>>> :::: snip ::::
>>>>> +The number of rings should be a multiple of the number of 
>>>>> processes. If not,
>>>>> +redundant queues will exist after queues are allocated to 
>>>>> processes. After RSS is
>>>>> +enabled, packet loss occurs when traffic is sent to all processes 
>>>>> at the same time.
>>>>> +Some traffic enters redundant queues and cannot be forwarded.
>>>>> +
>>>>> +Most dev ops is supported in primary and secondary process. While 
>>>>> secondary process
>>>> Most dev ops are supported in the primary and secondary process. 
>>>> While....
>>>>
>>>>> +is not permitted to allocate or release shared memory, so some ops 
>>>>> are not supported
>>>>> +as follows:
>>>>> +``dev_configure``
>>>>> +``dev_start``
>>>>> +``dev_stop``
>>>>> +``rx_queue_setup``
>>>>> +``tx_queue_setup``
>>>>> +``rx_queue_release``
>>>>> +``tx_queue_release``
>>>> :::: snip:::
>>>>
>>
>> .
> .
  
Ferruh Yigit April 8, 2021, 1:27 p.m. UTC | #8
On 4/8/2021 11:32 AM, Min Hu (Connor) wrote:
> Hi, Ferry and all,
>      This patch has been acked:
>      Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>      Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
>      Are there any other comments about that patch?
> 

Hi Connor,

In today's release status meeting I asked if more parties can test this patch,
Lets wait a little more for it to be tested.

> 
> 在 2021/3/30 18:43, Min Hu (Connor) 写道:
>>
>>
>> 在 2021/3/30 18:19, Ferruh Yigit 写道:
>>> On 3/30/2021 7:41 AM, Min Hu (Connor) wrote:
>>>>
>>>>
>>>> 在 2021/3/30 11:11, Ajit Khaparde 写道:
>>>>> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>>>>>>
>>>>>> From: Lijun Ou <oulijun@huawei.com>
>>>>>>
>>>>>> This patch adds multi-process support for testpmd.
>>>>>> The test cmd example as follows:
>>>>>> the primary cmd:
>>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>>>>
>>>>>> the secondary cmd:
>>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>>>>
>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>>> Some minor nits below. Otherwise looks fine to me.
>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>
>>>> Thanks Ajit.
>>>>
>>>> Hi, Ferruh,
>>>>      Should I send v9 to fix the grammar bugs in doc which Ajit point
>>>> out or fix it in future?
>>>>
>>>
>>> Hi Connor, if they are only outstanding issues, I can fix them while merging.
>>>
>> Thanks Ferruh.
>>>>>> ---
>>>>>> v8:
>>>>>> * Added warning info about queue numbers and process numbers.
>>>>>>
>>>>> :::snip::::
>>>>>
>>>>>> +*   ``--rxq=N``
>>>>>> +
>>>>>> +    Set the number of Rx queues per port to N. N is the sum of queues 
>>>>>> used by primary
>>>>>> +    and secondary process. Primary process and secondary process should 
>>>>>> have separate
>>>>>> +    queues, and each should occupy at least one queue. Where N should be 
>>>>>> the multiple
>>>>>> +    of number of processes.
>>>>> of the number of processes.
>>>>>
>>>>>> +
>>>>>> +*   ``--txq=N``
>>>>>> +
>>>>>> +    Set the number of Tx queues per port to N. N is the sum of queues 
>>>>>> used by primary
>>>>>> +    and secondary process. Primary process and secondary process should 
>>>>>> have separate
>>>>>> +    queues, and each should occupy at least one queue. Where N should be 
>>>>>> the multiple
>>>>>> +    of number of processes.
>>>>> of the number of processes.
>>>>>
>>>>>> +
>>>>>> +*   ``--num-procs=N``
>>>>>> +
>>>>>> +    The number of processes which will be used.
>>>>>> +
>>>>> :::: snip ::::
>>>>>> +The number of rings should be a multiple of the number of processes. If not,
>>>>>> +redundant queues will exist after queues are allocated to processes. 
>>>>>> After RSS is
>>>>>> +enabled, packet loss occurs when traffic is sent to all processes at the 
>>>>>> same time.
>>>>>> +Some traffic enters redundant queues and cannot be forwarded.
>>>>>> +
>>>>>> +Most dev ops is supported in primary and secondary process. While 
>>>>>> secondary process
>>>>> Most dev ops are supported in the primary and secondary process. While....
>>>>>
>>>>>> +is not permitted to allocate or release shared memory, so some ops are 
>>>>>> not supported
>>>>>> +as follows:
>>>>>> +``dev_configure``
>>>>>> +``dev_start``
>>>>>> +``dev_stop``
>>>>>> +``rx_queue_setup``
>>>>>> +``tx_queue_setup``
>>>>>> +``rx_queue_release``
>>>>>> +``tx_queue_release``
>>>>> :::: snip:::
>>>>>
>>>
>>> .
>> .
  
humin (Q) April 9, 2021, 12:45 a.m. UTC | #9
在 2021/4/8 21:27, Ferruh Yigit 写道:
> On 4/8/2021 11:32 AM, Min Hu (Connor) wrote:
>> Hi, Ferry and all,
>>      This patch has been acked:
>>      Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>      Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>
>>      Are there any other comments about that patch?
>>
> 
> Hi Connor,
> 
> In today's release status meeting I asked if more parties can test this 
> patch,
> Lets wait a little more for it to be tested.
> 
OK, thanks Ferruh.
>>
>> 在 2021/3/30 18:43, Min Hu (Connor) 写道:
>>>
>>>
>>> 在 2021/3/30 18:19, Ferruh Yigit 写道:
>>>> On 3/30/2021 7:41 AM, Min Hu (Connor) wrote:
>>>>>
>>>>>
>>>>> 在 2021/3/30 11:11, Ajit Khaparde 写道:
>>>>>> On Mon, Mar 29, 2021 at 6:48 PM Min Hu (Connor) 
>>>>>> <humin29@huawei.com> wrote:
>>>>>>>
>>>>>>> From: Lijun Ou <oulijun@huawei.com>
>>>>>>>
>>>>>>> This patch adds multi-process support for testpmd.
>>>>>>> The test cmd example as follows:
>>>>>>> the primary cmd:
>>>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>>>>>
>>>>>>> the secondary cmd:
>>>>>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>>>>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>>>>>
>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>>>>>> Some minor nits below. Otherwise looks fine to me.
>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>>
>>>>> Thanks Ajit.
>>>>>
>>>>> Hi, Ferruh,
>>>>>      Should I send v9 to fix the grammar bugs in doc which Ajit point
>>>>> out or fix it in future?
>>>>>
>>>>
>>>> Hi Connor, if they are only outstanding issues, I can fix them while 
>>>> merging.
>>>>
>>> Thanks Ferruh.
>>>>>>> ---
>>>>>>> v8:
>>>>>>> * Added warning info about queue numbers and process numbers.
>>>>>>>
>>>>>> :::snip::::
>>>>>>
>>>>>>> +*   ``--rxq=N``
>>>>>>> +
>>>>>>> +    Set the number of Rx queues per port to N. N is the sum of 
>>>>>>> queues used by primary
>>>>>>> +    and secondary process. Primary process and secondary process 
>>>>>>> should have separate
>>>>>>> +    queues, and each should occupy at least one queue. Where N 
>>>>>>> should be the multiple
>>>>>>> +    of number of processes.
>>>>>> of the number of processes.
>>>>>>
>>>>>>> +
>>>>>>> +*   ``--txq=N``
>>>>>>> +
>>>>>>> +    Set the number of Tx queues per port to N. N is the sum of 
>>>>>>> queues used by primary
>>>>>>> +    and secondary process. Primary process and secondary process 
>>>>>>> should have separate
>>>>>>> +    queues, and each should occupy at least one queue. Where N 
>>>>>>> should be the multiple
>>>>>>> +    of number of processes.
>>>>>> of the number of processes.
>>>>>>
>>>>>>> +
>>>>>>> +*   ``--num-procs=N``
>>>>>>> +
>>>>>>> +    The number of processes which will be used.
>>>>>>> +
>>>>>> :::: snip ::::
>>>>>>> +The number of rings should be a multiple of the number of 
>>>>>>> processes. If not,
>>>>>>> +redundant queues will exist after queues are allocated to 
>>>>>>> processes. After RSS is
>>>>>>> +enabled, packet loss occurs when traffic is sent to all 
>>>>>>> processes at the same time.
>>>>>>> +Some traffic enters redundant queues and cannot be forwarded.
>>>>>>> +
>>>>>>> +Most dev ops is supported in primary and secondary process. 
>>>>>>> While secondary process
>>>>>> Most dev ops are supported in the primary and secondary process. 
>>>>>> While....
>>>>>>
>>>>>>> +is not permitted to allocate or release shared memory, so some 
>>>>>>> ops are not supported
>>>>>>> +as follows:
>>>>>>> +``dev_configure``
>>>>>>> +``dev_start``
>>>>>>> +``dev_stop``
>>>>>>> +``rx_queue_setup``
>>>>>>> +``tx_queue_setup``
>>>>>>> +``rx_queue_release``
>>>>>>> +``tx_queue_release``
>>>>>> :::: snip:::
>>>>>>
>>>>
>>>> .
>>> .
> 
> .
  
Ferruh Yigit April 12, 2021, 4:37 p.m. UTC | #10
On 3/30/2021 2:48 AM, Min Hu (Connor) wrote:
> From: Lijun Ou <oulijun@huawei.com>
> 
> This patch adds multi-process support for testpmd.
> The test cmd example as follows:
> the primary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
> 
> the secondary cmd:
> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
> 

Hi Connor,

Please find a few minor comments below, since they are minor issues please feel 
free to keep the existing acks in next version.

> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
> v8:
> * Added warning info about queue numbers and process numbers.
> 
> v7:
> * Fixed compiling error for unexpected unindent.
> 
> v6:
> * Add rte flow description for multiple process.
> 
> v5:
> * Fixed run_app.rst for multiple process description.
> * Fix compiling error.
> 
> v4:
> * Fixed minimum vlaue of Rxq or Txq in doc.
> 
> v3:
> * Fixed compiling error using gcc10.0.
> 
> v2:
> * Added document for this patch.
> ---
>   app/test-pmd/cmdline.c                |  12 +++-
>   app/test-pmd/config.c                 |  14 +++-
>   app/test-pmd/parameters.c             |  11 +++
>   app/test-pmd/testpmd.c                | 127 ++++++++++++++++++++++------------
>   app/test-pmd/testpmd.h                |   7 ++
>   doc/guides/testpmd_app_ug/run_app.rst | 101 +++++++++++++++++++++++++++

Can you please add a release notes update too?

>   6 files changed, 226 insertions(+), 46 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index f44116b..38c97e3 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -71,8 +71,6 @@
>   #include "cmdline_tm.h"
>   #include "bpf_cmd.h"
>   
> -static struct cmdline *testpmd_cl;
> -
>   static void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
>   
>   /* *** Help command with introduction. *** */
> @@ -5351,6 +5349,12 @@ cmd_set_flush_rx_parsed(void *parsed_result,
>   		__rte_unused void *data)
>   {
>   	struct cmd_set_flush_rx *res = parsed_result;
> +
> +	if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
> +		printf("multi-process doesn't support to flush rx queues.\n");
> +		return;
> +	}
> +
>   	no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
>   }
>   
> @@ -17229,6 +17233,10 @@ prompt(void)
>   		printf("Cannot set exit function for cmdline\n");
>   
>   	cmdline_interact(testpmd_cl);
> +	if (unlikely(f_quit == 1)) {
> +		dup2(testpmd_fd_copy, testpmd_cl->s_in);
> +		close(testpmd_fd_copy);
> +	}

Why this is needed? Can you put some comment on the code to explain why this is 
needed?

>   	if (ret != 0)
>   		cmdline_stdin_exit(testpmd_cl);
>   }
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index ef0b978..ec7d030 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2827,6 +2827,8 @@ rss_fwd_config_setup(void)
>   	queueid_t  rxq;
>   	queueid_t  nb_q;
>   	streamid_t  sm_id;
> +	int start;
> +	int end;
>   
>   	nb_q = nb_rxq;
>   	if (nb_q > nb_txq)
> @@ -2844,7 +2846,15 @@ rss_fwd_config_setup(void)
>   	init_fwd_streams();
>   
>   	setup_fwd_config_of_each_lcore(&cur_fwd_config);
> -	rxp = 0; rxq = 0;
> +
> +	if (proc_id > 0 && nb_q % num_procs)
> +		printf("Warning! queue numbers should be multiple of "
> +			"processes, or packet loss will happen.\n");
> +
> +	start = proc_id * nb_q / num_procs;
> +	end = start + nb_q / num_procs;
> +	rxp = 0;
> +	rxq = start;
>   	for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
>   		struct fwd_stream *fs;
>   
> @@ -2861,6 +2871,8 @@ rss_fwd_config_setup(void)
>   			continue;
>   		rxp = 0;
>   		rxq++;
> +		if (rxq >= end)
> +			rxq = start;
>   	}
>   }
>   
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index a326c8c..ec3bc62 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -45,6 +45,8 @@
>   #include <rte_flow.h>
>   
>   #include "testpmd.h"
> +#define PARAM_PROC_ID "proc-id"
> +#define PARAM_NUM_PROCS "num-procs"
>   

Macros just after include is not good, can you please move these just after 
'launch_args_parse()' to keep locality.

>   static void
>   usage(char* progname)
> @@ -644,6 +646,8 @@ launch_args_parse(int argc, char** argv)
>   		{ "rx-mq-mode",                 1, 0, 0 },
>   		{ "record-core-cycles",         0, 0, 0 },
>   		{ "record-burst-stats",         0, 0, 0 },
> +		{ PARAM_NUM_PROCS,              1, 0, 0 },
> +		{ PARAM_PROC_ID,                1, 0, 0 },
>   		{ 0, 0, 0, 0 },
>   	};
>   
> @@ -1410,6 +1414,13 @@ launch_args_parse(int argc, char** argv)
>   				record_core_cycles = 1;
>   			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
>   				record_burst_stats = 1;
> +
> +			if (strncmp(lgopts[opt_idx].name,
> +				    PARAM_NUM_PROCS, 8) == 0)

9?
"num-procs"
  123456789

> +				num_procs = atoi(optarg);
> +			if (strncmp(lgopts[opt_idx].name,
> +				    PARAM_PROC_ID, 7) == 0)
> +				proc_id = atoi(optarg);
>   			break;
>   		case 'h':
>   			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 96d2e0f..c31234e 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -63,6 +63,9 @@
>   
>   #include "testpmd.h"
>   
> +int testpmd_fd_copy; /* the copy of STDIN_FILENO */
> +struct cmdline *testpmd_cl;
> +

Again the global variables just after include is not good place.

What do you think to keep 'testpmd_cl' in the 'cmdline.c', since the object 
allocated there and the variable seems match more to that file. 'extern' already 
added to header, so it can be used here.

'testpmd_fd_copy' can be moved at the bottom of the block that has global 
variables, and please add more comment on what this variable is for.

>   #ifndef MAP_HUGETLB
>   /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
>   #define HUGE_FLAG (0x40000)
> @@ -125,6 +128,9 @@ uint8_t port_numa[RTE_MAX_ETHPORTS];
>    */
>   uint8_t rxring_numa[RTE_MAX_ETHPORTS];
>   
> +int proc_id;
> +unsigned int num_procs = 1;
> +

Similar comments for these global variables.

The testpmd is already complex, it is good to have multi process support for the 
testpmd but the concern is additional complexity it bring, it may lead very 
tangled senarious. Please add much more comment on the multi process related 
additions.

There is a block at the beggining of the this file, that has all global 
variables with enough comments on them, can we follow same for these new global 
variables.
Group all multiprocess related variables together, appent to the global 
variables block, and comment all the variables in detail?

>   /*
>    * Store specified sockets on which TX ring to be used by ports
>    * is allocated.
> @@ -977,6 +983,11 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>   	mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
>   	mbuf_poolname_build(socket_id, pool_name, sizeof(pool_name), size_idx);
>   
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		rte_mp = rte_mempool_lookup(pool_name);
> +		goto err;
> +	}
> +
>   	TESTPMD_LOG(INFO,
>   		"create a new mbuf pool <%s>: n=%u, size=%u, socket=%u\n",
>   		pool_name, nb_mbuf, mbuf_seg_size, socket_id);
> @@ -1059,9 +1070,14 @@ mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
>   
>   err:
>   	if (rte_mp == NULL) {
> -		rte_exit(EXIT_FAILURE,
> -			"Creation of mbuf pool for socket %u failed: %s\n",
> -			socket_id, rte_strerror(rte_errno));
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +			rte_exit(EXIT_FAILURE,
> +				"Creation of mbuf pool for socket %u failed: %s\n",
> +				socket_id, rte_strerror(rte_errno));
> +		else
> +			rte_exit(EXIT_FAILURE,
> +				"Get mbuf pool for socket %u failed: %s\n",
> +				socket_id, rte_strerror(rte_errno));
>   	} else if (verbose_level > 0) {
>   		rte_mempool_dump(stdout, rte_mp);
>   	}
> @@ -2002,6 +2018,12 @@ flush_fwd_rx_queues(void)
>   	uint64_t prev_tsc = 0, diff_tsc, cur_tsc, timer_tsc = 0;
>   	uint64_t timer_period;
>   
> +	if (num_procs > 1) {
> +		printf("multi-process not support for flushing fwd rx "
> +		       "queues, skip the below lines and return.\n");
> +		return;
> +	}
> +
>   	/* convert to number of cycles */
>   	timer_period = rte_get_timer_hz(); /* 1 second timeout */
>   
> @@ -2511,21 +2533,28 @@ start_port(portid_t pid)
>   				return -1;
>   			}
>   			/* configure port */
> -			diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
> +			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +				diag = rte_eth_dev_configure(pi,
> +						     nb_rxq + nb_hairpinq,
>   						     nb_txq + nb_hairpinq,
>   						     &(port->dev_conf));
> -			if (diag != 0) {
> -				if (rte_atomic16_cmpset(&(port->port_status),
> -				RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
> -					printf("Port %d can not be set back "
> -							"to stopped\n", pi);
> -				printf("Fail to configure port %d\n", pi);
> -				/* try to reconfigure port next time */
> -				port->need_reconfig = 1;
> -				return -1;
> +				if (diag != 0) {
> +					if (rte_atomic16_cmpset(
> +							&(port->port_status),
> +							RTE_PORT_HANDLING,
> +							RTE_PORT_STOPPED) == 0)
> +						printf("Port %d can not be set "
> +						       "back to stopped\n", pi);
> +					printf("Fail to configure port %d\n",
> +						pi);
> +					/* try to reconfigure port next time */
> +					port->need_reconfig = 1;
> +					return -1;
> +				}
>   			}
>   		}
> -		if (port->need_reconfig_queues > 0) {
> +		if (port->need_reconfig_queues > 0 &&
> +		    rte_eal_process_type() == RTE_PROC_PRIMARY) {
>   			port->need_reconfig_queues = 0;
>   			/* setup tx queues */
>   			for (qi = 0; qi < nb_txq; qi++) {
> @@ -2626,17 +2655,20 @@ start_port(portid_t pid)
>   		cnt_pi++;
>   
>   		/* start port */
> -		diag = rte_eth_dev_start(pi);
> -		if (diag < 0) {
> -			printf("Fail to start port %d: %s\n", pi,
> -			       rte_strerror(-diag));
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +			diag = rte_eth_dev_start(pi);
> +			if (diag < 0) {
> +				printf("Fail to start port %d: %s\n", pi,
> +				       rte_strerror(-diag));
>   
> -			/* Fail to setup rx queue, return */
> -			if (rte_atomic16_cmpset(&(port->port_status),
> +				/* Fail to setup rx queue, return */
> +				if (rte_atomic16_cmpset(&(port->port_status),
>   				RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
> -				printf("Port %d can not be set back to "
> -							"stopped\n", pi);
> -			continue;
> +					printf("Port %d can not be set back to "
> +								"stopped\n",
> +						pi);
> +				continue;
> +			}
>   		}
>   
>   		if (rte_atomic16_cmpset(&(port->port_status),
> @@ -2765,7 +2797,8 @@ stop_port(portid_t pid)
>   		if (port->flow_list)
>   			port_flow_flush(pi);
>   
> -		if (rte_eth_dev_stop(pi) != 0)
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
> +		    rte_eth_dev_stop(pi) != 0)
>   			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>   				pi);
>   
> @@ -2834,8 +2867,10 @@ close_port(portid_t pid)
>   			continue;
>   		}
>   
> -		port_flow_flush(pi);
> -		rte_eth_dev_close(pi);
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +			port_flow_flush(pi);
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +			rte_eth_dev_close(pi);

Can combine two calls to prevent redundant check.

>   	}
>   
>   	remove_invalid_ports();
> @@ -3099,7 +3134,7 @@ pmd_test_exit(void)
>   		}
>   	}
>   	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
> -		if (mempools[i])
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY && mempools[i])
>   			rte_mempool_free(mempools[i]);
>   	}
>   
> @@ -3621,6 +3656,10 @@ init_port_dcb_config(portid_t pid,
>   	int retval;
>   	uint16_t i;
>   
> +	if (num_procs > 1) {
> +		printf("The multi-process feature doesn't support dcb.\n");
> +		return -ENOTSUP;
> +	}
>   	rte_port = &ports[pid];
>   
>   	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
> @@ -3719,13 +3758,6 @@ init_port(void)
>   }
>   
>   static void
> -force_quit(void)
> -{
> -	pmd_test_exit();
> -	prompt_exit();
> -}
> -
> -static void
>   print_stats(void)
>   {
>   	uint8_t i;
> @@ -3756,12 +3788,16 @@ signal_handler(int signum)
>   		if (latencystats_enabled != 0)
>   			rte_latencystats_uninit();
>   #endif
> -		force_quit();
>   		/* Set flag to indicate the force termination. */
>   		f_quit = 1;
> -		/* exit with the expected status */
> -		signal(signum, SIG_DFL);
> -		kill(getpid(), signum);
> +		if (interactive == 1) {
> +			dup2(testpmd_cl->s_in, testpmd_fd_copy);
> +			close(testpmd_cl->s_in);
> +		} else {
> +			dup2(0, testpmd_fd_copy);
> +			close(0);
> +		}
> +

Similarly can you please comment the code why this is needed.

>   	}
>   }
>   
> @@ -3786,10 +3822,6 @@ main(int argc, char** argv)
>   		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
>   			 rte_strerror(rte_errno));
>   
> -	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> -		rte_exit(EXIT_FAILURE,
> -			 "Secondary process type not supported.\n");
> -
>   	ret = register_eth_event_callback();
>   	if (ret != 0)
>   		rte_exit(EXIT_FAILURE, "Cannot register for ethdev events");
> @@ -3885,8 +3917,10 @@ main(int argc, char** argv)
>   		}
>   	}
>   
> -	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
> +	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
> +		pmd_test_exit();
>   		rte_exit(EXIT_FAILURE, "Start ports failed\n");
> +	}
>   
>   	/* set all ports to promiscuous mode by default */
>   	RTE_ETH_FOREACH_DEV(port_id) {
> @@ -3932,6 +3966,8 @@ main(int argc, char** argv)
>   		}
>   		prompt();
>   		pmd_test_exit();
> +		if (unlikely(f_quit == 1))
> +			prompt_exit();

Why additional prompt_exit() is required?

>   	} else
>   #endif
>   	{
> @@ -3967,6 +4003,11 @@ main(int argc, char** argv)
>   		printf("Press enter to exit\n");
>   		rc = read(0, &c, 1);
>   		pmd_test_exit();
> +		if (unlikely(f_quit == 1)) {
> +			dup2(testpmd_fd_copy, 0);
> +			close(testpmd_fd_copy);
> +			prompt_exit();
> +		}

By definition 'f_quit', force quit, is added to support exiting in container 
environment, since they can't exit with SIGINT/SIGTERM, so main thing with this 
global variable should be breaking the loop in main function.
I wonder why this variable used extensively in this patch, is it used in 
different purpose here? will quit/Ctrl-C behaviour differ in multi process?

>   		if (rc < 0)
>   			return 1;
>   	}
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index a87ccb0..640a377 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -13,6 +13,7 @@
>   #include <rte_gso.h>
>   #include <cmdline.h>
>   #include <sys/queue.h>
> +#include  "cmdline.h"
>   
>   #define RTE_PORT_ALL            (~(portid_t)0x0)
>   
> @@ -24,6 +25,10 @@
>   #define RTE_PORT_CLOSED         (uint16_t)2
>   #define RTE_PORT_HANDLING       (uint16_t)3
>   
> +extern uint8_t f_quit;
> +extern int testpmd_fd_copy;
> +extern struct cmdline *testpmd_cl;
> +

Again can you please move them in the file, there is already a block in below 
for extern variables, please place there with sufficient comments.

>   /*
>    * It is used to allocate the memory for hash key.
>    * The hash key size is NIC dependent.
> @@ -423,6 +428,8 @@ extern uint64_t noisy_lkup_mem_sz;
>   extern uint64_t noisy_lkup_num_writes;
>   extern uint64_t noisy_lkup_num_reads;
>   extern uint64_t noisy_lkup_num_reads_writes;
> +extern int proc_id;
> +extern unsigned int num_procs;
>   

Can you pleaes group multi process related changes together?

>   extern uint8_t dcb_config;
>   extern uint8_t dcb_test;
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index ec1dc7d..7e60e80 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -551,3 +551,104 @@ The command line options are:
>       bit 1 - two hairpin ports paired
>       bit 0 - two hairpin ports loop
>       The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
> +
> +
> +Testpmd Support Multi Process Command-line Options
> +--------------------------------------------------
> +
> +The following are the command-line options for the testpmd applications(support
> +multi process).They must be separated from the EAL options, shown in the previous
> +section, with a ``--`` separator:
> +
> +.. code-block:: console
> +
> +	primary process:
> +	sudo ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i --rxq=4 --txq=4 \
> +        --num-procs=2 --proc-id=0
> +
> +	secondary process:
> +	sudo ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i --rxq=4 --txq=4 \
> +        --num-procs=2 --proc-id=1
> +
> +The command line options are:
> +
> +*   ``-a, --allow``
> +
> +    Add a device to the allow list. ``xxx`` means device used which should be the
> +    same in primary process and secondary process.
> +
> +*   ``--proc-type``
> +
> +    Specify a given process instance as the primary or secondary DPDK instance.
> +    ``auto`` set here is OK.
> +
> +*   ``-l CORELIST``
> +
> +    List of cores to run on. the corelist should be different in primary process and
> +    secondary process.
> +

+1 for documentation, but above are eal parameters, what about not repeting them 
here but referencing eal documentation, 
'doc/guides/linux_gsg/eal_args.include.rst', for it?

> +*   ``--rxq=N``
> +
> +    Set the number of Rx queues per port to N. N is the sum of queues used by primary
> +    and secondary process. Primary process and secondary process should have separate
> +    queues, and each should occupy at least one queue. Where N should be the multiple
> +    of number of processes.
> +
> +*   ``--txq=N``
> +
> +    Set the number of Tx queues per port to N. N is the sum of queues used by primary
> +    and secondary process. Primary process and secondary process should have separate
> +    queues, and each should occupy at least one queue. Where N should be the multiple
> +    of number of processes.
> +
> +*   ``--num-procs=N``
> +
> +    The number of processes which will be used.
> +
> +*   ``--proc-id=id``
> +
> +    The id of the current process (id < num-procs). id should be different in primary
> +    process and secondary process.
> +
> +Calculation rule for queue:
> +All queues are allocated to different processes based on proc_num and proc_id.
> +Calculation rule for the Testpmd to allocate queues to each process:
> +start(queue start id) = proc_id * nb_q / num_procs;
> +end(queue end id) = start + nb_q / num_procs;
> +
> +For example, if supports 4 txq and rxq
> +the 0~1 for primary process
> +the 2~3 for secondary process
> +
> +The number of rings should be a multiple of the number of processes. If not,
> +redundant queues will exist after queues are allocated to processes. After RSS is
> +enabled, packet loss occurs when traffic is sent to all processes at the same time.
> +Some traffic enters redundant queues and cannot be forwarded.
> +
> +Most dev ops is supported in primary and secondary process. While secondary process
> +is not permitted to allocate or release shared memory, so some ops are not supported
> +as follows:
> +``dev_configure``
> +``dev_start``
> +``dev_stop``
> +``rx_queue_setup``
> +``tx_queue_setup``
> +``rx_queue_release``
> +``tx_queue_release``
> +
> +So, any command from testpmd which calls those APIs will not be supported in secondary
> +process, like:
> +``port config all rxq|txq|rxd|txd <value>``
> +``port config <port_id> rx_offload xxx on/off ``
> +``port config <port_id> tx_offload xxx on/off``
> +etc.
> +
> +RTE_FLOW supported, it applies only on its own process on SW side, but all on HW size.
> +stats supported, stats will not change when one quit and start, As they share the same
> +buffer to store the stats. Flow rules are maintained in process level: primary and secondary
> +has its own flow list(but one flow list in HW). The two can see all the queues, so setting
> +the flow rules for the other is OK. Of course, io(receive or transmit packets) in the queue
> +from others is not permitted.
> +
> +RSS supported, Primary process and secondary process has separate queues to use, RSS
> +will work in their own queues whether primary and secondary process.
>
  
Ferruh Yigit April 15, 2021, 7:54 a.m. UTC | #11
On 4/12/2021 5:37 PM, Ferruh Yigit wrote:
> On 3/30/2021 2:48 AM, Min Hu (Connor) wrote:
>> From: Lijun Ou <oulijun@huawei.com>
>>
>> This patch adds multi-process support for testpmd.
>> The test cmd example as follows:
>> the primary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>
>> the secondary cmd:
>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>
> 
> Hi Connor,
> 
> Please find a few minor comments below, since they are minor issues please feel 
> free to keep the existing acks in next version.
> 

Reminder of this patch, waiting for minor updates, it would be good to have it 
before -rc1, so that the change can be tested thoroughly.

Meanwhile I have concern/question on long term testing of the feature. It is 
possible to break the multi-process support unintentionally and not recognize 
it. How can we continually verify the feature?

Are you using DTS?
Does it make sense to add a testcase into dts to test testpmd multi-process 
support so that it is verified on each release?
  
humin (Q) April 16, 2021, 2:20 a.m. UTC | #12
在 2021/4/15 15:54, Ferruh Yigit 写道:
> On 4/12/2021 5:37 PM, Ferruh Yigit wrote:
>> On 3/30/2021 2:48 AM, Min Hu (Connor) wrote:
>>> From: Lijun Ou <oulijun@huawei.com>
>>>
>>> This patch adds multi-process support for testpmd.
>>> The test cmd example as follows:
>>> the primary cmd:
>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=0
>>>
>>> the secondary cmd:
>>> ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
>>> --rxq=4 --txq=4 --num-procs=2 --proc-id=1
>>>
>>
>> Hi Connor,
>>
>> Please find a few minor comments below, since they are minor issues 
>> please feel free to keep the existing acks in next version.
>>
> 
> Reminder of this patch, waiting for minor updates, it would be good to 
> have it before -rc1, so that the change can be tested thoroughly.
> Hi, Ferruh,
	v9 has been sent, please check it out, thanks.
> Meanwhile I have concern/question on long term testing of the feature. 
> It is possible to break the multi-process support unintentionally and 
> not recognize it. How can we continually verify the feature?
> 
> Are you using DTS?
> Does it make sense to add a testcase into dts to test testpmd 
> multi-process support so that it is verified on each release?
> .
Well, we have our own DTS system which contains testcase of multiple
process in testpmd:
1. When we upstream patches to DPDK,the patch should pass all the testcase.
2. Before DPDK version release(in RC1) in every three months, we rebase
the new DPDK version, and check all the testcase in our DTS system. If
some testcase fails, we will fix it and upstream the patch to DPDK, of
course, including multi-process support in testpmd.
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f44116b..38c97e3 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -71,8 +71,6 @@ 
 #include "cmdline_tm.h"
 #include "bpf_cmd.h"
 
-static struct cmdline *testpmd_cl;
-
 static void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
 
 /* *** Help command with introduction. *** */
@@ -5351,6 +5349,12 @@  cmd_set_flush_rx_parsed(void *parsed_result,
 		__rte_unused void *data)
 {
 	struct cmd_set_flush_rx *res = parsed_result;
+
+	if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
+		printf("multi-process doesn't support to flush rx queues.\n");
+		return;
+	}
+
 	no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
 }
 
@@ -17229,6 +17233,10 @@  prompt(void)
 		printf("Cannot set exit function for cmdline\n");
 
 	cmdline_interact(testpmd_cl);
+	if (unlikely(f_quit == 1)) {
+		dup2(testpmd_fd_copy, testpmd_cl->s_in);
+		close(testpmd_fd_copy);
+	}
 	if (ret != 0)
 		cmdline_stdin_exit(testpmd_cl);
 }
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index ef0b978..ec7d030 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2827,6 +2827,8 @@  rss_fwd_config_setup(void)
 	queueid_t  rxq;
 	queueid_t  nb_q;
 	streamid_t  sm_id;
+	int start;
+	int end;
 
 	nb_q = nb_rxq;
 	if (nb_q > nb_txq)
@@ -2844,7 +2846,15 @@  rss_fwd_config_setup(void)
 	init_fwd_streams();
 
 	setup_fwd_config_of_each_lcore(&cur_fwd_config);
-	rxp = 0; rxq = 0;
+
+	if (proc_id > 0 && nb_q % num_procs)
+		printf("Warning! queue numbers should be multiple of "
+			"processes, or packet loss will happen.\n");
+
+	start = proc_id * nb_q / num_procs;
+	end = start + nb_q / num_procs;
+	rxp = 0;
+	rxq = start;
 	for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
 		struct fwd_stream *fs;
 
@@ -2861,6 +2871,8 @@  rss_fwd_config_setup(void)
 			continue;
 		rxp = 0;
 		rxq++;
+		if (rxq >= end)
+			rxq = start;
 	}
 }
 
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index a326c8c..ec3bc62 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -45,6 +45,8 @@ 
 #include <rte_flow.h>
 
 #include "testpmd.h"
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
 
 static void
 usage(char* progname)
@@ -644,6 +646,8 @@  launch_args_parse(int argc, char** argv)
 		{ "rx-mq-mode",                 1, 0, 0 },
 		{ "record-core-cycles",         0, 0, 0 },
 		{ "record-burst-stats",         0, 0, 0 },
+		{ PARAM_NUM_PROCS,              1, 0, 0 },
+		{ PARAM_PROC_ID,                1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1410,6 +1414,13 @@  launch_args_parse(int argc, char** argv)
 				record_core_cycles = 1;
 			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
 				record_burst_stats = 1;
+
+			if (strncmp(lgopts[opt_idx].name,
+				    PARAM_NUM_PROCS, 8) == 0)
+				num_procs = atoi(optarg);
+			if (strncmp(lgopts[opt_idx].name,
+				    PARAM_PROC_ID, 7) == 0)
+				proc_id = atoi(optarg);
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 96d2e0f..c31234e 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -63,6 +63,9 @@ 
 
 #include "testpmd.h"
 
+int testpmd_fd_copy; /* the copy of STDIN_FILENO */
+struct cmdline *testpmd_cl;
+
 #ifndef MAP_HUGETLB
 /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
 #define HUGE_FLAG (0x40000)
@@ -125,6 +128,9 @@  uint8_t port_numa[RTE_MAX_ETHPORTS];
  */
 uint8_t rxring_numa[RTE_MAX_ETHPORTS];
 
+int proc_id;
+unsigned int num_procs = 1;
+
 /*
  * Store specified sockets on which TX ring to be used by ports
  * is allocated.
@@ -977,6 +983,11 @@  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 	mb_size = sizeof(struct rte_mbuf) + mbuf_seg_size;
 	mbuf_poolname_build(socket_id, pool_name, sizeof(pool_name), size_idx);
 
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		rte_mp = rte_mempool_lookup(pool_name);
+		goto err;
+	}
+
 	TESTPMD_LOG(INFO,
 		"create a new mbuf pool <%s>: n=%u, size=%u, socket=%u\n",
 		pool_name, nb_mbuf, mbuf_seg_size, socket_id);
@@ -1059,9 +1070,14 @@  mbuf_pool_create(uint16_t mbuf_seg_size, unsigned nb_mbuf,
 
 err:
 	if (rte_mp == NULL) {
-		rte_exit(EXIT_FAILURE,
-			"Creation of mbuf pool for socket %u failed: %s\n",
-			socket_id, rte_strerror(rte_errno));
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			rte_exit(EXIT_FAILURE,
+				"Creation of mbuf pool for socket %u failed: %s\n",
+				socket_id, rte_strerror(rte_errno));
+		else
+			rte_exit(EXIT_FAILURE,
+				"Get mbuf pool for socket %u failed: %s\n",
+				socket_id, rte_strerror(rte_errno));
 	} else if (verbose_level > 0) {
 		rte_mempool_dump(stdout, rte_mp);
 	}
@@ -2002,6 +2018,12 @@  flush_fwd_rx_queues(void)
 	uint64_t prev_tsc = 0, diff_tsc, cur_tsc, timer_tsc = 0;
 	uint64_t timer_period;
 
+	if (num_procs > 1) {
+		printf("multi-process not support for flushing fwd rx "
+		       "queues, skip the below lines and return.\n");
+		return;
+	}
+
 	/* convert to number of cycles */
 	timer_period = rte_get_timer_hz(); /* 1 second timeout */
 
@@ -2511,21 +2533,28 @@  start_port(portid_t pid)
 				return -1;
 			}
 			/* configure port */
-			diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
+			if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+				diag = rte_eth_dev_configure(pi,
+						     nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
 						     &(port->dev_conf));
-			if (diag != 0) {
-				if (rte_atomic16_cmpset(&(port->port_status),
-				RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
-					printf("Port %d can not be set back "
-							"to stopped\n", pi);
-				printf("Fail to configure port %d\n", pi);
-				/* try to reconfigure port next time */
-				port->need_reconfig = 1;
-				return -1;
+				if (diag != 0) {
+					if (rte_atomic16_cmpset(
+							&(port->port_status),
+							RTE_PORT_HANDLING,
+							RTE_PORT_STOPPED) == 0)
+						printf("Port %d can not be set "
+						       "back to stopped\n", pi);
+					printf("Fail to configure port %d\n",
+						pi);
+					/* try to reconfigure port next time */
+					port->need_reconfig = 1;
+					return -1;
+				}
 			}
 		}
-		if (port->need_reconfig_queues > 0) {
+		if (port->need_reconfig_queues > 0 &&
+		    rte_eal_process_type() == RTE_PROC_PRIMARY) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
 			for (qi = 0; qi < nb_txq; qi++) {
@@ -2626,17 +2655,20 @@  start_port(portid_t pid)
 		cnt_pi++;
 
 		/* start port */
-		diag = rte_eth_dev_start(pi);
-		if (diag < 0) {
-			printf("Fail to start port %d: %s\n", pi,
-			       rte_strerror(-diag));
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			diag = rte_eth_dev_start(pi);
+			if (diag < 0) {
+				printf("Fail to start port %d: %s\n", pi,
+				       rte_strerror(-diag));
 
-			/* Fail to setup rx queue, return */
-			if (rte_atomic16_cmpset(&(port->port_status),
+				/* Fail to setup rx queue, return */
+				if (rte_atomic16_cmpset(&(port->port_status),
 				RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
-				printf("Port %d can not be set back to "
-							"stopped\n", pi);
-			continue;
+					printf("Port %d can not be set back to "
+								"stopped\n",
+						pi);
+				continue;
+			}
 		}
 
 		if (rte_atomic16_cmpset(&(port->port_status),
@@ -2765,7 +2797,8 @@  stop_port(portid_t pid)
 		if (port->flow_list)
 			port_flow_flush(pi);
 
-		if (rte_eth_dev_stop(pi) != 0)
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
+		    rte_eth_dev_stop(pi) != 0)
 			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
 				pi);
 
@@ -2834,8 +2867,10 @@  close_port(portid_t pid)
 			continue;
 		}
 
-		port_flow_flush(pi);
-		rte_eth_dev_close(pi);
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			port_flow_flush(pi);
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+			rte_eth_dev_close(pi);
 	}
 
 	remove_invalid_ports();
@@ -3099,7 +3134,7 @@  pmd_test_exit(void)
 		}
 	}
 	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
-		if (mempools[i])
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY && mempools[i])
 			rte_mempool_free(mempools[i]);
 	}
 
@@ -3621,6 +3656,10 @@  init_port_dcb_config(portid_t pid,
 	int retval;
 	uint16_t i;
 
+	if (num_procs > 1) {
+		printf("The multi-process feature doesn't support dcb.\n");
+		return -ENOTSUP;
+	}
 	rte_port = &ports[pid];
 
 	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
@@ -3719,13 +3758,6 @@  init_port(void)
 }
 
 static void
-force_quit(void)
-{
-	pmd_test_exit();
-	prompt_exit();
-}
-
-static void
 print_stats(void)
 {
 	uint8_t i;
@@ -3756,12 +3788,16 @@  signal_handler(int signum)
 		if (latencystats_enabled != 0)
 			rte_latencystats_uninit();
 #endif
-		force_quit();
 		/* Set flag to indicate the force termination. */
 		f_quit = 1;
-		/* exit with the expected status */
-		signal(signum, SIG_DFL);
-		kill(getpid(), signum);
+		if (interactive == 1) {
+			dup2(testpmd_cl->s_in, testpmd_fd_copy);
+			close(testpmd_cl->s_in);
+		} else {
+			dup2(0, testpmd_fd_copy);
+			close(0);
+		}
+
 	}
 }
 
@@ -3786,10 +3822,6 @@  main(int argc, char** argv)
 		rte_exit(EXIT_FAILURE, "Cannot init EAL: %s\n",
 			 rte_strerror(rte_errno));
 
-	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-		rte_exit(EXIT_FAILURE,
-			 "Secondary process type not supported.\n");
-
 	ret = register_eth_event_callback();
 	if (ret != 0)
 		rte_exit(EXIT_FAILURE, "Cannot register for ethdev events");
@@ -3885,8 +3917,10 @@  main(int argc, char** argv)
 		}
 	}
 
-	if (!no_device_start && start_port(RTE_PORT_ALL) != 0)
+	if (!no_device_start && start_port(RTE_PORT_ALL) != 0) {
+		pmd_test_exit();
 		rte_exit(EXIT_FAILURE, "Start ports failed\n");
+	}
 
 	/* set all ports to promiscuous mode by default */
 	RTE_ETH_FOREACH_DEV(port_id) {
@@ -3932,6 +3966,8 @@  main(int argc, char** argv)
 		}
 		prompt();
 		pmd_test_exit();
+		if (unlikely(f_quit == 1))
+			prompt_exit();
 	} else
 #endif
 	{
@@ -3967,6 +4003,11 @@  main(int argc, char** argv)
 		printf("Press enter to exit\n");
 		rc = read(0, &c, 1);
 		pmd_test_exit();
+		if (unlikely(f_quit == 1)) {
+			dup2(testpmd_fd_copy, 0);
+			close(testpmd_fd_copy);
+			prompt_exit();
+		}
 		if (rc < 0)
 			return 1;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a87ccb0..640a377 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -13,6 +13,7 @@ 
 #include <rte_gso.h>
 #include <cmdline.h>
 #include <sys/queue.h>
+#include  "cmdline.h"
 
 #define RTE_PORT_ALL            (~(portid_t)0x0)
 
@@ -24,6 +25,10 @@ 
 #define RTE_PORT_CLOSED         (uint16_t)2
 #define RTE_PORT_HANDLING       (uint16_t)3
 
+extern uint8_t f_quit;
+extern int testpmd_fd_copy;
+extern struct cmdline *testpmd_cl;
+
 /*
  * It is used to allocate the memory for hash key.
  * The hash key size is NIC dependent.
@@ -423,6 +428,8 @@  extern uint64_t noisy_lkup_mem_sz;
 extern uint64_t noisy_lkup_num_writes;
 extern uint64_t noisy_lkup_num_reads;
 extern uint64_t noisy_lkup_num_reads_writes;
+extern int proc_id;
+extern unsigned int num_procs;
 
 extern uint8_t dcb_config;
 extern uint8_t dcb_test;
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index ec1dc7d..7e60e80 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -551,3 +551,104 @@  The command line options are:
     bit 1 - two hairpin ports paired
     bit 0 - two hairpin ports loop
     The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
+
+
+Testpmd Support Multi Process Command-line Options
+--------------------------------------------------
+
+The following are the command-line options for the testpmd applications(support
+multi process).They must be separated from the EAL options, shown in the previous
+section, with a ``--`` separator:
+
+.. code-block:: console
+
+	primary process:
+	sudo ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i --rxq=4 --txq=4 \
+        --num-procs=2 --proc-id=0
+
+	secondary process:
+	sudo ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i --rxq=4 --txq=4 \
+        --num-procs=2 --proc-id=1
+
+The command line options are:
+
+*   ``-a, --allow``
+
+    Add a device to the allow list. ``xxx`` means device used which should be the
+    same in primary process and secondary process.
+
+*   ``--proc-type``
+
+    Specify a given process instance as the primary or secondary DPDK instance.
+    ``auto`` set here is OK.
+
+*   ``-l CORELIST``
+
+    List of cores to run on. the corelist should be different in primary process and
+    secondary process.
+
+*   ``--rxq=N``
+
+    Set the number of Rx queues per port to N. N is the sum of queues used by primary
+    and secondary process. Primary process and secondary process should have separate
+    queues, and each should occupy at least one queue. Where N should be the multiple
+    of number of processes.
+
+*   ``--txq=N``
+
+    Set the number of Tx queues per port to N. N is the sum of queues used by primary
+    and secondary process. Primary process and secondary process should have separate
+    queues, and each should occupy at least one queue. Where N should be the multiple
+    of number of processes.
+
+*   ``--num-procs=N``
+
+    The number of processes which will be used.
+
+*   ``--proc-id=id``
+
+    The id of the current process (id < num-procs). id should be different in primary
+    process and secondary process.
+
+Calculation rule for queue:
+All queues are allocated to different processes based on proc_num and proc_id.
+Calculation rule for the Testpmd to allocate queues to each process:
+start(queue start id) = proc_id * nb_q / num_procs;
+end(queue end id) = start + nb_q / num_procs;
+
+For example, if supports 4 txq and rxq
+the 0~1 for primary process
+the 2~3 for secondary process
+
+The number of rings should be a multiple of the number of processes. If not,
+redundant queues will exist after queues are allocated to processes. After RSS is
+enabled, packet loss occurs when traffic is sent to all processes at the same time.
+Some traffic enters redundant queues and cannot be forwarded.
+
+Most dev ops is supported in primary and secondary process. While secondary process
+is not permitted to allocate or release shared memory, so some ops are not supported
+as follows:
+``dev_configure``
+``dev_start``
+``dev_stop``
+``rx_queue_setup``
+``tx_queue_setup``
+``rx_queue_release``
+``tx_queue_release``
+
+So, any command from testpmd which calls those APIs will not be supported in secondary
+process, like:
+``port config all rxq|txq|rxd|txd <value>``
+``port config <port_id> rx_offload xxx on/off ``
+``port config <port_id> tx_offload xxx on/off``
+etc.
+
+RTE_FLOW supported, it applies only on its own process on SW side, but all on HW size.
+stats supported, stats will not change when one quit and start, As they share the same
+buffer to store the stats. Flow rules are maintained in process level: primary and secondary
+has its own flow list(but one flow list in HW). The two can see all the queues, so setting
+the flow rules for the other is OK. Of course, io(receive or transmit packets) in the queue
+from others is not permitted.
+
+RSS supported, Primary process and secondary process has separate queues to use, RSS
+will work in their own queues whether primary and secondary process.