DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
@ 2021-04-28 12:10 Singh, Aman Deep
  2021-04-29 10:57 ` Min Hu (Connor)
  0 siblings, 1 reply; 11+ messages in thread
From: Singh, Aman Deep @ 2021-04-28 12:10 UTC (permalink / raw)
  To: Min Hu (Connor, dev
  Cc: Mcnamara, John, Kovacevic, Marko, Burakov, Anatoly,
	andrew.rybchenko, thomas, Li, Xiaoyun, Yigit, Ferruh

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<mailto:humin29@huawei.com>>

Signed-off-by: Lijun Ou <oulijun@huawei.com<mailto:oulijun@huawei.com>>

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com<mailto:xiaoyun.li@intel.com>>

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com<mailto:ajit.khaparde@broadcom.com>>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>>

<snip>



Acked-by: Aman Deep Singh <aman.deep.singh@intel.com<mailto:aman.deep.singh@intel.com>>

Tested-by: Aman Deep Singh <aman.deep.singh@intel.com<mailto:aman.deep.singh@intel.com>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-04-28 12:10 [dpdk-dev] [PATCH v13] app/testpmd: support multi-process Singh, Aman Deep
@ 2021-04-29 10:57 ` Min Hu (Connor)
  2021-04-29 11:25   ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-29 10:57 UTC (permalink / raw)
  To: Singh, Aman Deep, dev
  Cc: Mcnamara, John, Kovacevic, Marko, Burakov, Anatoly,
	andrew.rybchenko, thomas, Li, Xiaoyun, Yigit, Ferruh

Thanks Aman.

Hi, Thomas, Ferruh, any other comments about this patch?

在 2021/4/28 20:10, Singh, Aman Deep 写道:
> 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 
> <mailto:humin29@huawei.com>>
> 
> Signed-off-by: Lijun Ou <oulijun@huawei.com <mailto:oulijun@huawei.com>>
> 
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com <mailto:xiaoyun.li@intel.com>>
> 
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com 
> <mailto:ajit.khaparde@broadcom.com>>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com 
> <mailto:ferruh.yigit@intel.com>>
> 
> <snip>
> 
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com 
> <mailto:aman.deep.singh@intel.com>>
> 
> Tested-by: Aman Deep Singh <aman.deep.singh@intel.com 
> <mailto:aman.deep.singh@intel.com>>
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-04-29 10:57 ` Min Hu (Connor)
@ 2021-04-29 11:25   ` Ferruh Yigit
  2021-04-29 11:34     ` Min Hu (Connor)
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-29 11:25 UTC (permalink / raw)
  To: Min Hu (Connor), Singh, Aman Deep, dev
  Cc: Mcnamara, John, Kovacevic, Marko, Burakov, Anatoly,
	andrew.rybchenko, thomas, Li, Xiaoyun

On 4/29/2021 11:57 AM, Min Hu (Connor) wrote:
> Thanks Aman.
> 
> Hi, Thomas, Ferruh, any other comments about this patch?
> 

Hi Connor,

There is no outstanding comments, also the patch has review/test tags in place.

But the multi-process support may affect many operations, and assuming it is not
critical for this release, to prevent any unexpected side affects I am for
getting this patch early next release instead of this release.

This gives more time in next release to address any unexpected issue.

Thanks,
ferruh

> 在 2021/4/28 20:10, Singh, Aman Deep 写道:
>> 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 <mailto:humin29@huawei.com>>
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com <mailto:oulijun@huawei.com>>
>>
>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com <mailto:xiaoyun.li@intel.com>>
>>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com
>> <mailto:ajit.khaparde@broadcom.com>>
>>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com
>> <mailto:ferruh.yigit@intel.com>>
>>
>> <snip>
>>
>> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com
>> <mailto:aman.deep.singh@intel.com>>
>>
>> Tested-by: Aman Deep Singh <aman.deep.singh@intel.com
>> <mailto:aman.deep.singh@intel.com>>
>>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-04-29 11:25   ` Ferruh Yigit
@ 2021-04-29 11:34     ` Min Hu (Connor)
  0 siblings, 0 replies; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-29 11:34 UTC (permalink / raw)
  To: Ferruh Yigit, Singh, Aman Deep, dev
  Cc: Mcnamara, John, Kovacevic, Marko, Burakov, Anatoly,
	andrew.rybchenko, thomas, Li, Xiaoyun

Got it, thanks Ferruh.

在 2021/4/29 19:25, Ferruh Yigit 写道:
> On 4/29/2021 11:57 AM, Min Hu (Connor) wrote:
>> Thanks Aman.
>>
>> Hi, Thomas, Ferruh, any other comments about this patch?
>>
> 
> Hi Connor,
> 
> There is no outstanding comments, also the patch has review/test tags in place.
> 
> But the multi-process support may affect many operations, and assuming it is not
> critical for this release, to prevent any unexpected side affects I am for
> getting this patch early next release instead of this release.
> 
> This gives more time in next release to address any unexpected issue.
>
> Thanks,
> ferruh
> 
>> 在 2021/4/28 20:10, Singh, Aman Deep 写道:
>>> 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 <mailto:humin29@huawei.com>>
>>>
>>> Signed-off-by: Lijun Ou <oulijun@huawei.com <mailto:oulijun@huawei.com>>
>>>
>>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com <mailto:xiaoyun.li@intel.com>>
>>>
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com
>>> <mailto:ajit.khaparde@broadcom.com>>
>>>
>>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com
>>> <mailto:ferruh.yigit@intel.com>>
>>>
>>> <snip>
>>>
>>> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com
>>> <mailto:aman.deep.singh@intel.com>>
>>>
>>> Tested-by: Aman Deep Singh <aman.deep.singh@intel.com
>>> <mailto:aman.deep.singh@intel.com>>
>>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-06-08  8:42   ` Andrew Rybchenko
  2021-06-08 10:22     ` Thomas Monjalon
@ 2021-06-15 12:04     ` Min Hu (Connor)
  1 sibling, 0 replies; 11+ messages in thread
