From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH 04/22] ethdev: enable hotplug on multi-process Date: Tue, 19 Jun 2018 09:37:08 +0100 Message-ID: <2693ba60-83f0-7698-46b9-54e8ad2c49e3@intel.com> References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180607123849.14439-5-qi.z.zhang@intel.com> <3c273e40-9954-e9a3-2c78-9e245bacc19d@intel.com> <039ED4275CED7440929022BC67E7061153239FAD@SHSMSX103.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" To: "Zhang, Qi Z" , "thomas@monjalon.net" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 6F4AF2BDF for ; Tue, 19 Jun 2018 10:37:13 +0200 (CEST) In-Reply-To: <039ED4275CED7440929022BC67E7061153239FAD@SHSMSX103.ccr.corp.intel.com> Content-Language: en-US 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 19-Jun-18 4:22 AM, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Burakov, Anatoly >> Sent: Monday, June 18, 2018 4:18 PM >> To: Zhang, Qi Z ; thomas@monjalon.net >> Cc: Ananyev, Konstantin ; dev@dpdk.org; >> Richardson, Bruce ; Yigit, Ferruh >> ; Shelton, Benjamin H >> ; Vangati, Narender >> >> Subject: Re: [PATCH 04/22] ethdev: enable hotplug on multi-process >> >> On 07-Jun-18 1:38 PM, Qi Zhang wrote: >>> The patch introduce the solution to handle different hotplug cases in >>> multi-process situation, it include below scenario: >>> >>> 1. Attach a share device from primary >>> 2. Detach a share device from primary >>> 3. Attach a share device from secondary 4. Detach a share device from >>> secondary 5. Attach a private device from secondary 6. Detach a >>> private device from secondary 7. Detach a share device from secondary >>> privately 8. Attach a share device from secondary privately >>> >>> In primary-secondary process model, we assume device is shared by >> default. >>> that means attach or detach a device on any process will broadcast to >>> all other processes through mp channel then device information will be >>> synchronized on all processes. >>> >>> Any failure during attaching process will cause inconsistent status >>> between processes, so proper rollback action should be considered. >>> Also it is not safe to detach a share device when other process still >>> use it, so a handshake mechanism is introduced, it will be implemented >>> in following separate patch. >>> >>> Scenario for Case 1, 2: >>> >>> attach device >>> a) primary attach the new device if failed goto h). >>> b) primary send attach sync request to all secondary. >>> c) secondary receive request and attach device and send reply. >>> d) primary check the reply if all success go to i). >>> e) primary send attach rollback sync request to all secondary. >>> f) secondary receive the request and detach device and send reply. >>> g) primary receive the reply and detach device as rollback action. >>> h) attach fail >>> i) attach success >>> >>> detach device >>> a) primary perform pre-detach check, if device is locked, goto i). >>> b) primary send pre-detach sync request to all secondary. >>> c) secondary perform pre-detach check and send reply. >>> d) primary check the reply if any fail goto i). >>> e) primary send detach sync request to all secondary >>> f) secondary detach the device and send reply (assume no fail) >>> g) primary detach the device. >>> h) detach success >>> i) detach failed >>> >>> Case 3, 4: >>> This will be implemented in following patch. >> >> If these will be implemented in following patch, why spend half the commit >> message talking about it? :) > > Sorry, I didn't get your point about "see half commit to talk about it" :) > This patch covered an overview, and also the implementation of case 1,2,5,6,7,8 > > For case 3, 4, just below 4 lines to describe it > > 3. Attach a share device from secondary. > 4. Detach a share device from secondary. > Case 3, 4: > This will be implemented in following patch. > >> is commit doesn't implement secondary >> process functionality at all, so the commit message should probably be >> reworded to only include primary process logic, no? > > OK, I will reword it to highlight the patch's scope as description at above. Thanks! > > The return value of rte_eth_dev_request_to_primary only means communication fail, > (message not able to send, or not get reply in time). > but not the fail on attach/detach itself. (which comes from req->result) > Ah, yes, my apologies, you're right! The log message is fine then. >> >> Do we have to do a similar check for failsafe devices? > > Just keep it same logic as before, it could be a separate patch to fix I guess. Sure. >> Here and in other places: rte_strlcpy? > > OK Apologies, this should read strlcpy, not rte_strlcpy. -- Thanks, Anatoly