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

[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.

> @@ -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).

> +.. 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?

> +*   ``--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?
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)?

> +
> +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?

> +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