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 6F4AF2BDF for ; Tue, 19 Jun 2018 10:37:13 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Jun 2018 01:37:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,242,1526367600"; d="scan'208";a="65336366" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.26.59]) ([10.252.26.59]) by fmsmga001.fm.intel.com with ESMTP; 19 Jun 2018 01:37:09 -0700 To: "Zhang, Qi Z" , "thomas@monjalon.net" Cc: "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" 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> From: "Burakov, Anatoly" Message-ID: <2693ba60-83f0-7698-46b9-54e8ad2c49e3@intel.com> Date: Tue, 19 Jun 2018 09:37:08 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E7061153239FAD@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 04/22] ethdev: enable hotplug on multi-process 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: Tue, 19 Jun 2018 08:37:14 -0000 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