From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id CD9AF2C50 for ; Thu, 20 Apr 2017 08:11:11 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Apr 2017 23:11:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,224,1488873600"; d="scan'208";a="1137973768" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 19 Apr 2017 23:11:07 -0700 Date: Thu, 20 Apr 2017 14:07:58 +0800 From: Yuanhan Liu To: "Zhao1, Wei" Cc: Konstantin Ananyev , "Mcnamara, John" , "dev@dpdk.org" , "Lu, Wenzhuo" , Thomas Monjalon , "Liu, Yu Y" Message-ID: <20170420060758.GA12462@yliu-dev.sh.intel.com> References: <1490866456-52241-1-git-send-email-wei.zhao1@intel.com> <1490866456-52241-2-git-send-email-wei.zhao1@intel.com> <2969664.trJitADFWx@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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: Thu, 20 Apr 2017 06:11:12 -0000 On Thu, Apr 06, 2017 at 02:57:29AM +0000, Zhao1, Wei wrote: > > > + * Reset an ethernet device when it's not working. One scenario is, > > > + after PF > > > + * port is down and up, the related VF port should be reset. > > > + * The API will stop the port, clear the rx/tx queues, re-setup the > > > + rx/tx > > > + * queues, restart the port. > > > > s/The API/This function/ > > > > 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? 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? 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(); } Assume the application already has a function (say, port_init()) to initiate a specific port, it then just needs do something like following to handle the case you described in the commit log: rte_eth_reset(port); port_init(port); Makes sense? Sorry it's completely wrong; I've limited knowledge on NIC pmd drivers after all :/ --yliu