From: Min Hu (Connor) @ 2021-06-15 12:04 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: ferruh.yigit, thomas, aman.deep.singh

Hi, Andrew,
	see replies below, and others without no reply will be fixed in v14.

在 2021/6/8 16:42, Andrew Rybchenko 写道:
> @Thomas, @Ferruh, please, see question below.
> 
> On 4/22/21 4:18 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
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> v13:
>> * Modified the doc syntax.
>>
>> v12:
>> * Updated doc info.
>>
>> v11:
>> * Fixed some minor syntax.
>>
>> v10:
>> * Hid process type checks behind new functions.
>> * Added comments.
>>
>> v9:
>> * Updated release notes and rst doc.
>> * Deleted deprecated codes.
>> * move macro and variable.
>>
>> 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                 |   6 ++
>>   app/test-pmd/config.c                  |  21 +++++-
>>   app/test-pmd/parameters.c              |  11 +++
>>   app/test-pmd/testpmd.c                 | 129 ++++++++++++++++++++++++++-------
>>   app/test-pmd/testpmd.h                 |   9 +++
>>   doc/guides/rel_notes/release_21_05.rst |   1 +
>>   doc/guides/testpmd_app_ug/run_app.rst  |  70 ++++++++++++++++++
>>   7 files changed, 220 insertions(+), 27 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 12efbc0..f0fa6e8 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -5450,6 +5450,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");
> 
> rx -> Rx
> 
>> +		return;
>> +	}
>> +
>>   	no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
>>   }
>>   
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index e189062..9eb1fa7 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -2971,6 +2971,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)
>> @@ -2988,7 +2990,22 @@ 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)
> 
> Please, compare result with 0 explicitly.
> 
>> +		printf("Warning! queue numbers should be multiple of "
>> +			"processes, or packet loss will happen.\n");
> 
> Do not split format string across multiple lines.
> 
> Frankly speaking I don't undertand why. Why is it impossible to
> serve 2 queues in the first process and 1 queue in the second
> process if 3 queues and 2 processes are configured.
> I think RSS redirection table can perfectly do it.
> 
Well, currently, my patch is one design implementation. I think
this can be done for later improvemnet.
>> +
>> +	/**
>> +	 * In multi-process, All queues are allocated to different
>> +	 * processes based on num_procs and proc_id. For example:
>> +	 * if supports 4 queues(nb_q), 2 processes(num_procs),
>> +	 * the 0~1 queue for primary process.
>> +	 * the 2~3 queue for secondary process.
>> +	 */
>> +	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;
>>   
>> @@ -3005,6 +3022,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 f3954c1..ece05c1 100644
>> --- a/app/test-pmd/parameters.c
>> +++ b/app/test-pmd/parameters.c
>> @@ -508,6 +508,9 @@ parse_link_speed(int n)
>>   void
>>   launch_args_parse(int argc, char** argv)
>>   {
>> +#define PARAM_PROC_ID "proc-id"
>> +#define PARAM_NUM_PROCS "num-procs"
>> +
>>   	int n, opt;
>>   	char **argvopt;
>>   	int opt_idx;
>> @@ -625,6 +628,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 },
>>   	};
>>   
>> @@ -1391,6 +1396,12 @@ 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, 9) == 0)
> 
> I strongly dislike 9 here and 7 below. Why is strncmp() used
> here, but just strcmp() is used for all other options.
> It makes the code inconsistent.
> 
>> +				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 d4be23f..afa2a6b 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -518,6 +518,62 @@ enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
>>    */
>>   uint32_t eth_link_speed;
>>   
>> +/*
>> + * Id of the current process in multi-process, used to
> 
> Id -> ID in accordance with devtools/words-case.txt
> 
>> + * configure the queues to be polled.
>> + */
>> +int proc_id;
>> +
>> +/*
>> + * Number of processes in multi-process, used to
>> + * configure the queues to be polled.
>> + */
>> +unsigned int num_procs = 1;
>> +
>> +static int
>> +eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>> +		      const struct rte_eth_conf *dev_conf)
>> +{
>> +	if (is_proc_primary())
>> +		return rte_eth_dev_configure(port_id, nb_rx_q, nb_tx_q,
>> +					dev_conf);
>> +	return 0;
>> +}
>> +
>> +static int
>> +eth_dev_start_mp(uint16_t port_id)
>> +{
>> +	if (is_proc_primary())
>> +		return rte_eth_dev_start(port_id);
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +eth_dev_stop_mp(uint16_t port_id)
>> +{
>> +	if (is_proc_primary())
>> +		return rte_eth_dev_stop(port_id);
>> +
>> +	return 0;
>> +}
>> +
>> +static void
>> +mempool_free_mp(struct rte_mempool *mp)
>> +{
>> +	if (is_proc_primary())
>> +		return rte_mempool_free(mp);
> 
> As far as I remember some compilers do not like it for void.
> Just remove 'return'.
> 
>> +}
>> +
>> +static int
>> +eth_dev_set_mtu_mp(uint16_t port_id, uint16_t mtu)
>> +{
>> +	if (is_proc_primary())
>> +		return rte_eth_dev_set_mtu(port_id, mtu);
>> +
>> +	return 0;
>> +}
>> +
>>   /* Forward function declarations */
>>   static void setup_attached_port(portid_t pi);
>>   static void check_all_ports_link_status(uint32_t port_mask);
>> @@ -977,6 +1033,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 (!is_proc_primary()) {
>> +		rte_mp = rte_mempool_lookup(pool_name);
>> +		goto err;
> 
> It looks like error path, but it works in the case of success
> as well. Looks confusing.
> 
>> +	}
>> +
>>   	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 +1120,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 (is_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 +2068,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 "
> 
> rx -> Rx
> also it is better to avoid like split.
> 
>> +		       "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 +2583,24 @@ start_port(portid_t pid)
>>   				return -1;
>>   			}
>>   			/* configure port */
>> -			diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
>> -						     nb_txq + nb_hairpinq,
>> -						     &(port->dev_conf));
>> +			diag = eth_dev_configure_mp(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);
>> +				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",
> 
> can not -> cannot (since you touch the line anyway)
> 
>> +						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 && is_proc_primary()) {
>>   			port->need_reconfig_queues = 0;
>>   			/* setup tx queues */
>>   			for (qi = 0; qi < nb_txq; qi++) {
>> @@ -2548,8 +2623,8 @@ start_port(portid_t pid)
>>   				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("Port %d can not be set back to stopped\n",
> 
> can not -> cannot
> 
>> +						pi);
>>   				printf("Fail to configure port %d tx queues\n",
>>   				       pi);
>>   				/* try to reconfigure queues next time */
>> @@ -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",
> 
> can not -> cannot
> 
>> +				       pi);
>>   			continue;
>>   		}
>>   
>> @@ -2765,7 +2840,7 @@ stop_port(portid_t pid)
>>   		if (port->flow_list)
>>   			port_flow_flush(pi);
>>   
>> -		if (rte_eth_dev_stop(pi) != 0)
>> +		if (eth_dev_stop_mp(pi) != 0)
>>   			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>>   				pi);
>>   
>> @@ -2834,8 +2909,10 @@ close_port(portid_t pid)
>>   			continue;
>>   		}
>>   
>> -		port_flow_flush(pi);
>> -		rte_eth_dev_close(pi);
>> +		if (is_proc_primary()) {
>> +			port_flow_flush(pi);
>> +			rte_eth_dev_close(pi);
>> +		}
>>   	}
>>   
>>   	remove_invalid_ports();
>> @@ -3101,7 +3178,7 @@ pmd_test_exit(void)
>>   	}
>>   	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
>>   		if (mempools[i])
>> -			rte_mempool_free(mempools[i]);
>> +			mempool_free_mp(mempools[i]);
>>   	}
>>   
>>   	printf("\nBye...\n");
>> @@ -3432,7 +3509,7 @@ update_jumbo_frame_offload(portid_t portid)
>>   	 * if unset do it here
>>   	 */
>>   	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
>> -		ret = rte_eth_dev_set_mtu(portid,
>> +		ret = eth_dev_set_mtu_mp(portid,
>>   				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
>>   		if (ret)
>>   			printf("Failed to set MTU to %u for port %u\n",
>> @@ -3622,6 +3699,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));
>> @@ -3787,10 +3868,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");
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 6ca872d..3318e8f 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -632,6 +632,15 @@ extern enum rte_eth_rx_mq_mode rx_mq_mode;
>>   
>>   extern struct rte_flow_action_conntrack conntrack_context;
>>   
>> +extern int proc_id;
>> +extern unsigned int num_procs;
>> +
>> +static inline bool
>> +is_proc_primary(void)
>> +{
>> +	return rte_eal_process_type() == RTE_PROC_PRIMARY;
>> +}
>> +
>>   static inline unsigned int
>>   lcore_num(void)
>>   {
>> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
>> index b36c120..9361332 100644
>> --- a/doc/guides/rel_notes/release_21_05.rst
>> +++ b/doc/guides/rel_notes/release_21_05.rst
>> @@ -235,6 +235,7 @@ New Features
>>       ``port cleanup (port_id) txq (queue_id) (free_cnt)``
>>     * Added command to show link flow control info.
>>       ``show port (port_id) flow_ctrl``
>> +  * Added support multi-process for testpmd.
> 
> Please, rebase to 21.08.
> 
>>   
>>   * **Updated ipsec-secgw sample application.**
>>   
>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
>> index d062165..74efa4f 100644
>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>> @@ -543,3 +543,73 @@ 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 Multi-Process Command-line Options
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +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
> 
> Do we really need --rxq=4 --txq=4 in secodary processes?
> Can't we get it from already configured device in primary
> process?
> 
> Same question is applicable to --num-procs. May be testpmd
> can publish in shared memory? If yes, I'm OK to either
> do it in the patch or defer as a later improvement.
> 
Yes, agree with you, it can be done for later improvement.
>> +
>> +The command line options are:
>> +
>> +*   ``--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, which starts from '0'.
>> +
>> +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 testpmd 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.
>> +
>> +All the dev ops is supported in primary 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``
> 
> @Thomas, @Ferrh, shouldn't it be handled on ethdev level as
> well if it is really that strict.Yes, API modification may be handled as another patch for later improvement.
> 
>> +
>> +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.
>> +
>> +Flow API is supported, it applies only on its own process on SW side, but all on HW side.
> 
> Sorry, I don't understand it.
This may be confusing, I will delete the lines.
What I mean is Flow API is supported,that is it.
> 
>> +
>> +Stats is 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. But in the testpmd primary process receiving or transmitting
>> +packets from the queue allocated for secondary process is not permitted, and same for
>> +secondary process.
>> +
>> +RSS is supported, primary process and secondary process has separate queues to use, RSS
>> +will work in their own queues whether primary or secondary process.
> 
> For me it sounds like secondary process has own RSS
> configuration. If so, it is false. I guess I simply
> misunderstand above paragraph.
This may be confusing, I will delete the lines.
What I mean is that primary process and secondary process has its
own queues, but both the queues are processed by RSS.
> .
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-06-08 10:39       ` Andrew Rybchenko
  2021-06-08 12:02         ` Thomas Monjalon
@ 2021-06-08 12:36         ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-06-08 12:36 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon, Min Hu (Connor)
  Cc: dev, aman.deep.singh, Anatoly Burakov

On 6/8/2021 11:39 AM, Andrew Rybchenko wrote:
> On 6/8/21 1:22 PM, Thomas Monjalon wrote:
>> 08/06/2021 10:42, Andrew Rybchenko:
>>> On 4/22/21 4:18 AM, Min Hu (Connor) wrote:
>>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>> [...]
>>>> +All the dev ops is supported in primary 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``
>>>
>>> @Thomas, @Ferrh, shouldn't it be handled on ethdev level as
>>> well if it is really that strict.
>>
>> Yes it should be documented at ethdev level, not testpmd.
>> I think it was kept fuzzy for too long.
> 
> To document is good, but I'm talking about more -
> add checks in corresponding API functions and
> return error.
> 

+Anatoly as multi process maintainer.

There are already some PMDs have these checks in the PMD level.

But current approach is providing more flexibility, if application takes care of
the synchronization, is there any reason some of above APIs can't be used by the
secondary process?

I am not sure if this flexibility is used or needed, if not needed +1 have the
checks in the ethdev layer and remove the PMD ones.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-06-08 10:39       ` Andrew Rybchenko
@ 2021-06-08 12:02         ` Thomas Monjalon
  2021-06-08 12:36         ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2021-06-08 12:02 UTC (permalink / raw)
  To: Min Hu (Connor), Andrew Rybchenko; +Cc: dev, ferruh.yigit, aman.deep.singh

08/06/2021 12:39, Andrew Rybchenko:
> On 6/8/21 1:22 PM, Thomas Monjalon wrote:
> > 08/06/2021 10:42, Andrew Rybchenko:
> >> On 4/22/21 4:18 AM, Min Hu (Connor) wrote:
> >>> --- a/doc/guides/testpmd_app_ug/run_app.rst
> >>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > [...]
> >>> +All the dev ops is supported in primary 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``
> >>
> >> @Thomas, @Ferrh, shouldn't it be handled on ethdev level as
> >> well if it is really that strict.
> > 
> > Yes it should be documented at ethdev level, not testpmd.
> > I think it was kept fuzzy for too long.
> 
> To document is good, but I'm talking about more -
> add checks in corresponding API functions and
> return error.

Yes I am OK with adding checks of the running process.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-06-08 10:22     ` Thomas Monjalon
@ 2021-06-08 10:39       ` Andrew Rybchenko
  2021-06-08 12:02         ` Thomas Monjalon
  2021-06-08 12:36         ` Ferruh Yigit
  0 siblings, 2 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2021-06-08 10:39 UTC (permalink / raw)
  To: Thomas Monjalon, Min Hu (Connor); +Cc: dev, ferruh.yigit, aman.deep.singh

On 6/8/21 1:22 PM, Thomas Monjalon wrote:
> 08/06/2021 10:42, Andrew Rybchenko:
>> On 4/22/21 4:18 AM, Min Hu (Connor) wrote:
>>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> [...]
>>> +All the dev ops is supported in primary 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``
>>
>> @Thomas, @Ferrh, shouldn't it be handled on ethdev level as
>> well if it is really that strict.
> 
> Yes it should be documented at ethdev level, not testpmd.
> I think it was kept fuzzy for too long.

To document is good, but I'm talking about more -
add checks in corresponding API functions and
return error.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-06-08  8:42   ` Andrew Rybchenko
@ 2021-06-08 10:22     ` Thomas Monjalon
  2021-06-08 10:39       ` Andrew Rybchenko
  2021-06-15 12:04     ` Min Hu (Connor)
  1 sibling, 1 reply; 11+ messages in thread
From: Thomas Monjalon @ 2021-06-08 10:22 UTC (permalink / raw)
  To: Min Hu (Connor), Andrew Rybchenko; +Cc: dev, ferruh.yigit, aman.deep.singh

08/06/2021 10:42, Andrew Rybchenko:
> On 4/22/21 4:18 AM, Min Hu (Connor) wrote:
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
[...]
> > +All the dev ops is supported in primary 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``
> 
> @Thomas, @Ferrh, shouldn't it be handled on ethdev level as
> well if it is really that strict.

Yes it should be documented at ethdev level, not testpmd.
I think it was kept fuzzy for too long.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-04-22  1:18 ` [dpdk-dev] [PATCH v13] " Min Hu (Connor)
@ 2021-06-08  8:42   ` Andrew Rybchenko
  2021-06-08 10:22     ` Thomas Monjalon
  2021-06-15 12:04     ` Min Hu (Connor)
  0 siblings, 2 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2021-06-08  8:42 UTC (permalink / raw)
  To: Min Hu (Connor), dev; +Cc: ferruh.yigit, thomas, aman.deep.singh

@Thomas, @Ferruh, please, see question below.

On 4/22/21 4:18 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
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> v13:
> * Modified the doc syntax.
> 
> v12:
> * Updated doc info.
> 
> v11:
> * Fixed some minor syntax.
> 
> v10:
> * Hid process type checks behind new functions.
> * Added comments.
> 
> v9:
> * Updated release notes and rst doc.
> * Deleted deprecated codes.
> * move macro and variable.
> 
> 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                 |   6 ++
>  app/test-pmd/config.c                  |  21 +++++-
>  app/test-pmd/parameters.c              |  11 +++
>  app/test-pmd/testpmd.c                 | 129 ++++++++++++++++++++++++++-------
>  app/test-pmd/testpmd.h                 |   9 +++
>  doc/guides/rel_notes/release_21_05.rst |   1 +
>  doc/guides/testpmd_app_ug/run_app.rst  |  70 ++++++++++++++++++
>  7 files changed, 220 insertions(+), 27 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 12efbc0..f0fa6e8 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -5450,6 +5450,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");

rx -> Rx

> +		return;
> +	}
> +
>  	no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
>  }
>  
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index e189062..9eb1fa7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2971,6 +2971,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)
> @@ -2988,7 +2990,22 @@ 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)

