From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <anatoly.burakov@intel.com>
Received: from mga12.intel.com (mga12.intel.com [192.55.52.136])
by dpdk.org (Postfix) with ESMTP id C87F01B96E
for <dev@dpdk.org>; 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" <qi.z.zhang@intel.com>,
"thomas@monjalon.net" <thomas@monjalon.net>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce"
<bruce.richardson@intel.com>, "Yigit, Ferruh" <ferruh.yigit@intel.com>,
"Shelton, Benjamin H" <benjamin.h.shelton@intel.com>,
"Vangati, Narender" <narender.vangati@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>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <qi.z.zhang@intel.com>; thomas@monjalon.net
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
>> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Shelton, Benjamin H
>> <benjamin.h.shelton@intel.com>; Vangati, Narender
>> <narender.vangati@intel.com>
>> 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 <qi.z.zhang@intel.com>
>>> ---
>>>
>>
>> <snip>
>>
>>> -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