[dpdk-dev] [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test

Ferruh Yigit ferruh.yigit at intel.com
Sat Apr 10 02:56:28 CEST 2021


On 3/25/2021 2:21 AM, Li, Xiaoyun wrote:
> 
> 
>> -----Original Message-----
>> From: oulijun <oulijun at huawei.com>
>> Sent: Wednesday, March 24, 2021 21:40
>> To: linuxarm at openeuler.org; Li, Xiaoyun <xiaoyun.li at intel.com>; dev
>> <dev at dpdk.org>
>> Subject: Re: [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding
>> configuration when DCB test
>>
>>
>>
>> 在 2021/3/24 10:03, Li, Xiaoyun 写道:
>>>
>>>
>>>> -----Original Message-----
>>>> From: oulijun <oulijun at huawei.com>
>>>> Sent: Tuesday, March 23, 2021 22:19
>>>> To: Li, Xiaoyun <xiaoyun.li at intel.com>; Yigit, Ferruh
>>>> <ferruh.yigit at intel.com>
>>>> Cc: dev at dpdk.org; linuxarm at openeuler.org
>>>> Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration
>>>> when DCB test
>>>>
>>> <snip>
>>>>>> @@ -2707,14 +2707,16 @@ stop_port(portid_t pid)
>>>>>>     	portid_t peer_pl[RTE_MAX_ETHPORTS];
>>>>>>     	int peer_pi;
>>>>>>
>>>>>> -	if (dcb_test) {
>>>>>> -		dcb_test = 0;
>>>>>> -		dcb_config = 0;
>>>>>> -	}
>>>>>> -
>>>>>>     	if (port_id_is_invalid(pid, ENABLED_WARN))
>>>>>>     		return;
>>>>>>
>>>>>> +	/*
>>>>>> +	 * In "start_port" function, dcb_test is set to 1 based on dcb_config.
>>>>>> +	 * So it should be cleared when dcb_config is 0.
>>>>>> +	 */
>>>>>> +	if (dcb_config == 0)
>>>>>> +		dcb_test = 0;
>>>>>> +
>>>>>
>>>>> I don't understand why are you changing this.
>>>>> dcb_test will only be set when dcb_config is 1 when starting ports.
>>>>> And both
>>>> dcb_test and dcb_config will be cleared when stopping ports.
>>>>> So dcb will only affect when you set port dcb and then start port
>>>>> and when
>>>> stop port dcb will be cleared.
>>>>>
>>>> Yes, I think. The forwarding streams should not be changed from
>>>> "dcb_fwd_config_setup" to "rss_fwd_config_setup" after dcb info is
>> configured.
>>>> But, now, the logical codes do it when stopping ports and then starting ports.
>>>>> So what's the problem of original codes?
>>>>>
>>>>> Your change will cause issues that there's no place to set
>>>>> dcb_config as 0. If
>>>> you config dcb, then it'll be always dcb mode unless restart the whole
>> testpmd.
>>>>>
>>>> As far as I know, the current testpmd only supports switching from
>>>> the normal mode to the dcb mode, but does not support the reverse
>> operation.
>>>> And " dcb_config" is set to 1, and then "dcb_test" is set to 1 after
>>>> config.
>>>
>>> You're not answering my questions. Why are you changing the behavior of
>> testpmd?
>>> Your change will make testpmd stay dcb mode once set dcb mode and can't go
>> back to normal mode. If users want to go back to normal mode, he/she has to
>> restart the whole testpmd.
>>>
>> Yes. Testpmd and PMD driver are both in dcb mode after dcb info is configured.
>> In my opinion, the 'dcb_test' flag can't be clear to go back to normal mode after
>> stopping port and then starting port. Because PMD driver is still dcb mode. If
>> users want to go back it, users should disable dcb mode and enable RSS or other
>> mode.
>>
>>> It worked as you can set dcb mode and start port. After stopping port, if you
>> still want dcb mode, you need to set dcb mode command again. But at least the
>> old way won't break anything.
>>> @Yigit, Ferruh Not sure which behavior is better, what do you think?
>>>
>>> And @oulijun can you just answer all comments in one thread?
>>>
>> After stopping port, the 'dcb_test' flag is clear. At this moment, the dcb
>> configuration in testpmd has not be changed.  users may not understand why
>> the DCB mode needs to be reconfigured.
> 
> OK. You're right. There's no place writing port->dcb_flag back to 0.
> 

If there is no way to turn off the DCB, isn't this code logically dead? This 
code is run when "dcb_config == 0" but in that case 'dcb_test' is already 0.
If so what is the point of the code, can it be removed?

btw, do you know what 'dcb_test' does at all? It seems it is set when 
'dcb_config' set, and reset when 'dcb_config' reset.

>>>>>>     	printf("Stopping ports...\n");
>>>>>>
>>>>>>     	RTE_ETH_FOREACH_DEV(pi) {
>>>>>> --
>>>>>> 2.7.4
>>>>>
>>>>> .
>>>>>
>>> _______________________________________________
>>> Linuxarm mailing list -- linuxarm at openeuler.org To unsubscribe send an
>>> email to linuxarm-leave at openeuler.org
>>>



More information about the dev mailing list