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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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@redhat.com>
>>> Sent: Wednesday, May 31, 2023 5:29 PM
>>> To: Xia, Chenbo <chenbo.xia@intel.com>; Thomas Monjalon
>>> <thomas@monjalon.net>; Eelco Chaudron <echaudro@redhat.com>;
>>> david.marchand@redhat.com
>>> Cc: dev@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@redhat.com>
>>>>> Sent: Tuesday, May 30, 2023 11:17 PM
>>>>> To: Thomas Monjalon <thomas@monjalon.net>; Eelco Chaudron
>>>>> <echaudro@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>;
>>>>> david.marchand@redhat.com
>>>>> Cc: dev@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@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@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/

Thanks,
Maxime

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