[PATCH v3 4/4] vhost: add device op to offload the interrupt kick

[Eelco Chaudron]

On 1 Jun 2023, at 10:29, Maxime Coquelin wrote:

> On 6/1/23 10:15, Eelco Chaudron wrote:
>>
>>
>> On 1 Jun 2023, at 4:18, Xia, Chenbo wrote:
>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>>>> Sent: Wednesday, May 31, 2023 5:29 PM
>>>> To: Xia, Chenbo <chenbo.xia at intel.com>; Thomas Monjalon
>>>> <thomas at monjalon.net>; Eelco Chaudron <echaudro at redhat.com>;
>>>> david.marchand at redhat.com
>>>> Cc: dev at dpdk.org
>>>> Subject: Re: [PATCH v3 4/4] vhost: add device op to offload the interrupt
>>>> kick
>>>>
>>>>
>>>>
>>>> On 5/31/23 08:19, Xia, Chenbo wrote:
>>>>>> -----Original Message-----
>>>>>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>>>>>> Sent: Tuesday, May 30, 2023 11:17 PM
>>>>>> To: Thomas Monjalon <thomas at monjalon.net>; Eelco Chaudron
>>>>>> <echaudro at redhat.com>; Xia, Chenbo <chenbo.xia at intel.com>;
>>>>>> david.marchand at redhat.com
>>>>>> Cc: dev at dpdk.org
>>>>>> Subject: Re: [PATCH v3 4/4] vhost: add device op to offload the
>>>> interrupt
>>>>>> kick
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 5/30/23 15:16, Thomas Monjalon wrote:
>>>>>>> 30/05/2023 15:02, Maxime Coquelin:
>>>>>>>>
>>>>>>>> On 5/17/23 11:09, Eelco Chaudron wrote:
>>>>>>>>> This patch adds an operation callback which gets called every time
>>>> the
>>>>>>>>> library wants to call eventfd_write(). This eventfd_write() call
>>>> could
>>>>>>>>> result in a system call, which could potentially block the PMD
>>>> thread.
>>>>>>>>>
>>>>>>>>> The callback function can decide whether it's ok to handle the
>>>>>>>>> eventfd_write() now or have the newly introduced function,
>>>>>>>>> rte_vhost_notify_guest(), called at a later time.
>>>>>>>>>
>>>>>>>>> This can be used by 3rd party applications, like OVS, to avoid
>>>> system
>>>>>>>>> calls being called as part of the PMD threads.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
>>>>>>>>> ---
>>>>>>>>>      lib/vhost/meson.build |    2 ++
>>>>>>>>>      lib/vhost/rte_vhost.h |   23 +++++++++++++++++-
>>>>>>>>>      lib/vhost/socket.c    |   63
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++---
>>>>>>>>>      lib/vhost/version.map |    9 +++++++
>>>>>>>>>      lib/vhost/vhost.c     |   38 ++++++++++++++++++++++++++++++
>>>>>>>>>      lib/vhost/vhost.h     |   58 ++++++++++++++++++++++++++++++++---
>>>> ---
>>>>>> -------
>>>>>>>>>      6 files changed, 171 insertions(+), 22 deletions(-)
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The patch looks good to me, but that's the first time we use function
>>>>>>>> versioning in Vhost library, so I'd like another pair of eyes to be
>>>>>> sure
>>>>>>>> I don't miss anything.
>>>>>>>>
>>>>>>>> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
>>>>>>>>
>>>>>>>> Thomas, do we need to mention it somewhere in the release note?
>>>>>>>
>>>>>>> If compatibility is kept, I think we don't need to mention it.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Thanks Thomas for the information.
>>>>>>
>>>>>> Maxime
>>>>>
>>>>> About release note, except the versioning, there is also one new API
>>>> introduced
>>>>> in this patch, so we still need to mention this in release note
>>>>
>>>> Right, good catch.
>>>> Eelco, let me know what you would put, I'll add it while applying (No
>>>> need for a new revision).
>>>
>>> Btw, the vhost_lib.rst also needs a new item..
>>
>> What about the following?
>>
>> diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
>> index e8bb8c9b7b..0f964d7a4a 100644
>> --- a/doc/guides/prog_guide/vhost_lib.rst
>> +++ b/doc/guides/prog_guide/vhost_lib.rst
>> @@ -334,6 +334,11 @@ The following is an overview of some key Vhost API functions:
>>     Clean DMA vChannel finished to use. After this function is called,
>>     the specified DMA vChannel should no longer be used by the Vhost library.
>>
>> +* ``rte_vhost_notify_guest(int vid, uint16_t queue_id)``
>> +
>> +  Inject the offloaded interrupt received by the 'guest_notify' callback,
>> +  into the vhost device's queue.
>> +
>>   Vhost-user Implementations
>>   --------------------------
>>
>> Maxime do you want to add it, or do you want a new rev?
>
> That's fine, I just added it now :) Can you check the next-virtio main
> branch and confirm all is OK for your series?
>
> https://git.dpdk.org/next/dpdk-next-virtio/log/
>

One small spelling issue, the rest looks good:

+* **Added callback API support for interrupt handling in the vhost library.**
+
+  A new callback, guest_notify, is introduced that can be used to handle the
+  interrupt kick outside of the datapath fast path. In addition, a new API,
+  rte_vhost_notify_guest(), is added to raise the interrupt outside of the
+  past path.

Last sentence spells ‘past path’ instead of ‘fast path’.


> Thanks,
> Maxime
>
>>
>>> Thanks,
>>> Chenbo
>>>
>>>>
>>>> Thanks,
>>>> Maxime
>>>>
>>>>> Thanks,
>>>>> Chenbo
>>