From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH v2 06/22] ethdev: support attach or detach share device from secondary Date: Thu, 21 Jun 2018 13:56:32 +0100 Message-ID: <3599db6e-f76c-6534-632d-56859bd34cf2@intel.com> References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180621020059.1198-1-qi.z.zhang@intel.com> <20180621020059.1198-7-qi.z.zhang@intel.com> <24f21c6f-e04c-0aa6-61fe-00893aa07a49@intel.com> <039ED4275CED7440929022BC67E706115323B395@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 mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id C87F01B96E for ; Thu, 21 Jun 2018 14:56:36 +0200 (CEST) In-Reply-To: <039ED4275CED7440929022BC67E706115323B395@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 21-Jun-18 1:50 PM, Zhang, Qi Z wrote: > > >> -----Original Message----- >> From: Burakov, Anatoly >> Sent: Thursday, June 21, 2018 5:06 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 v2 06/22] ethdev: support attach or detach share device >> from secondary >> >> On 21-Jun-18 3:00 AM, Qi Zhang wrote: >>> This patch cover the multi-process hotplug case when a share device >>> attach/detach request be issued from secondary process, the >>> implementation references malloc_mp.c. >>> >>> device attach on secondary: >>> a) secondary send async request to primary and wait on a condition >>> which will be released by matched response from primary. >>> b) primary receive the request and attach the new device if failed >>> goto i). >>> c) primary forward attach request to all secondary as async request >>> (because this in mp thread context, use sync request will >>> deadlock) >>> d) secondary receive request and attach device and send reply. >>> e) primary check the reply if all success go to j). >>> f) primary send attach rollback async request to all secondary. >>> g) secondary receive the request and detach device and send reply. >>> h) primary receive the reply and detach device as rollback action. >>> i) send fail response to secondary, goto k). >>> j) send success response to secondary. >>> k) secondary process receive response and return. >>> >>> device detach on secondary: >>> a) secondary send async request to primary and wait on a condition >>> which will be released by matched response from primary. >>> b) primary receive the request and perform pre-detach check, if device >>> is locked, goto j). >>> c) primary send pre-detach async request to all secondary. >>> d) secondary perform pre-detach check and send reply. >>> e) primary check the reply if any fail goto j). >>> f) primary send detach async request to all secondary >>> g) secondary detach the device and send reply >>> h) primary detach the device. >>> i) send success response to secondary, goto k). >>> j) send fail response to secondary. >>> k) secondary process receive response and return. >>> >>> Signed-off-by: Qi Zhang >>> --- >>> >> >> >> >>> -static int handle_secondary_request(const struct rte_mp_msg *msg, >>> const void *peer) >>> +static int >>> +check_reply(const struct eth_dev_mp_req *req, const struct >>> +rte_mp_reply *reply) { >>> + struct eth_dev_mp_req *resp; >>> + int i; >>> + >>> + if (reply->nb_received != reply->nb_sent) >>> + return -EINVAL; >>> + >>> + for (i = 0; i < reply->nb_received; i++) { >>> + resp = (struct eth_dev_mp_req *)reply->msgs[i].param; >>> + >>> + if (resp->t != req->t) { >>> + ethdev_log(ERR, "Unexpected response to async request\n"); >>> + return -EINVAL; >>> + } >>> + >>> + if (resp->id != req->id) { >>> + ethdev_log(ERR, "response to wrong async request\n"); >>> + return -EINVAL; >>> + } >>> + >>> + if (resp->result) >>> + return resp->result; >>> + } >>> + >>> + return 0; >>> +} >> >> As far as i understand, return values from this will propagate all the way up to >> user return value. > Yes >> How would a user differentiate between -EINVAL returned >> from invalid parameters, and -EINVAL from failed reply? > > My understanding is if > (resp->t != req->t) or (resp->id != req->id) is not expected to happen at any condition. > there should be a bug if it does happen. > So the return value is not necessary to be sensitive. > Am I right? You're right, it won't happen under normal conditions. However, on the off-chance that it does, the error return should still be meaningful. Under normal conditions, malloc() doesn't fail either :) -- Thanks, Anatoly