[dpdk-dev] [PATCH v2 07/12] eal: add channel for primary/secondary communication

[Burakov, Anatoly]

On 30-Sep-17 5:07 AM, Tan, Jianfeng wrote:
> 
> 
> On 9/29/2017 6:00 PM, Burakov, Anatoly wrote:
>> On 29-Sep-17 2:03 AM, Tan, Jianfeng wrote:
>>> + Reshma and Jan.
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Burakov, Anatoly
>>>> Sent: Thursday, September 28, 2017 11:30 PM
>>>> To: dev at dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH v2 07/12] eal: add channel for
>>>> primary/secondary communication
>>>>
>>>> On 28-Sep-17 4:01 PM, Ananyev, Konstantin wrote:
>>>>> Hi Jianfeng,
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Tan, Jianfeng
>>>>>> Sent: Thursday, September 28, 2017 2:56 PM
>>>>>> To: dev at dpdk.org
>>>>>> Cc: Richardson, Bruce <bruce.richardson at intel.com>; Ananyev,
>>>> Konstantin <konstantin.ananyev at intel.com>; De Lara Guarch, Pablo
>>>>>> <pablo.de.lara.guarch at intel.com>; thomas at monjalon.net;
>>>> yliu at fridaylinux.org; maxime.coquelin at redhat.com; mtetsuyah at gmail.com;
>>>>>> Yigit, Ferruh <ferruh.yigit at intel.com>; Tan, Jianfeng
>>>> <jianfeng.tan at intel.com>
>>>>>> Subject: [PATCH v2 07/12] eal: add channel for primary/secondary
>>>> communication
>>>>>>
>>>>>> Previouly, there is only one way for primary/secondary to exchange
>>>>>> messages, that is, primary process writes info into some predefind
>>>>>> file, and secondary process reads info out. That cannot address
>>>>>> the requirements:
>>>>>>     a. Secondary wants to send info to primary, for example, 
>>>>>> secondary
>>>>>>        would like to send request (about some specific vdev to 
>>>>>> primary).
>>>>>>     b. Sending info at any time, instead of just initialization time.
>>>>>>     c. Share FDs with the other side, for vdev like vhost, related 
>>>>>> FDs
>>>>>>        (memory region, kick) should be shared.
>>>>>>
>>>>>> This patch proposes to create a communication channel, as an unix
>>>>>> socket connection, for above requirements. Primary will listen on
>>>>>> the unix socket; secondary will connect this socket to talk.
>>>>>>
>>>>>> Three new APIs are added:
>>>>>>
>>>>>>     1. rte_eal_mp_action_register is used to register an action,
>>>>>>        indexed by a string; if the calling component wants to
>>>>>>        response the messages from the corresponding component in
>>>>>>        its primary process or secondary processes.
>>>>>>     2. rte_eal_mp_action_unregister is used to unregister the action
>>>>>>        if the calling component does not want to response the 
>>>>>> messages.
>>>>>>     3. rte_eal_mp_sendmsg is used to send a message.
>>>>>
>>>>> I think we already have similar channel in librte_pdump().
>>>>> Also it seems like eal_vfio also has it's own socket to communicate
>>>> between mp/sp.
>>>>> Could we probably make it generic - so same code (and socket) be 
>>>>> used by
>>>> all such  places.
>>>>> Konstantin
>>>>>
>>>>
>>>> Agreed, however looking at this, it's already a generic-enough 
>>>> solution,
>>>> and other places could be fixed to use this in later releases.
>>>
>>> Yes, to provide a generic communication way, instead of one channel 
>>> for each subsystem, is the goal of this patch.
>>>
>>> Reshma and Jan, can I ask comment from you to have a look if the way 
>>> of this patch is generic enough to cover pdump and vfio-sync's 
>>> requirement?
>>>
>>> Possible limitation of this patch (so far) is that it only provides 
>>> an async way for request/response, do we need synchronous way?
>>>
>>>> That said, i believe this particular part of the patchset should go 
>>>> in as a
>>>> separate patchset and more design consideration and input from others.
>>>
>>> OK, let's collect more info here, and then take out this patch out as 
>>> a separate patch.
>>>
>>> Thanks,
>>> Jianfeng
>>>
>> Hi Jianfeng,
>>
>> Yes, i believe VFIO does need synchronous communcation, because it 
>> relies on passing fd's from primary to secondary, on request. It could 
>> be rewritten to be asynchronous, similarly to how you handle vdevs 
>> here, but as it stands, it assumes synchronous communication.
>>
> 
> Good to know, thanks Anatoly. Even it can be rewritten to do in asyn 
> way, do we need to propose sync way now?
> 
> Thanks,
> Jianfeng
> 

I believe that we do, because we can't assume that everything can be 
rewritten to be asynchronous. I'm open to other opinions though :)

-- 
Thanks,
Anatoly