Please, compare result with 0 explicitly.

> +		printf("Warning! queue numbers should be multiple of "
> +			"processes, or packet loss will happen.\n");

Do not split format string across multiple lines.

Frankly speaking I don't undertand why. Why is it impossible to
serve 2 queues in the first process and 1 queue in the second
process if 3 queues and 2 processes are configured.
I think RSS redirection table can perfectly do it.

> +
> +	/**
> +	 * In multi-process, All queues are allocated to different
> +	 * processes based on num_procs and proc_id. For example:
> +	 * if supports 4 queues(nb_q), 2 processes(num_procs),
> +	 * the 0~1 queue for primary process.
> +	 * the 2~3 queue for secondary process.
> +	 */
> +	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;
>  
> @@ -3005,6 +3022,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 f3954c1..ece05c1 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -508,6 +508,9 @@ parse_link_speed(int n)
>  void
>  launch_args_parse(int argc, char** argv)
>  {
> +#define PARAM_PROC_ID "proc-id"
> +#define PARAM_NUM_PROCS "num-procs"
> +
>  	int n, opt;
>  	char **argvopt;
>  	int opt_idx;
> @@ -625,6 +628,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 },
>  	};
>  
> @@ -1391,6 +1396,12 @@ 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, 9) == 0)

I strongly dislike 9 here and 7 below. Why is strncmp() used
here, but just strcmp() is used for all other options.
It makes the code inconsistent.

