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

[Ananyev, Konstantin]

Hi Jianfeng,

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Tuesday, January 16, 2018 8:11 AM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org; Burakov, Anatoly <anatoly.burakov at intel.com>
> Cc: Richardson, Bruce <bruce.richardson at intel.com>; thomas at 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, &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?

The benefit is that one thread is blocked waiting for response,
your mp_handler can still receive and handle other messages.
Plus as you said - other threads can keep sending messages.
Konstantin 

> 
> Thanks,
> Jianfeng