From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 7ED1E9E3 for ; Fri, 21 Apr 2017 10:55:42 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Apr 2017 01:55:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,229,1488873600"; d="scan'208";a="92169639" Received: from pgsmsx106.gar.corp.intel.com ([10.221.44.98]) by fmsmga005.fm.intel.com with ESMTP; 21 Apr 2017 01:55:40 -0700 Received: from pgsmsx103.gar.corp.intel.com ([169.254.2.175]) by PGSMSX106.gar.corp.intel.com ([169.254.9.228]) with mapi id 14.03.0319.002; Fri, 21 Apr 2017 16:55:38 +0800 From: "Zhao1, Wei" To: Yuanhan Liu CC: "Ananyev, Konstantin" , "Mcnamara, John" , "dev@dpdk.org" , "Lu, Wenzhuo" , Thomas Monjalon , "Liu, Yu Y" Thread-Topic: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset Thread-Index: AQHSqTm8K6Xc/Z+bh0WPnxNjoB7PqKGtRo6AgAplLjCAFbSeAIAAugdggACayYCAAO+b0A== Date: Fri, 21 Apr 2017 08:55:38 +0000 Message-ID: References: <1490866456-52241-1-git-send-email-wei.zhao1@intel.com> <1490866456-52241-2-git-send-email-wei.zhao1@intel.com> <2969664.trJitADFWx@xps13> <20170420060758.GA12462@yliu-dev.sh.intel.com> <20170421022747.GA11512@yliu-dev.sh.intel.com> In-Reply-To: <20170421022747.GA11512@yliu-dev.sh.intel.com> 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:55:43 -0000 > -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > Sent: Friday, April 21, 2017 10:28 AM > To: Zhao1, Wei > Cc: Ananyev, Konstantin ; Mcnamara, John > ; dev@dpdk.org; Lu, Wenzhuo > ; Thomas Monjalon ; Liu, > Yu Y > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for = port > reset >=20 > 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 user. > > Now, the kinds of supported is list in patch set comment. And only i40e= NIC > support this feature. >=20 > Why it's the configurations listed in patch 2? Because they are requested= by > customers? >=20 > Is that all could be saved? If not, are you going to save & restore all p= ossible > configurations? >=20 > Assuming the configurations saved & restored may differ from different > PMD drivers, how could you keep the consistency then? And judging that th= e > application has no idea about the difference (yet it knows nothing about > what kind of configurations will be saved), how could the application kno= w > that some configurations are not saved & restored by the driver that it h= as to > do re-configuration by itself? >=20 Good idea, so maybe I should add some words in doc\guides\nics\i40e.rst to Record which configurations are saved and restored by the PMD driver in r= eset function. Which not list in that are recognized as not saved and restored by defaul= t. Is that ok ? > > > 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? >=20 > Right, I'm just aware of the configuration might be done by PF (but not o= nly > 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 i= t is > done in this patch set though. I also don't think the API is well named: = as said, > reset should literally reset everything. >=20 > We may need think how to do it properly. >=20 > Thomas, Konstantin, what do you guys think of it? So, your opinion is if it is named "reset", we had better do not do any res= tore work? If we keep this restore feature, we had better change function name? May be use the name "reset_and_restore" as function and feature name ? =20 >=20 > --yliu