> +				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 d4be23f..afa2a6b 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -518,6 +518,62 @@ enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
>   */
>  uint32_t eth_link_speed;
>  
> +/*
> + * Id of the current process in multi-process, used to

Id -> ID in accordance with devtools/words-case.txt

> + * configure the queues to be polled.
> + */
> +int proc_id;
> +
> +/*
> + * Number of processes in multi-process, used to
> + * configure the queues to be polled.
> + */
> +unsigned int num_procs = 1;
> +
> +static int
> +eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
> +		      const struct rte_eth_conf *dev_conf)
> +{
> +	if (is_proc_primary())
> +		return rte_eth_dev_configure(port_id, nb_rx_q, nb_tx_q,
> +					dev_conf);
> +	return 0;
> +}
> +
> +static int
> +eth_dev_start_mp(uint16_t port_id)
> +{
> +	if (is_proc_primary())
> +		return rte_eth_dev_start(port_id);
> +
> +	return 0;
> +}
> +
> +static int
> +eth_dev_stop_mp(uint16_t port_id)
> +{
> +	if (is_proc_primary())
> +		return rte_eth_dev_stop(port_id);
> +
> +	return 0;
> +}
> +
> +static void
> +mempool_free_mp(struct rte_mempool *mp)
> +{
> +	if (is_proc_primary())
> +		return rte_mempool_free(mp);

As far as I remember some compilers do not like it for void.
Just remove 'return'.

> +}
> +
> +static int
> +eth_dev_set_mtu_mp(uint16_t port_id, uint16_t mtu)
> +{
> +	if (is_proc_primary())
> +		return rte_eth_dev_set_mtu(port_id, mtu);
> +
> +	return 0;
> +}
> +
>  /* Forward function declarations */
>  static void setup_attached_port(portid_t pi);
>  static void check_all_ports_link_status(uint32_t port_mask);
> @@ -977,6 +1033,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 (!is_proc_primary()) {
> +		rte_mp = rte_mempool_lookup(pool_name);
> +		goto err;

It looks like error path, but it works in the case of success
as well. Looks confusing.

> +	}
> +
>  	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 +1120,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 (is_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 +2068,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 "

rx -> Rx
also it is better to avoid like split.

> +		       "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 +2583,24 @@ start_port(portid_t pid)
>  				return -1;
>  			}
>  			/* configure port */
> -			diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
> -						     nb_txq + nb_hairpinq,
> -						     &(port->dev_conf));
> +			diag = eth_dev_configure_mp(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);
> +				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",

