From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9170EA0C44; Mon, 12 Apr 2021 14:20:32 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 77CF6141178; Mon, 12 Apr 2021 14:20:32 +0200 (CEST) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mails.dpdk.org (Postfix) with ESMTP id 35A64141163 for ; Mon, 12 Apr 2021 14:20:30 +0200 (CEST) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4FJnmx24JXzPpwN; Mon, 12 Apr 2021 20:17:37 +0800 (CST) Received: from [10.78.49.194] (10.78.49.194) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.498.0; Mon, 12 Apr 2021 20:20:26 +0800 To: Ferruh Yigit , "Li, Xiaoyun" , "linuxarm@openeuler.org" , dev References: <1614939741-63927-1-git-send-email-oulijun@huawei.com> <1614939741-63927-2-git-send-email-oulijun@huawei.com> <2a0bee90-2c74-5f07-aaf0-cba8b94944e8@huawei.com> <918ea8b0-8d5d-ea2b-efce-7995d787b873@huawei.com> <8caa6ece-eec1-726f-da53-c99ea85aa1be@intel.com> <46eb78ec-f1ee-c411-3d00-031840e28d04@huawei.com> <5380fe49-4b31-b73f-5ef0-3751c1d353f7@intel.com> From: oulijun Message-ID: <089eecab-582b-38d4-c72d-2e1866753150@huawei.com> Date: Mon, 12 Apr 2021 20:20:26 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <5380fe49-4b31-b73f-5ef0-3751c1d353f7@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.78.49.194] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [Linuxarm] Re: [PATCH 1/3] app/testpmd: fix forwarding configuration when DCB test X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 在 2021/4/12 15:52, Ferruh Yigit 写道: > On 4/10/2021 3:58 AM, oulijun wrote: >> >> >> 在 2021/4/10 8:56, Ferruh Yigit 写道: >>> On 3/25/2021 2:21 AM, Li, Xiaoyun wrote: >>>> >>>> >>>>> -----Original Message----- >>>>> From: oulijun >>>>> Sent: Wednesday, March 24, 2021 21:40 >>>>> To: linuxarm@openeuler.org; Li, Xiaoyun ; dev >>>>> >>>>> 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 >>>>>>> Sent: Tuesday, March 23, 2021 22:19 >>>>>>> To: Li, Xiaoyun ; Yigit, Ferruh >>>>>>> >>>>>>> Cc: dev@dpdk.org; linuxarm@openeuler.org >>>>>>> Subject: Re: [PATCH 1/3] app/testpmd: fix forwarding configuration >>>>>>> when DCB test >>>>>>> >>>>>> >>>>>>>>> @@ -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. > > can 'dcb_config' be sufficient on its own for this? so that we can > remove 'dcb_test' and simplify the logic. > I agree with your proposal. Because the 'dcb_flag' field in struct rte_port indicates whether the current port is DCB, It is sufficient to have 'dcb_config' as a global variable to control the DCB test status. ok，I will send V2 to update all of the previous discussions. >>> 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@openeuler.org To unsubscribe >>>>>> send an >>>>>> email to linuxarm-leave@openeuler.org >>>>>> >>> >>> . >>> >> > > . >