From mboxrd@z Thu Jan 1 00:00:00 1970 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) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Jun 2018 05:56:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,251,1526367600"; d="scan'208";a="66054821" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.4.182]) ([10.252.4.182]) by fmsmga001.fm.intel.com with ESMTP; 21 Jun 2018 05:56:33 -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> <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> From: "Burakov, Anatoly" Message-ID: <3599db6e-f76c-6534-632d-56859bd34cf2@intel.com> Date: Thu, 21 Jun 2018 13:56:32 +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: <039ED4275CED7440929022BC67E706115323B395@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 v2 06/22] ethdev: support attach or detach share device from secondary 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, 21 Jun 2018 12:56:37 -0000 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