can not -> cannot (since you touch the line anyway)

> +						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 && is_proc_primary()) {
>  			port->need_reconfig_queues = 0;
>  			/* setup tx queues */
>  			for (qi = 0; qi < nb_txq; qi++) {
> @@ -2548,8 +2623,8 @@ start_port(portid_t pid)
>  				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("Port %d can not be set back to stopped\n",

can not -> cannot

> +						pi);
>  				printf("Fail to configure port %d tx queues\n",
>  				       pi);
>  				/* try to reconfigure queues next time */
> @@ -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",

can not -> cannot

> +				       pi);
>  			continue;
>  		}
>  
> @@ -2765,7 +2840,7 @@ stop_port(portid_t pid)
>  		if (port->flow_list)
>  			port_flow_flush(pi);
>  
> -		if (rte_eth_dev_stop(pi) != 0)
> +		if (eth_dev_stop_mp(pi) != 0)
>  			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>  				pi);
>  
> @@ -2834,8 +2909,10 @@ close_port(portid_t pid)
>  			continue;
>  		}
>  
> -		port_flow_flush(pi);
> -		rte_eth_dev_close(pi);
> +		if (is_proc_primary()) {
> +			port_flow_flush(pi);
> +			rte_eth_dev_close(pi);
> +		}
>  	}
>  
>  	remove_invalid_ports();
> @@ -3101,7 +3178,7 @@ pmd_test_exit(void)
>  	}
>  	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
>  		if (mempools[i])
> -			rte_mempool_free(mempools[i]);
> +			mempool_free_mp(mempools[i]);
>  	}
>  
>  	printf("\nBye...\n");
> @@ -3432,7 +3509,7 @@ update_jumbo_frame_offload(portid_t portid)
>  	 * if unset do it here
>  	 */
>  	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> -		ret = rte_eth_dev_set_mtu(portid,
> +		ret = eth_dev_set_mtu_mp(portid,
>  				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
>  		if (ret)
>  			printf("Failed to set MTU to %u for port %u\n",
> @@ -3622,6 +3699,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));
> @@ -3787,10 +3868,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");
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 6ca872d..3318e8f 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -632,6 +632,15 @@ extern enum rte_eth_rx_mq_mode rx_mq_mode;
>  
>  extern struct rte_flow_action_conntrack conntrack_context;
>  
> +extern int proc_id;
> +extern unsigned int num_procs;
> +
> +static inline bool
> +is_proc_primary(void)
> +{
> +	return rte_eal_process_type() == RTE_PROC_PRIMARY;
> +}
> +
>  static inline unsigned int
>  lcore_num(void)
>  {
> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
> index b36c120..9361332 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -235,6 +235,7 @@ New Features
>      ``port cleanup (port_id) txq (queue_id) (free_cnt)``
>    * Added command to show link flow control info.
>      ``show port (port_id) flow_ctrl``
> +  * Added support multi-process for testpmd.

Please, rebase to 21.08.

>  
>  * **Updated ipsec-secgw sample application.**
>  
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index d062165..74efa4f 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -543,3 +543,73 @@ 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 Multi-Process Command-line Options
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +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

Do we really need --rxq=4 --txq=4 in secodary processes?
Can't we get it from already configured device in primary
process?

Same question is applicable to --num-procs. May be testpmd
can publish in shared memory? If yes, I'm OK to either
do it in the patch or defer as a later improvement.

> +
> +The command line options are:
> +
> +*   ``--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, which starts from '0'.
> +
> +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 testpmd 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.
> +
> +All the dev ops is supported in primary 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``

@Thomas, @Ferrh, shouldn't it be handled on ethdev level as
well if it is really that strict.

> +
> +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.
> +
> +Flow API is supported, it applies only on its own process on SW side, but all on HW side.

Sorry, I don't understand it.

> +
> +Stats is 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. But in the testpmd primary process receiving or transmitting
> +packets from the queue allocated for secondary process is not permitted, and same for
> +secondary process.
> +
> +RSS is supported, primary process and secondary process has separate queues to use, RSS
> +will work in their own queues whether primary or secondary process.

For me it sounds like secondary process has own RSS
configuration. If so, it is false. I guess I simply
misunderstand above paragraph.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v13] app/testpmd: support multi-process
  2021-03-05  1:04 [dpdk-dev] [PATCH] " Lijun Ou
@ 2021-04-22  1:18 ` Min Hu (Connor)
  2021-06-08  8:42   ` Andrew Rybchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Min Hu (Connor) @ 2021-04-22  1:18 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, thomas, aman.deep.singh

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>
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v13:
* Modified the doc syntax.

v12:
* Updated doc info.

v11:
* Fixed some minor syntax.

v10:
* Hid process type checks behind new functions.
* Added comments.

v9:
* Updated release notes and rst doc.
* Deleted deprecated codes.
* move macro and variable.

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                 |   6 ++
 app/test-pmd/config.c                  |  21 +++++-
 app/test-pmd/parameters.c              |  11 +++
 app/test-pmd/testpmd.c                 | 129 ++++++++++++++++++++++++++-------
 app/test-pmd/testpmd.h                 |   9 +++
 doc/guides/rel_notes/release_21_05.rst |   1 +
 doc/guides/testpmd_app_ug/run_app.rst  |  70 ++++++++++++++++++
 7 files changed, 220 insertions(+), 27 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 12efbc0..f0fa6e8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5450,6 +5450,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);
 }
 
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e189062..9eb1fa7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2971,6 +2971,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)
@@ -2988,7 +2990,22 @@ 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");
+
+	/**
+	 * In multi-process, All queues are allocated to different
+	 * processes based on num_procs and proc_id. For example:
+	 * if supports 4 queues(nb_q), 2 processes(num_procs),
+	 * the 0~1 queue for primary process.
+	 * the 2~3 queue for secondary process.
+	 */
+	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;
 
