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 54D6BA0524; Mon, 19 Apr 2021 15:42:34 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CF201412E6; Mon, 19 Apr 2021 15:42:33 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 07BEF4123B for ; Mon, 19 Apr 2021 15:42:31 +0200 (CEST) IronPort-SDR: VF5z6m5URT/V6+HB1n8njI0paB7kZj2KE1ARQndNOORXu8Afn93GiXCpiSb8cts+/F21O8/BKT 3T41Y0OSv0Rw== X-IronPort-AV: E=McAfee;i="6200,9189,9959"; a="195444026" X-IronPort-AV: E=Sophos;i="5.82,234,1613462400"; d="scan'208";a="195444026" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Apr 2021 06:42:30 -0700 IronPort-SDR: bXg8h7qyv037+Qhk8LlyqcKsw4SshZjFtwlKcfM0jlX6fdPYpYGeHhfoXBQXGpQI2MiR95YpOb xgw8FgpbL0+Q== X-IronPort-AV: E=Sophos;i="5.82,234,1613462400"; d="scan'208";a="462745484" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.220.224]) ([10.213.220.224]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Apr 2021 06:42:28 -0700 To: "Min Hu (Connor)" , dev@dpdk.org, John McNamara , Marko Kovacevic Cc: anatoly.burakov@intel.com, andrew.rybchenko@oktetlabs.ru, thomas@monjalon.net, xiaoyun.li@intel.com References: <1614906276-34293-1-git-send-email-oulijun@huawei.com> <1618794213-44460-1-git-send-email-humin29@huawei.com> From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Mon, 19 Apr 2021 14:42:25 +0100 MIME-Version: 1.0 In-Reply-To: <1618794213-44460-1-git-send-email-humin29@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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. > +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?