Re: [dpdk-dev] [PATCH 2/3] app/testpmd: remove forwarding config from parsing Rx and Tx

["Li, Xiaoyun" <xiaoyun.li@intel.com>]

Hi

> -----Original Message-----
> From: Lijun Ou <oulijun@huawei.com>
> Sent: Friday, March 5, 2021 18:22
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; dev@dpdk.org;
> linuxarm@openeuler.org
> Subject: [PATCH 2/3] app/testpmd: remove forwarding config from parsing Rx
> and Tx
> 
> From: Huisong Li <lihuisong@huawei.com>
> 

The commit message should be more simple and avoids grammar mistakes.

> The "fwd_config_setup()" function does release and apply for memory of
> forwarding flows, and re-establish these streams when rxq/txq or rxd/txd is
> changed. The function is also called by "start_packet_forwarding()" when user
> executes "start" cmd.
> All changes for rxq/txq or rxd/txd can be updated uniformly when this command
> is executed. Therefore, it is a little redundant in the "cmd_config_rx_tx_parsed"
> function.

It's not redundant. This command may configure number of rxq/txq. So the fwd streams map may change.
Then it's common to check the fwd streams after this command using "show config fwd".
If you remove this fwd stream update, users can't get the correct new fwd streams until they start the traffic.
But they may change a lot of things and want to check if the setting is correct before they start the traffic.

> 
> In addition, the forwarding stream under one TC is configured based on number
> of queues allocated to TC. And number of queues allocated to TC is updated
> after calling  "rte_eth_dev_configure"
> again. If the number of queues is reduced after configuring the DCB, and then,
> release and apply for stream memory, and reinitialize the forwarding stream
> under the DCB mode based on the old TC information. As a result, null pointer
> may be accessed.

I think you should add "rte_eth_dev_configure " into dcb_fwd_config_setup() before rte_eth_dev_get_dcb_info().

And the commit message should be similar like the following:
Segment fault might happen after configuring queue number to less because dcb_fwd_config_setup setup dcb based on old dcb info.
And dcb info can only update after rte_eth_dev_configure().
So this patch adds rte_eth_dev_configure() before rte_eth_dev_get_dcb_info() to get updated dcb info to fix this issue.

> 
> Like:
> set nbcore 4
> port stop all
> port config 0 dcb vt off 4 pfc on
> port start all
> port stop all
> port config all rxq 8
> port config all txq 8
> 
> At the moment, a segmentation fault occurs.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> ---
> V1->V2:
> - use stream instead of flow
> ---
>  app/test-pmd/cmdline.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 4df0c32..e316f5c 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1837,8 +1837,6 @@ cmd_config_rx_tx_parsed(void *parsed_result,
>  		return;
>  	}
> 
> -	fwd_config_setup();
> -
>  	init_port_config();
> 
>  	cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
> --
> 2.7.4