@@ -3005,6 +3022,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 f3954c1..ece05c1 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -508,6 +508,9 @@ parse_link_speed(int n)
 void
 launch_args_parse(int argc, char** argv)
 {
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
+
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
@@ -625,6 +628,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 },
 	};
 
@@ -1391,6 +1396,12 @@ 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, 9) == 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 d4be23f..afa2a6b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -518,6 +518,62 @@ enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
  */
 uint32_t eth_link_speed;
 
+/*
+ * Id of the current process in multi-process, used to
+ * configure the queues to be polled.
+ */
+int proc_id;
+
+/*
+ * Number of processes in multi-process, used to
+ * configure the queues to be polled.
+ */
+unsigned int num_procs = 1;
+
+static int
+eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
+		      const struct rte_eth_conf *dev_conf)
+{
+	if (is_proc_primary())
+		return rte_eth_dev_configure(port_id, nb_rx_q, nb_tx_q,
+					dev_conf);
+	return 0;
+}
+
+static int
+eth_dev_start_mp(uint16_t port_id)
+{
+	if (is_proc_primary())
+		return rte_eth_dev_start(port_id);
+
+	return 0;
+}
+
+static int
+eth_dev_stop_mp(uint16_t port_id)
+{
+	if (is_proc_primary())
+		return rte_eth_dev_stop(port_id);
+
+	return 0;
+}
+
+static void
+mempool_free_mp(struct rte_mempool *mp)
+{
+	if (is_proc_primary())
+		return rte_mempool_free(mp);
+}
+
+static int
+eth_dev_set_mtu_mp(uint16_t port_id, uint16_t mtu)
+{
+	if (is_proc_primary())
+		return rte_eth_dev_set_mtu(port_id, mtu);
+
+	return 0;
+}
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -977,6 +1033,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 (!is_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 +1120,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 (is_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 +2068,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 +2583,24 @@ start_port(portid_t pid)
 				return -1;
 			}
 			/* configure port */
-			diag = rte_eth_dev_configure(pi, nb_rxq + nb_hairpinq,
-						     nb_txq + nb_hairpinq,
-						     &(port->dev_conf));
+			diag = eth_dev_configure_mp(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);
+				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 && is_proc_primary()) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
 			for (qi = 0; qi < nb_txq; qi++) {
@@ -2548,8 +2623,8 @@ start_port(portid_t pid)
 				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("Port %d can not be set back to stopped\n",
+						pi);
 				printf("Fail to configure port %d tx queues\n",
 				       pi);
 				/* try to reconfigure queues next time */
@@ -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);
 			continue;
 		}
 
