[dpdk-dev] [PATCH v4] app/testpmd: support multi-process

[Min Hu (Connor)]

在 2021/3/24 16:08, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: dev <dev-bounces at dpdk.org> On Behalf Of Min Hu (Connor)
>> Sent: Monday, March 22, 2021 15:07
>> To: dev at dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit at intel.com>; ajit.khaparde at broadcom.com
>> Subject: [dpdk-dev] [PATCH v4] app/testpmd: support multi-process
>>
>> From: Lijun Ou <oulijun at 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 at huawei.com>
>> Signed-off-by: Lijun Ou <oulijun at huawei.com>
>> ---
>> 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                 |   9 ++-
>>   app/test-pmd/parameters.c             |  11 +++
>>   app/test-pmd/testpmd.c                | 138 ++++++++++++++++++++++------------
>>   app/test-pmd/testpmd.h                |   7 ++
>>   doc/guides/testpmd_app_ug/run_app.rst |  69 +++++++++++++++++
>>   6 files changed, 196 insertions(+), 50 deletions(-)
>>
> <snip>
>> +			if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>> +				rte_mp = rte_pktmbuf_pool_create(pool_name,
>> +					 nb_mbuf, mb_mempool_cache, 0,
>> +					 mbuf_seg_size, heap_socket);
>> +			else
>> +				rte_mp = rte_mempool_lookup(pool_name);
>> +
>>   			break;
>>   		}
>>   	case MP_ALLOC_XBUF:
> 
> What about this one when users use external bufs? Why not addressing secondary process here?
> If it works for all cases, you should add a condition at the start of this function, if it's secondary, goto err to check mp and return.
> 
Yes, your are right, I have fixed it in v5, thanks.
>> @@ -1994,6 +2013,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");
> 
> <snip>
>> +uint8_t f_quit;
>> +int testpmd_fd_copy;
>> +struct cmdline *testpmd_cl;
>> +
> 
> Please address the compilation failure on patchwork related to these variables (multiple definitions).
>
Done in v5.
>> +.. 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
>> +
> <snip>
>> +*   ``--rxq=N``
>> +
>> +    Set the number of RX queues per port to N, where 1 <= N <= 65535.
>> +    The default value is 1. N is the sum of queues used by primary and secondary
>> process.
>> +
> 
> Did you upstream wrong patch?
> You said you would address the queue number issue Ajit Khaparde mentioned but you didn't in this patch.
> The number of queues should be a multiple of the number of processes?
> 
Done in v5.
>> +*   ``--txq=N``
>> +
>> +    Set the number of TX queues per port to N, where 1 <= N <= 65535.
>> +    The default value is 1. N is the sum of queues used by primary and secondary
>> process.
>> +
> Same as above.
> 
>> +*   ``--num-procs=N``
> <snip>
>> +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_start``
>> +``dev_stop``
>> +``rx_queue_setup``
>> +``tx_queue_setup``
>> +``rx_queue_release``
>> +``tx_queue_release``
> 
> What about some config commands?
> Such as "clear port stats all". Should this be allowed by secondary?
 >
I think so, actually, all the queues is visible to primary and
secondary. The only thing we do is to separate queues for different
process for io (packets) in Rx/Tx. It is of for secondary "clear port
stats all".
> And like "port config all rxq". If primary hasn't started ports, should the secondary allowed to change traffic related stuff (offloads, rx/txd, rx/txq and so on)?
> 
Yes, port config all rxq/txq/rxd/txd/offload is not supported in the
secondary process. It has been done in v5.
>> +
>> +RTE_FLOW supported, it applies only on its own process on SW side, but all on
>> HW size.
> 
> About rte flow, what do you mean apply only on its own process on SW side?
> If I set number-procs=2, rxq=4
> Then on secondary process, I set a flow which directs 192.168.0.1 traffic to queue 0. It seems it will directs this kind of traffic to primary process. But I can't see this rule from primary process side.
> Is this behavior right for multiple process?
> 
According to doc rte_flow.rst, we maintain flow rules in process level:
primary and secondary has its own flow list(but one flow list in HW).
As previously mentioned, 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 in others is not
permitted.
>> +stats supported, stats will not change when one quit and start, As they share
>> the same buffer to store the stats.
>> +RSS supported, Primary process and secondary process has separate queues to
>> use, RSS will work in their own queues whether primary and secondary process.
>> --
>> 2.7.4
>