[2/3] app/testpmd: remove forwarding config from parsing Rx and Tx

  From: Huisong Li <lihuisong@huawei.com>

The "fwd_config_setup()" function does release and apply for
memory of forwarding flows, and re-establish these flows 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.

In addition, the forwarding flows 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 flow memory, and reinitialize
the forwarding flows under the DCB mode based on the old TC
information. As a result, null pointer may be accessed.

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>
---
 app/test-pmd/cmdline.c | 2 --
 1 file changed, 2 deletions(-)
  

Hi

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou
> Sent: Friday, March 5, 2021 08:56
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; linuxarm@openeuler.org
> Subject: [dpdk-dev] [PATCH 2/3] app/testpmd: remove forwarding config from
> parsing Rx and Tx
> 
> From: Huisong Li <lihuisong@huawei.com>
> 
> The "fwd_config_setup()" function does release and apply for memory of
> forwarding flows, and re-establish these flows 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.
> 
> In addition, the forwarding flows 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 flow memory, and reinitialize the forwarding flows under
> the DCB mode based on the old TC information. As a result, null pointer may be
> accessed.

The patch looks good to me. But in commit log, you mean forwarding streams, right? Flows will confuse to mean rte_flow.

> 
> 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>
> ---
>  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
> 832ae70..8b0f7d5 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
  

在 2021/3/5 11:21, Li, Xiaoyun 写道:
> Hi
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou
>> Sent: Friday, March 5, 2021 08:56
>> To: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Cc: dev@dpdk.org; linuxarm@openeuler.org
>> Subject: [dpdk-dev] [PATCH 2/3] app/testpmd: remove forwarding config from
>> parsing Rx and Tx
>>
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> The "fwd_config_setup()" function does release and apply for memory of
>> forwarding flows, and re-establish these flows 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.
>>
>> In addition, the forwarding flows 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 flow memory, and reinitialize the forwarding flows under
>> the DCB mode based on the old TC information. As a result, null pointer may be
>> accessed.
> 
> The patch looks good to me. But in commit log, you mean forwarding streams, right? Flows will confuse to mean rte_flow.
Yes. it is forwarding streams.
> 
>>
>> 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>
>> ---
>>   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
>> 832ae70..8b0f7d5 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
> 
> .
>