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

[oulijun <oulijun@huawei.com>]

在 2021/4/12 18:35, Ferruh Yigit 写道:
> On 3/5/2021 10:22 AM, Lijun Ou wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> 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.
>>
>> 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.
>>
>> 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.
>>
> 
> Hi Lijun, Xiaoyun,
> 
> I read the thread and checked the code, but still I am not completely 
> clear with the dcp implementation in testpmd.
> 
> But the "dcb_config = 0" in the 'stop_port()' that has been removed in 
> the patch 1/3 seems preventing these kind of config combination, with 
> implicitly resetting the dcb feature with port stop.
> 
> What do you think just have the number of forwarding cores adjustment 
> change in the patch 1/3, and keep the 'stop_port()' as it is, will it 
> solve this problem, and how acceptable it will be do you think?
> 
I think it might be more appropriate to follow the discussion with Xiao 
Yun, since it does have problems.

In addition, I found another problem. Before calling 
dcb_fwd_config_setup(), testpmd must ensure that all ports have been 
configured with dcb.
Let's look at the next V3.
>> 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);
>>
> 
> .
>