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

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/17/2021 7:12 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>

Hi Connor,

I put some minor syntax comments, when they are fixed,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

<...>

>   			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);
> -				printf("Fail to configure port %d\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);

It is OK to have long line for the strings, so it can be like
printf("Port %d can not be set back to stopped\n",
	pi);

> +				printf("Fail to configure port %d\n",
> +					pi);

No need to update this line.

<...>

>   			/* Fail to setup rx queue, return */
>   			if (rte_atomic16_cmpset(&(port->port_status),
> -				RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
> +			RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
>   				printf("Port %d can not be set back to "
> -							"stopped\n", pi);
> +							"stopped\n",
> +					pi);

These are syntax changes, I think can keep these lines as it is.