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 E1256A0C44; Mon, 12 Apr 2021 09:53:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6732340698; Mon, 12 Apr 2021 09:53:00 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 8C2664068E for ; Mon, 12 Apr 2021 09:52:58 +0200 (CEST) IronPort-SDR: PS12jA2U9M2ZUDTCinuHAOFPlB+n2yhjKrsDCbtORt7MPrCKDVroWi5EFASHOoqI3n3UhbqDMr Sv9JDg1B7TCg== X-IronPort-AV: E=McAfee;i="6000,8403,9951"; a="192001428" X-IronPort-AV: E=Sophos;i="5.82,214,1613462400"; d="scan'208";a="192001428" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Apr 2021 00:52:56 -0700 IronPort-SDR: xH2yQtT++0qMW1hVUTk952PElnKcEzh0qTC5CutorP94JA8UHyValCUJ5qkOvap/Lvs64rz+91 V8KfYZmxskYg== X-IronPort-AV: E=Sophos;i="5.82,214,1613462400"; d="scan'208";a="460067103" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.203.254]) ([10.213.203.254]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Apr 2021 00:52:53 -0700 To: oulijun , "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> From: Ferruh Yigit X-User: ferruhy Message-ID: <5380fe49-4b31-b73f-5ef0-3751c1d353f7@intel.com> Date: Mon, 12 Apr 2021 08:52:49 +0100 MIME-Version: 1.0 In-Reply-To: <46eb78ec-f1ee-c411-3d00-031840e28d04@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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. >> 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 >>>>> >> >> . >> >