From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 163F92C08 for ; Fri, 21 Apr 2017 10:59:20 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP; 21 Apr 2017 01:59:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,229,1488873600"; d="scan'208";a="77147783" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by orsmga002.jf.intel.com with ESMTP; 21 Apr 2017 01:59:18 -0700 Received: from pgsmsx103.gar.corp.intel.com ([169.254.2.175]) by PGSMSX102.gar.corp.intel.com ([169.254.6.18]) with mapi id 14.03.0319.002; Fri, 21 Apr 2017 16:59:17 +0800 From: "Zhao1, Wei" To: Thomas Monjalon , Yuanhan Liu CC: "Ananyev, Konstantin" , "Mcnamara, John" , "dev@dpdk.org" , "Lu, Wenzhuo" , "Liu, Yu Y" Thread-Topic: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset Thread-Index: AQHSqTm8K6Xc/Z+bh0WPnxNjoB7PqKGtRo6AgAplLjCAFbSeAIAAugdggACayYCAAGSYgIAAjhsA Date: Fri, 21 Apr 2017 08:59:17 +0000 Message-ID: References: <1490866456-52241-1-git-send-email-wei.zhao1@intel.com> <20170421022747.GA11512@yliu-dev.sh.intel.com> <1581788.2CBIGuQB5e@xps> In-Reply-To: <1581788.2CBIGuQB5e@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.30.20.205] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Apr 2017 08:59:21 -0000 Hi, thomas > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Friday, April 21, 2017 4:28 PM > To: Yuanhan Liu ; Zhao1, Wei > > Cc: Ananyev, Konstantin ; Mcnamara, John > ; dev@dpdk.org; Lu, Wenzhuo > ; Liu, Yu Y > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for = port > reset >=20 > 21/04/2017 04:27, Yuanhan Liu: > > On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote: > > > > > > Please explain exactly the responsibility of this function, > > > > > > and how it is different from calling stop/configure/start. > > > > > > > > > > In this reset feature, reset function can do the calling > > > > > stop/configure/start process, but also It can also do some > > > > > restore work for the port, for example, it can restore the added > > > > > parameters of > > > > > > > > vlan, mac_addrs, promisc_unicast_enabled falg and > > > > promisc_multicast_enabled flag. > > > > > > > > > Maybe , I should add this explanation in the patch comments or > > > > > function > > > > > > > > comments? > > > > > > > > I'm curious why we have to do save & restore for a reset operation. > > > > Why some configures have to be saved and restored? Doesn't "reset" > > > > literally means reset everything? > > > > > > Users maybe do not want to do a second configuration operation to > > > waste time after reset which lost all previous configuration. But he > > > still want these configuration valid after reset. > > > So, save & restore can help them to save this process time and effort= . > > > > > > > Even though, how do you tell what kind of configures need be > > > > restored and what should not? Again, even though, will all PMDs > > > > supports restoring the same set of configurations? > > > > > > Yes, this is hard to say what may be need and what may be not for use= r. > > > Now, the kinds of supported is list in patch set comment. And only > > > i40e NIC support this feature. > > Why it's the configurations listed in patch 2? Because they are > > requested by customers? > > > > Is that all could be saved? If not, are you going to save & restore > > all possible configurations? > > > > Assuming the configurations saved & restored may differ from different > > PMD drivers, how could you keep the consistency then? And judging that > > the application has no idea about the difference (yet it knows nothing > > about what kind of configurations will be saved), how could the > > application know that some configurations are not saved & restored by > > the driver that it has to do re-configuration by itself? > > > > > > While looking at your reset implementation for i40e, it looks more > > > > complex than necessary: just thinking we have to call > > > > "xxx_queue_setup" > > > > for all PMDs. > > > > > > > > I'm thinking a simple hardware reset might be enough? > > > > > > > > /* literally reset the hardware: reset everything */ > > > > rte_eth_reset(port) > > > > { > > > > > > > > eth_dev->ops->reset(); > > > > > > > > } > > > > > > You mean just do a reset and do not restore any configuration? > > > That may not meet the need for this feature from customer? > > > > Right, I'm just aware of the configuration might be done by PF (but > > not only by the application), that the VF port may be not aware of > > those configurations. So the save & restore is needed. I don't quite > > like how it is done in this patch set though. I also don't think the > > API is well named: as said, reset should literally reset everything. > > > > We may need think how to do it properly. > > > > Thomas, Konstantin, what do you guys think of it? >=20 > I have the same concerns. > I think we should better document the current status of start/stop and wh= ich > configuration parameters are lost or saved. Maybe I should add some words in doc\guides\nics\i40e.rst to Record which c= onfigurations are saved and restored by the PMD driver in reset function. Which not list in that are recognized as not saved and restored by defaul= t. OTHER NIC for this feature can add similar record in their xxx.rst.