From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <jianfeng.tan@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
by dpdk.org (Postfix) with ESMTP id 145F11B2DB
for <dev@dpdk.org>; Tue, 16 Jan 2018 17:47:48 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
16 Jan 2018 08:47:48 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.46,369,1511856000"; d="scan'208";a="22793518"
Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.255.27.209])
([10.255.27.209])
by fmsmga001.fm.intel.com with ESMTP; 16 Jan 2018 08:47:46 -0800
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>, "Burakov, Anatoly" <anatoly.burakov@intel.com>
References: <1512067450-59203-1-git-send-email-jianfeng.tan@intel.com>
<1515643654-129489-1-git-send-email-jianfeng.tan@intel.com>
<1515643654-129489-4-git-send-email-jianfeng.tan@intel.com>
<2601191342CEEE43887BDE71AB9772588627E0E5@irsmsx105.ger.corp.intel.com>
<5733adcf-ef47-2c07-a39c-7eda01add6e0@intel.com>
<2601191342CEEE43887BDE71AB9772588627E2D8@irsmsx105.ger.corp.intel.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
"thomas@monjalon.net" <thomas@monjalon.net>
From: "Tan, Jianfeng" <jianfeng.tan@intel.com>
Message-ID: <f388e217-7acf-684a-dd98-cf33f421d2fb@intel.com>
Date: Wed, 17 Jan 2018 00:47:46 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101
Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <2601191342CEEE43887BDE71AB9772588627E2D8@irsmsx105.ger.corp.intel.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v2 3/4] eal: add synchronous multi-process
communication
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://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 16 Jan 2018 16:47:49 -0000
On 1/16/2018 7:12 PM, Ananyev, Konstantin wrote:
> Hi Jianfeng,
>
>> -----Original Message-----
>> From: Tan, Jianfeng
>> Sent: Tuesday, January 16, 2018 8:11 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; thomas@monjalon.net
>> Subject: Re: [PATCH v2 3/4] eal: add synchronous multi-process communication
>>
>> Thank you, Konstantin and Anatoly firstly. Other comments are well
>> received and I'll send out a new version.
>>
>>
>> On 1/16/2018 8:00 AM, Ananyev, Konstantin wrote:
>>>> We need the synchronous way for multi-process communication,
>>>> i.e., blockingly waiting for reply message when we send a request
>>>> to the peer process.
>>>>
>>>> We add two APIs rte_eal_mp_request() and rte_eal_mp_reply() for
>>>> such use case. By invoking rte_eal_mp_request(), a request message
>>>> is sent out, and then it waits there for a reply message. The
>>>> timeout is hard-coded 5 Sec. And the replied message will be copied
>>>> in the parameters of this API so that the caller can decide how
>>>> to translate those information (including params and fds). Note
>>>> if a primary process owns multiple secondary processes, this API
>>>> will fail.
>>>>
>>>> The API rte_eal_mp_reply() is always called by an mp action handler.
>>>> Here we add another parameter for rte_eal_mp_t so that the action
>>>> handler knows which peer address to reply.
>>>>
>>>> We use mutex in rte_eal_mp_request() to guarantee that only one
>>>> request is on the fly for one pair of processes.
>>> You don't need to do things in such strange and restrictive way.
>>> Instead you can do something like that:
>>> 1) Introduce new struct, list for it and mutex
>>> struct sync_request {
>>> int reply_received;
>>> char dst[PATH_MAX];
>>> char reply[...];
>>> LIST_ENTRY(sync_request) next;
>>> };
>>>
>>> static struct
>>> LIST_HEAD(list, sync_request);
>>> pthread_mutex_t lock;
>>> pthead_cond_t cond;
>>> } sync_requests;
>>>
>>> 2) then at request() call:
>>> Grab sync_requests.lock
>>> Check do we already have a pending request for that destination,
>>> If yes - the release the lock and returns with error.
>>> - allocate and init new sync_request struct, set reply_received=0
>>> - do send_msg()
>>> -then in a cycle:
>>> pthread_cond_timed_wait(&sync_requests.cond, &sync_request.lock, ×pec);
>>> - at return from it check if sync_request.reply_received == 1, if not
>>> check if timeout expired and either return a failure or go to the start of the cycle.
>>>
>>> 3) at mp_handler() if REPLY received - grab sync_request.lock,
>>> search through sync_requests.list for dst[] ,
>>> if found, then set it's reply_received=1, copy the received message into reply
>>> and call pthread_cond_braodcast((&sync_requests.cond);
>> The only benefit I can see is that now the sender can request to
>> multiple receivers at the same time. And it makes things more
>> complicated. Do we really need this?
> The benefit is that one thread is blocked waiting for response,
> your mp_handler can still receive and handle other messages.
This can already be done in the original implementation. mp_handler
listens for msg, request from the other peer(s), and replies the
requests, which is not affected.
> Plus as you said - other threads can keep sending messages.
For this one, in the original implementation, other threads can still
send msg, but not request. I suppose the request is not in a fast path,
why we care to make it fast?
Thanks,
Jianfeng
> Konstantin
>
>> Thanks,
>> Jianfeng