[v3,4/4] vhost: add device op to offload the interrupt kick
Checks
Commit Message
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(-)
Comments
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?
Thanks,
Maxime
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.
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
> -----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
Thanks,
Chenbo
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).
Thanks,
Maxime
> Thanks,
> Chenbo
On 31 May 2023, at 11:29, Maxime Coquelin wrote:
> 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).
Thanks guys! What about the following release note addition:
* **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.
Please change if you see fit.
Thanks,
Eelco
> Thanks,
> Maxime
>
>> Thanks,
>> Chenbo
On Tue, May 30, 2023 at 3:02 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> 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>
There may be some enhancement to do in the symbol check (wrt to the
warning http://mails.dpdk.org/archives/test-report/2023-May/394865.html)
but otherwise the versioning part lgtm.
Eelco, Maxime,
On Wed, May 17, 2023 at 11:09 AM Eelco Chaudron <echaudro@redhat.com> wrote:
> @@ -846,6 +848,11 @@ vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
> vsocket->path = NULL;
> }
>
> + if (vsocket && vsocket->malloc_notify_ops) {
> + free(vsocket->malloc_notify_ops);
> + vsocket->malloc_notify_ops = NULL;
> + }
> +
> if (vsocket) {
> free(vsocket);
> vsocket = NULL;
Nit: we had several cleanups in the tree to remove patterns like if
(ptr) free(ptr).
Here, this helper could look for vsocket being NULL first thing, then
call free() unconditionnally.
And resetting the fields to NULL is probably not that useful, since
the vsocket is freed at the end.
Wdyt of:
static void
vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
{
if (vsocket == NULL)
return;
free(vsocket->path);
free(vsocket->malloc_notify_ops);
free(vsocket);
}
On 5/31/23 14:01, David Marchand wrote:
> Eelco, Maxime,
>
> On Wed, May 17, 2023 at 11:09 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>> @@ -846,6 +848,11 @@ vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
>> vsocket->path = NULL;
>> }
>>
>> + if (vsocket && vsocket->malloc_notify_ops) {
>> + free(vsocket->malloc_notify_ops);
>> + vsocket->malloc_notify_ops = NULL;
>> + }
>> +
>> if (vsocket) {
>> free(vsocket);
>> vsocket = NULL;
>
> Nit: we had several cleanups in the tree to remove patterns like if
> (ptr) free(ptr).
> Here, this helper could look for vsocket being NULL first thing, then
> call free() unconditionnally.
> And resetting the fields to NULL is probably not that useful, since
> the vsocket is freed at the end.
> Wdyt of:
>
> static void
> vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
> {
> if (vsocket == NULL)
> return;
>
> free(vsocket->path);
> free(vsocket->malloc_notify_ops);
> free(vsocket);
> }
>
>
It is indeed better, I can fix while applying if Eelco agrees.
Thanks,
Maxime
On 31 May 2023, at 14:48, Maxime Coquelin wrote:
> On 5/31/23 14:01, David Marchand wrote:
>> Eelco, Maxime,
>>
>> On Wed, May 17, 2023 at 11:09 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>> @@ -846,6 +848,11 @@ vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
>>> vsocket->path = NULL;
>>> }
>>>
>>> + if (vsocket && vsocket->malloc_notify_ops) {
>>> + free(vsocket->malloc_notify_ops);
>>> + vsocket->malloc_notify_ops = NULL;
>>> + }
>>> +
>>> if (vsocket) {
>>> free(vsocket);
>>> vsocket = NULL;
>>
>> Nit: we had several cleanups in the tree to remove patterns like if
>> (ptr) free(ptr).
>> Here, this helper could look for vsocket being NULL first thing, then
>> call free() unconditionnally.
>> And resetting the fields to NULL is probably not that useful, since
>> the vsocket is freed at the end.
>> Wdyt of:
>>
>> static void
>> vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
>> {
>> if (vsocket == NULL)
>> return;
>>
>> free(vsocket->path);
>> free(vsocket->malloc_notify_ops);
>> free(vsocket);
>> }
>>
>>
>
> It is indeed better, I can fix while applying if Eelco agrees.
Looks good to me.
//Eelco
Maxime,
On Wed, May 17, 2023 at 11:09 AM Eelco Chaudron <echaudro@redhat.com> wrote:
> @@ -974,11 +994,8 @@ vhost_vring_call_packed(struct virtio_net *dev, struct vhost_virtqueue *vq)
> if (vhost_need_event(off, new, old))
> kick = true;
> kick:
> - if (kick && vq->callfd >= 0) {
> - eventfd_write(vq->callfd, (eventfd_t)1);
> - if (dev->notify_ops->guest_notified)
> - dev->notify_ops->guest_notified(dev->vid);
> - }
> + if (kick && vq->callfd >= 0)
> + vhost_vring_inject_irq(dev, vq);
Thinking again about the VDUSE series overlap...
This hunk here is implicitely fixing an issue.
You addressed it in
https://patchwork.dpdk.org/project/dpdk/patch/20230525162551.70359-2-maxime.coquelin@redhat.com/
So I suggest you apply this patch of yours before Eelco patch 4.
This way, the fix backport will be trivial.
On 5/31/23 16:12, David Marchand wrote:
> Maxime,
>
> On Wed, May 17, 2023 at 11:09 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>> @@ -974,11 +994,8 @@ vhost_vring_call_packed(struct virtio_net *dev, struct vhost_virtqueue *vq)
>> if (vhost_need_event(off, new, old))
>> kick = true;
>> kick:
>> - if (kick && vq->callfd >= 0) {
>> - eventfd_write(vq->callfd, (eventfd_t)1);
>> - if (dev->notify_ops->guest_notified)
>> - dev->notify_ops->guest_notified(dev->vid);
>> - }
>> + if (kick && vq->callfd >= 0)
>> + vhost_vring_inject_irq(dev, vq);
>
> Thinking again about the VDUSE series overlap...
>
> This hunk here is implicitely fixing an issue.
> You addressed it in
> https://patchwork.dpdk.org/project/dpdk/patch/20230525162551.70359-2-maxime.coquelin@redhat.com/
>
> So I suggest you apply this patch of yours before Eelco patch 4.
> This way, the fix backport will be trivial.
>
>
Thanks David, this is indeed the best thing to do to ease backporting
the fix to v22.11.
Maxime
> -----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..
Thanks,
Chenbo
>
> Thanks,
> Maxime
>
> > Thanks,
> > Chenbo
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?
> Thanks,
> Chenbo
>
>>
>> Thanks,
>> Maxime
>>
>>> Thanks,
>>> Chenbo
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
>
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@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/
>
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
>>
On 6/1/23 10:49, Eelco Chaudron wrote:
>
>
> 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@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/
>>
>
> 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’.
I edited the line but it didn't caught my eye!
Fixing it now.
Thanks,
Maxime
>
>> Thanks,
>> Maxime
>>
>>>
>>>> Thanks,
>>>> Chenbo
>>>>
>>>>>
>>>>> Thanks,
>>>>> Maxime
>>>>>
>>>>>> Thanks,
>>>>>> Chenbo
>>>
>
@@ -38,3 +38,5 @@ driver_sdk_headers = files(
'vdpa_driver.h',
)
deps += ['ethdev', 'cryptodev', 'hash', 'pci', 'dmadev']
+
+use_function_versioning = true
@@ -298,7 +298,13 @@ struct rte_vhost_device_ops {
*/
void (*guest_notified)(int vid);
- void *reserved[1]; /**< Reserved for future extension */
+ /**
+ * If this callback is registered, notification to the guest can
+ * be handled by the front-end calling rte_vhost_notify_guest().
+ * If it's not handled, 'false' should be returned. This can be used
+ * to remove the "slow" eventfd_write() syscall from the datapath.
+ */
+ bool (*guest_notify)(int vid, uint16_t queue_id);
};
/**
@@ -433,6 +439,21 @@ void rte_vhost_log_used_vring(int vid, uint16_t vring_idx,
int rte_vhost_enable_guest_notification(int vid, uint16_t queue_id, int enable);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice.
+ *
+ * Inject the offloaded interrupt into the vhost device's queue. For more
+ * details see the 'guest_notify' vhost device operation.
+ *
+ * @param vid
+ * vhost device ID
+ * @param queue_id
+ * virtio queue index
+ */
+__rte_experimental
+void rte_vhost_notify_guest(int vid, uint16_t queue_id);
+
/**
* Register vhost driver. path could be different for multiple
* instance support.
@@ -15,6 +15,7 @@
#include <fcntl.h>
#include <pthread.h>
+#include <rte_function_versioning.h>
#include <rte_log.h>
#include "fd_man.h"
@@ -59,6 +60,7 @@ struct vhost_user_socket {
struct rte_vdpa_device *vdpa_dev;
struct rte_vhost_device_ops const *notify_ops;
+ struct rte_vhost_device_ops *malloc_notify_ops;
};
struct vhost_user_connection {
@@ -846,6 +848,11 @@ vhost_user_socket_mem_free(struct vhost_user_socket *vsocket)
vsocket->path = NULL;
}
+ if (vsocket && vsocket->malloc_notify_ops) {
+ free(vsocket->malloc_notify_ops);
+ vsocket->malloc_notify_ops = NULL;
+ }
+
if (vsocket) {
free(vsocket);
vsocket = NULL;
@@ -1099,21 +1106,69 @@ rte_vhost_driver_unregister(const char *path)
/*
* Register ops so that we can add/remove device to data core.
*/
-int
-rte_vhost_driver_callback_register(const char *path,
- struct rte_vhost_device_ops const * const ops)
+static int
+vhost_driver_callback_register(const char *path,
+ struct rte_vhost_device_ops const * const ops,
+ struct rte_vhost_device_ops *malloc_ops)
{
struct vhost_user_socket *vsocket;
pthread_mutex_lock(&vhost_user.mutex);
vsocket = find_vhost_user_socket(path);
- if (vsocket)
+ if (vsocket) {
vsocket->notify_ops = ops;
+ free(vsocket->malloc_notify_ops);
+ vsocket->malloc_notify_ops = malloc_ops;
+ }
pthread_mutex_unlock(&vhost_user.mutex);
return vsocket ? 0 : -1;
}
+int __vsym
+rte_vhost_driver_callback_register_v24(const char *path,
+ struct rte_vhost_device_ops const * const ops)
+{
+ return vhost_driver_callback_register(path, ops, NULL);
+}
+
+int __vsym
+rte_vhost_driver_callback_register_v23(const char *path,
+ struct rte_vhost_device_ops const * const ops)
+{
+ int ret;
+
+ /*
+ * Although the ops structure is a const structure, we do need to
+ * override the guest_notify operation. This is because with the
+ * previous APIs it was "reserved" and if any garbage value was passed,
+ * it could crash the application.
+ */
+ if (ops && !ops->guest_notify) {
+ struct rte_vhost_device_ops *new_ops;
+
+ new_ops = malloc(sizeof(*new_ops));
+ if (new_ops == NULL)
+ return -1;
+
+ memcpy(new_ops, ops, sizeof(*new_ops));
+ new_ops->guest_notify = NULL;
+
+ ret = vhost_driver_callback_register(path, new_ops, new_ops);
+ } else {
+ ret = vhost_driver_callback_register(path, ops, NULL);
+ }
+
+ return ret;
+}
+
+/* Mark the v23 function as the old version, and v24 as the default version. */
+VERSION_SYMBOL(rte_vhost_driver_callback_register, _v23, 23);
+BIND_DEFAULT_SYMBOL(rte_vhost_driver_callback_register, _v24, 24);
+MAP_STATIC_SYMBOL(int rte_vhost_driver_callback_register(const char *path,
+ struct rte_vhost_device_ops const * const ops),
+ rte_vhost_driver_callback_register_v24);
+
struct rte_vhost_device_ops const *
vhost_driver_callback_get(const char *path)
{
@@ -64,6 +64,12 @@ DPDK_23 {
local: *;
};
+DPDK_24 {
+ global:
+
+ rte_vhost_driver_callback_register;
+} DPDK_23;
+
EXPERIMENTAL {
global:
@@ -98,6 +104,9 @@ EXPERIMENTAL {
# added in 22.11
rte_vhost_async_dma_unconfigure;
rte_vhost_vring_call_nonblock;
+
+ # added in 23.07
+ rte_vhost_notify_guest;
};
INTERNAL {
@@ -44,6 +44,10 @@ static const struct vhost_vq_stats_name_off vhost_vq_stat_strings[] = {
{"size_1024_1518_packets", offsetof(struct vhost_virtqueue, stats.size_bins[6])},
{"size_1519_max_packets", offsetof(struct vhost_virtqueue, stats.size_bins[7])},
{"guest_notifications", offsetof(struct vhost_virtqueue, stats.guest_notifications)},
+ {"guest_notifications_offloaded", offsetof(struct vhost_virtqueue,
+ stats.guest_notifications_offloaded)},
+ {"guest_notifications_error", offsetof(struct vhost_virtqueue,
+ stats.guest_notifications_error)},
{"iotlb_hits", offsetof(struct vhost_virtqueue, stats.iotlb_hits)},
{"iotlb_misses", offsetof(struct vhost_virtqueue, stats.iotlb_misses)},
{"inflight_submitted", offsetof(struct vhost_virtqueue, stats.inflight_submitted)},
@@ -1467,6 +1471,40 @@ rte_vhost_enable_guest_notification(int vid, uint16_t queue_id, int enable)
return ret;
}
+void
+rte_vhost_notify_guest(int vid, uint16_t queue_id)
+{
+ struct virtio_net *dev = get_device(vid);
+ struct vhost_virtqueue *vq;
+
+ if (!dev || queue_id >= VHOST_MAX_VRING)
+ return;
+
+ vq = dev->virtqueue[queue_id];
+ if (!vq)
+ return;
+
+ rte_rwlock_read_lock(&vq->access_lock);
+
+ if (vq->callfd >= 0) {
+ int ret = eventfd_write(vq->callfd, (eventfd_t)1);
+
+ if (ret) {
+ if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+ __atomic_fetch_add(&vq->stats.guest_notifications_error,
+ 1, __ATOMIC_RELAXED);
+ } else {
+ if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+ __atomic_fetch_add(&vq->stats.guest_notifications,
+ 1, __ATOMIC_RELAXED);
+ if (dev->notify_ops->guest_notified)
+ dev->notify_ops->guest_notified(dev->vid);
+ }
+ }
+
+ rte_rwlock_read_unlock(&vq->access_lock);
+}
+
void
rte_vhost_log_write(int vid, uint64_t addr, uint64_t len)
{
@@ -141,6 +141,8 @@ struct virtqueue_stats {
uint64_t inflight_completed;
/* Counters below are atomic, and should be incremented as such. */
uint64_t guest_notifications;
+ uint64_t guest_notifications_offloaded;
+ uint64_t guest_notifications_error;
};
/**
@@ -884,6 +886,34 @@ vhost_need_event(uint16_t event_idx, uint16_t new_idx, uint16_t old)
return (uint16_t)(new_idx - event_idx - 1) < (uint16_t)(new_idx - old);
}
+static __rte_always_inline void
+vhost_vring_inject_irq(struct virtio_net *dev, struct vhost_virtqueue *vq)
+{
+ int ret;
+
+ if (dev->notify_ops->guest_notify &&
+ dev->notify_ops->guest_notify(dev->vid, vq->index)) {
+ if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+ __atomic_fetch_add(&vq->stats.guest_notifications_offloaded,
+ 1, __ATOMIC_RELAXED);
+ return;
+ }
+
+ ret = eventfd_write(vq->callfd, (eventfd_t) 1);
+ if (ret) {
+ if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+ __atomic_fetch_add(&vq->stats.guest_notifications_error,
+ 1, __ATOMIC_RELAXED);
+ return;
+ }
+
+ if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
+ __atomic_fetch_add(&vq->stats.guest_notifications,
+ 1, __ATOMIC_RELAXED);
+ if (dev->notify_ops->guest_notified)
+ dev->notify_ops->guest_notified(dev->vid);
+}
+
static __rte_always_inline void
vhost_vring_call_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
{
@@ -906,23 +936,13 @@ vhost_vring_call_split(struct virtio_net *dev, struct vhost_virtqueue *vq)
if ((vhost_need_event(vhost_used_event(vq), new, old) ||
unlikely(!signalled_used_valid)) &&
vq->callfd >= 0) {
- eventfd_write(vq->callfd, (eventfd_t) 1);
- if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
- __atomic_fetch_add(&vq->stats.guest_notifications,
- 1, __ATOMIC_RELAXED);
- if (dev->notify_ops->guest_notified)
- dev->notify_ops->guest_notified(dev->vid);
+ vhost_vring_inject_irq(dev, vq);
}
} else {
/* Kick the guest if necessary. */
if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT)
&& (vq->callfd >= 0)) {
- eventfd_write(vq->callfd, (eventfd_t)1);
- if (dev->flags & VIRTIO_DEV_STATS_ENABLED)
- __atomic_fetch_add(&vq->stats.guest_notifications,
- 1, __ATOMIC_RELAXED);
- if (dev->notify_ops->guest_notified)
- dev->notify_ops->guest_notified(dev->vid);
+ vhost_vring_inject_irq(dev, vq);
}
}
}
@@ -974,11 +994,8 @@ vhost_vring_call_packed(struct virtio_net *dev, struct vhost_virtqueue *vq)
if (vhost_need_event(off, new, old))
kick = true;
kick:
- if (kick && vq->callfd >= 0) {
- eventfd_write(vq->callfd, (eventfd_t)1);
- if (dev->notify_ops->guest_notified)
- dev->notify_ops->guest_notified(dev->vid);
- }
+ if (kick && vq->callfd >= 0)
+ vhost_vring_inject_irq(dev, vq);
}
static __rte_always_inline void
@@ -1017,4 +1034,11 @@ mbuf_is_consumed(struct rte_mbuf *m)
uint64_t hua_to_alignment(struct rte_vhost_memory *mem, void *ptr);
void mem_set_dump(void *ptr, size_t size, bool enable, uint64_t alignment);
+
+/* Versioned functions */
+int rte_vhost_driver_callback_register_v23(const char *path,
+ struct rte_vhost_device_ops const * const ops);
+int rte_vhost_driver_callback_register_v24(const char *path,
+ struct rte_vhost_device_ops const * const ops);
+
#endif /* _VHOST_NET_CDEV_H_ */