Re: [PATCH v2 3/4] eal: add synchronous multi-process communication

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

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, &timespec);
>    - 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?

Thanks,
Jianfeng