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

[oulijun]

在 2021/4/10 8:56, Ferruh Yigit 写道:
> 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?
>
yes. Currently, it is a dead logic unless testpmd adds support for 
switching from DCB mode to other modes.
I think dcb_test is a flag at the packet forwarding layer in testpmd. 
The original purpose is to indicate that the current forwarding test is 
being performed in DCB mode.
> 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.
>
In my opinion, it is only a global flag. During a DCB test, DCB must be 
configured on all forwarding ports.
>>>>>>>         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
>>>>
>
> .
>