From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 225EB9AE7 for ; Tue, 21 Jun 2016 11:26:16 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 21 Jun 2016 02:26:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,503,1459839600"; d="scan'208";a="1002164764" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by orsmga002.jf.intel.com with ESMTP; 21 Jun 2016 02:26:14 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.51]) by IRSMSX152.ger.corp.intel.com ([169.254.6.247]) with mapi id 14.03.0248.002; Tue, 21 Jun 2016 10:26:13 +0100 From: "Ananyev, Konstantin" To: Jerin Jacob , "Lu, Wenzhuo" CC: Stephen Hemminger , "dev@dpdk.org" , "Richardson, Bruce" , "Chen, Jing D" , "Liang, Cunming" , "Wu, Jingjing" , "Zhang, Helin" , "thomas.monjalon@6wind.com" Thread-Topic: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset Thread-Index: AQHRyrx6upx+ZylbjUKBoLEe8UVg7p/yAfqAgAB2MgCAAMH0gIAAJ/mAgAAXG4CAAA1AAIAACKQAgAAXzkA= Date: Tue, 21 Jun 2016 09:26:12 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B73FD0@irsmsx105.ger.corp.intel.com> References: <1465191653-28408-1-git-send-email-wenzhuo.lu@intel.com> <1466403870-6840-1-git-send-email-wenzhuo.lu@intel.com> <1466403870-6840-2-git-send-email-wenzhuo.lu@intel.com> <20160620091410.GA9323@localhost.localdomain> <20160620091714.276c186c@xeon-e3> <20160621035124.GC4903@localhost.localdomain> <6A0DE07E22DDAD4C9103DF62FEBC090903488DD1@shsmsx102.ccr.corp.intel.com> <20160621073710.GA30638@localhost.localdomain> <6A0DE07E22DDAD4C9103DF62FEBC090903488F5E@shsmsx102.ccr.corp.intel.com> <20160621085531.GA31880@localhost.localdomain> In-Reply-To: <20160621085531.GA31880@localhost.localdomain> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jun 2016 09:26:17 -0000 Hi Jerin, > -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Tuesday, June 21, 2016 9:56 AM > To: Lu, Wenzhuo > Cc: Stephen Hemminger; dev@dpdk.org; Ananyev, Konstantin; Richardson, Bru= ce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, > Helin; thomas.monjalon@6wind.com > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device r= eset >=20 > On Tue, Jun 21, 2016 at 08:24:36AM +0000, Lu, Wenzhuo wrote: > > Hi Jerin, >=20 > Hi Wenzhuo, >=20 > > > > > > > On Mon, Jun 20, 2016 at 02:24:27PM +0800, Wenzhuo Lu wrote: > > > > > > > > Add an API to reset the device. > > > > > > > > It's for VF device in this scenario, kernel PF + DPDK VF. > > > > > > > > When the PF port down->up, APP should call this API to rese= t > > > > > > > > VF port. Most likely, APP should call it in its management > > > > > > > > thread and guarantee the thread safe. It means APP should s= top > > > > > > > > the rx/tx and the device, then reset the device, then recov= er > > > > > > > > the device and rx/tx. > > > > > > > > > > > > > > Following is _a_ use-case for Device reset. But may be not be > > > > > > > _the_ use case. IMO, We need to first say expected behavior o= f > > > > > > > this API and add a use-case later. > > > > > > > > > > > > > > Other use-case would be, PCIe VF with functional level reset = for > > > > > > > SRIOV migration. > > > > > > > Are we on same page? > > > > > > > > > > > > > > > > > > In my experience with Linux devices, this is normally handled b= y > > > > > > the device driver in the start routine. Since any use case whi= ch > > > > > > needs this is going to do a stop/reset/start sequence, why not > > > > > > just have the VF device driver do this in the start routine?. > > > > > > > > > > > > Adding yet another API and state transistion if not necessary > > > > > > increases the complexity and required test cases for all device= s. > > > > > > > > > > I agree with Stephen here.I think if application needs to call st= art > > > > > after the device reset then we could add this logic in start itse= lf > > > > > rather exposing a yet another API > > > > Do you mean changing the device_start to include all these actions,= stop > > > device -> stop queue -> re-setup queue -> start queue -> start device= ? > > > > > > What was the expected API call sequence when you were introduced this= API? > > > > > > Point was to have implicit device reset in the API call sequence(Wher= ever make > > > sense for specific PMD) > > I think the API call sequence depends on the implementation of the APP.= Let's say if there's not this reset API, APP can use this API > call sequence to handle the PF link down/up event, rte_eth_dev_close -> r= te_eth_rx_queue_setup -> rte_eth_tx_queue_setup -> > rte_eth_dev_start. > > Actually our purpose is to use this reset API instead of the API call s= equence. You can see the reset API is not necessary. The benefit > is to save the code for APP. >=20 > Then I am bit confused with original commit log description. > | > |It means APP should stop the rx/tx and the device, then reset the > |device, then recover the device and rx/tx. > | > I was under impression that it a low level reset API for this device? Is > n't it? >=20 > The other issue is generalized outlook of the API, Certain PMD will not > have PF link down/up event? Link down/up and only connected to VF and PF > only for configuration. >=20 > How about fixing it more transparently in PMD driver itself as > PMD driver knows the PF link up/down event, Is it possible to > recover the VF on that event if its only matter of resetting it? I think we already went through that discussion on the list. Unfortunately with current dpdk design it is hardly possible. To achieve that we need to introduce some sort of synchronisation between IO and control APIs (locking or so). Actually I am not sure why having a special reset function will be a proble= m. Yes, it would exist only for VFs, for PF it could be left unimplemented. Though it definitely seems more convenient from user point of view, they would know: to handle VF reset event, they just need to call that particular function, not to re-implement their own. Konstantin >=20 > Jerin