From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 514A9A0548; Wed, 21 Apr 2021 11:09:07 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD8C3419EC; Wed, 21 Apr 2021 11:09:06 +0200 (CEST) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id BB0E9419D3 for ; Wed, 21 Apr 2021 11:09:03 +0200 (CEST) Received: from DGGEMS406-HUB.china.huawei.com (unknown [172.30.72.59]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FQF6Q08NZz7wdS; Wed, 21 Apr 2021 17:06:38 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS406-HUB.china.huawei.com (10.3.19.206) with Microsoft SMTP Server id 14.3.498.0; Wed, 21 Apr 2021 17:08:57 +0800 To: Ferruh Yigit , , John McNamara , Marko Kovacevic CC: , , , References: <1614906276-34293-1-git-send-email-oulijun@huawei.com> <1618794213-44460-1-git-send-email-humin29@huawei.com> From: "Min Hu (Connor)" Message-ID: <1462881f-454c-60c4-da60-636bfb07b3ba@huawei.com> Date: Wed, 21 Apr 2021 17:08:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v11] app/testpmd: support multi-process X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Ferruh, 在 2021/4/19 21:42, Ferruh Yigit 写道: > On 4/19/2021 2:03 AM, Min Hu (Connor) wrote: >> 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, > > I put some comments on the documentation, can you please check them? > > Meanwhile, John, Marko can you please help on the doc review? > > Thanks, > ferruh > >> Signed-off-by: Min Hu (Connor) >> Signed-off-by: Lijun Ou >> Acked-by: Xiaoyun Li >> Acked-by: Ajit Khaparde >> Reviewed-by: Ferruh Yigit > > <...> > >> @@ -2626,16 +2701,16 @@ start_port(portid_t pid) >>           cnt_pi++; >>           /* start port */ >> -        diag = rte_eth_dev_start(pi); >> +        diag = eth_dev_start_mp(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), >> -                RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) >> -                printf("Port %d can not be set back to " >> -                            "stopped\n", pi); >> +            RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0) >> +                printf("Port %d can not be set back to stopped\n", >> +                    pi); > > Indentation is wrong. > > <...> > >> --- a/doc/guides/testpmd_app_ug/run_app.rst >> +++ b/doc/guides/testpmd_app_ug/run_app.rst >> @@ -543,3 +543,89 @@ 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 >> +-------------------------------------------------- > > What do you think making this a sub-section of the "Testpmd Command-line > Options"? > > And the section name can be "Testpmd Multi-Process Command-line Options" > >> + >> +The following are the command-line options for the testpmd >> applications(support > > Space before '('. > >> +multi process).They must be separated from the EAL options, shown in >> the previous >> +section, with a ``--`` separator: > > If this becomes sub-section of the "Testpmd Command-line Options", above > will become duplication, and I think it can be simplified as: > > "The following are the command-line options for testpmd multi-process > support:" > > >> + >> +.. 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: >> + >> +*   ``--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. >> + > > The famous question, does it start from '0' or '1'? > >> +Calculation rule for queue: >> +All queues are allocated to different processes based on proc_num and >> proc_id. > > Can highlight proc_num and proc_id, as ``proc_num`` and ``proc_id``. > >> +Calculation rule for the Testpmd to allocate queues to each process: > > Not sure if testpmd should start with uppercase. > >> +start(queue start id) = proc_id * nb_q / num_procs； >> +end(queue end id) = start + nb_q / num_procs； >> + > > Can you put above into a code-block. > Have already put above into a code-block in previous version. >> +For example, if supports 4 txq and rxq > > ".., if testpmp supports 4 Tx and Rx queues" > >> +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 > > "Most of the device operations are supported in ..."? > "While the 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`` >> + > > This list displayed as single line in the html output, if the intention > is to have them listed as been in the source, may need following update: > >  -as follows: >  +as follows:: >  + > >> +So, any command from testpmd which calls those APIs will not be >> supported in secondary >> +process, like: >> +``port config all rxq|txq|rxd|txd `` >> +``port config rx_offload xxx on/off `` >> +``port config tx_offload xxx on/off`` >> +etc. > > Same here for the list view. > >> + >> +RTE_FLOW supported, it applies only on its own process on SW side, >> but all on HW size. > > You may use "Flow API", instead of RTE_FLOW, so it becomes: "Flow API is > supported." > >> +stats supported, stats will not change when one quit and start, As >> they share the same >> +buffer to store the stats. > > The above stats related sentences looks like squeezed within the flow > API related description, it may be good to move it. > >> 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 > > Space before '('. > >> +the flow rules for the other is OK. Of course, io(receive or transmit >> packets) in the queue >> +from others is not permitted. > > I understand what you mean, but wording can be improved, can you please > try to reword it? > >> + >> +RSS supported, Primary process and secondary process has separate >> queues to use, RSS > > "RSS is supported"? And start 'primary' with lowercase. > >> +will work in their own queues whether primary and secondary process. >> > > There is a chance that when RSS is enabled, the packet may end up in > other process' queues? When RSS is enabled, packet could end up in other process's queues, but primary and secondary receives packerts from their own queues, and they do not interfere with each other, do not conflict with each other. > .