@@ -2765,7 +2840,7 @@ stop_port(portid_t pid)
 		if (port->flow_list)
 			port_flow_flush(pi);
 
-		if (rte_eth_dev_stop(pi) != 0)
+		if (eth_dev_stop_mp(pi) != 0)
 			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
 				pi);
 
@@ -2834,8 +2909,10 @@ close_port(portid_t pid)
 			continue;
 		}
 
-		port_flow_flush(pi);
-		rte_eth_dev_close(pi);
+		if (is_proc_primary()) {
+			port_flow_flush(pi);
+			rte_eth_dev_close(pi);
+		}
 	}
 
 	remove_invalid_ports();
@@ -3101,7 +3178,7 @@ pmd_test_exit(void)
 	}
 	for (i = 0 ; i < RTE_DIM(mempools) ; i++) {
 		if (mempools[i])
-			rte_mempool_free(mempools[i]);
+			mempool_free_mp(mempools[i]);
 	}
 
 	printf("\nBye...\n");
@@ -3432,7 +3509,7 @@ update_jumbo_frame_offload(portid_t portid)
 	 * if unset do it here
 	 */
 	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
-		ret = rte_eth_dev_set_mtu(portid,
+		ret = eth_dev_set_mtu_mp(portid,
 				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
 		if (ret)
 			printf("Failed to set MTU to %u for port %u\n",
@@ -3622,6 +3699,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));
@@ -3787,10 +3868,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");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 6ca872d..3318e8f 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -632,6 +632,15 @@ extern enum rte_eth_rx_mq_mode rx_mq_mode;
 
 extern struct rte_flow_action_conntrack conntrack_context;
 
+extern int proc_id;
+extern unsigned int num_procs;
+
+static inline bool
+is_proc_primary(void)
+{
+	return rte_eal_process_type() == RTE_PROC_PRIMARY;
+}
+
 static inline unsigned int
 lcore_num(void)
 {
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index b36c120..9361332 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -235,6 +235,7 @@ New Features
     ``port cleanup (port_id) txq (queue_id) (free_cnt)``
   * Added command to show link flow control info.
     ``show port (port_id) flow_ctrl``
+  * Added support multi-process for testpmd.
 
 * **Updated ipsec-secgw sample application.**
 
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index d062165..74efa4f 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -543,3 +543,73 @@ 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 Multi-Process Command-line Options
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+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:
+
+*   ``--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, which starts from '0'.
+
+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 testpmd 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.
+
+All the dev ops is supported in primary 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.
+
+Flow API is supported, it applies only on its own process on SW side, but all on HW side.
+
+Stats is 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. But in the testpmd primary process receiving or transmitting
+packets from the queue allocated for secondary process is not permitted, and same for
+secondary process.
+
+RSS is supported, primary process and secondary process has separate queues to use, RSS
+will work in their own queues whether primary or secondary process.
-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-15 12:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 12:10 [dpdk-dev] [PATCH v13] app/testpmd: support multi-process Singh, Aman Deep
2021-04-29 10:57 ` Min Hu (Connor)
2021-04-29 11:25   ` Ferruh Yigit
2021-04-29 11:34     ` Min Hu (Connor)
  -- strict thread matches above, loose matches on Subject: below --
2021-03-05  1:04 [dpdk-dev] [PATCH] " Lijun Ou
2021-04-22  1:18 ` [dpdk-dev] [PATCH v13] " Min Hu (Connor)
2021-06-08  8:42   ` Andrew Rybchenko
2021-06-08 10:22     ` Thomas Monjalon
2021-06-08 10:39       ` Andrew Rybchenko
2021-06-08 12:02         ` Thomas Monjalon
2021-06-08 12:36         ` Ferruh Yigit
2021-06-15 12:04     ` Min Hu (Connor)

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git