DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
@ 2018-02-09 17:34 Tomasz Kulasek
  2018-02-11  4:21 ` Tan, Jianfeng
  0 siblings, 1 reply; 4+ messages in thread
From: Tomasz Kulasek @ 2018-02-09 17:34 UTC (permalink / raw)
  To: yliu; +Cc: dev, yuanhan.liu, stable, Changpeng Liu

For each virt queue's kickfd and callfd, there are 2 invalid
status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
Don't set the virt queue to ready status until got the valid
descriptor.

This is safe for polling mode drivers in Guest OS, the backend
vhost process will not post notification to interrupt vector for
PMD mode in guest, but the interrupt vector still valid.

Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
Cc: yuanhan.liu@linux.intel.com
Cc: stable@dpdk.org

Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
 lib/librte_vhost/vhost_user.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 65ee33919..4508f697b 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
 {
 	return vq && vq->desc && vq->avail && vq->used &&
 	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
-	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
+	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
+	       vq->kickfd != VIRTIO_INVALID_EVENTFD &&
+	       vq->callfd != VIRTIO_INVALID_EVENTFD;
 }
 
 static int
-- 
2.14.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
  2018-02-09 17:34 [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor Tomasz Kulasek
@ 2018-02-11  4:21 ` Tan, Jianfeng
  2018-07-26 15:48   ` Thomas Monjalon
  0 siblings, 1 reply; 4+ messages in thread
From: Tan, Jianfeng @ 2018-02-11  4:21 UTC (permalink / raw)
  To: Kulasek, TomaszX, yliu; +Cc: dev, stable, Liu,  Changpeng



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tomasz Kulasek
> Sent: Saturday, February 10, 2018 1:35 AM
> To: yliu@fridaylinux.org
> Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; stable@dpdk.org; Liu,
> Changpeng
> Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> 
> For each virt queue's kickfd and callfd, there are 2 invalid
> status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> Don't set the virt queue to ready status until got the valid
> descriptor.

Cannot remember why we introduce two invalid status.

If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is ready, then defining one status (invalid) is enough.

Thanks,
Jianfeng

> 
> This is safe for polling mode drivers in Guest OS, the backend
> vhost process will not post notification to interrupt vector for
> PMD mode in guest, but the interrupt vector still valid.
> 
> Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
> Cc: yuanhan.liu@linux.intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 65ee33919..4508f697b 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
>  {
>  	return vq && vq->desc && vq->avail && vq->used &&
>  	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> -	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> +	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> +	       vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> +	       vq->callfd != VIRTIO_INVALID_EVENTFD;
>  }
> 
>  static int
> --
> 2.14.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
  2018-02-11  4:21 ` Tan, Jianfeng
@ 2018-07-26 15:48   ` Thomas Monjalon
  2018-07-27  6:11     ` Tiwei Bie
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2018-07-26 15:48 UTC (permalink / raw)
  To: maxime.coquelin, tiwei.bie; +Cc: dev, Kulasek, TomaszX, yliu, Liu, Changpeng

What is the status of this patch?


11/02/2018 05:21, Tan, Jianfeng:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tomasz Kulasek
> > Sent: Saturday, February 10, 2018 1:35 AM
> > To: yliu@fridaylinux.org
> > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; stable@dpdk.org; Liu,
> > Changpeng
> > Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> > 
> > For each virt queue's kickfd and callfd, there are 2 invalid
> > status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> > Don't set the virt queue to ready status until got the valid
> > descriptor.
> 
> Cannot remember why we introduce two invalid status.
> 
> If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is ready, then defining one status (invalid) is enough.
> 
> Thanks,
> Jianfeng
> 
> > 
> > This is safe for polling mode drivers in Guest OS, the backend
> > vhost process will not post notification to interrupt vector for
> > PMD mode in guest, but the interrupt vector still valid.
> > 
> > Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
> > Cc: yuanhan.liu@linux.intel.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > ---
> >  lib/librte_vhost/vhost_user.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index 65ee33919..4508f697b 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
> >  {
> >  	return vq && vq->desc && vq->avail && vq->used &&
> >  	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > -	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> > +	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > +	       vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> > +	       vq->callfd != VIRTIO_INVALID_EVENTFD;
> >  }
> > 
> >  static int
> > --
> > 2.14.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
  2018-07-26 15:48   ` Thomas Monjalon
@ 2018-07-27  6:11     ` Tiwei Bie
  0 siblings, 0 replies; 4+ messages in thread
From: Tiwei Bie @ 2018-07-27  6:11 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: maxime.coquelin, dev, Kulasek, TomaszX, yliu, Liu, Changpeng

On Thu, Jul 26, 2018 at 05:48:04PM +0200, Thomas Monjalon wrote:
> What is the status of this patch?
> 
> 
> 11/02/2018 05:21, Tan, Jianfeng:
> > 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tomasz Kulasek
> > > Sent: Saturday, February 10, 2018 1:35 AM
> > > To: yliu@fridaylinux.org
> > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; stable@dpdk.org; Liu,
> > > Changpeng
> > > Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> > > 
> > > For each virt queue's kickfd and callfd, there are 2 invalid
> > > status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> > > Don't set the virt queue to ready status until got the valid
> > > descriptor.
> > 
> > Cannot remember why we introduce two invalid status.
> > 
> > If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is ready, then defining one status (invalid) is enough.
> > 
> > Thanks,
> > Jianfeng
> > 
> > > 
> > > This is safe for polling mode drivers in Guest OS, the backend
> > > vhost process will not post notification to interrupt vector for
> > > PMD mode in guest, but the interrupt vector still valid.
> > > 
> > > Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")

This fixes line isn't right. It's the below commit [1]
that introduced the code changed by this patch. The code
was written in this way intentionally by that commit.
With a quick glance at that commit, it seems that this
patch isn't valid.

[1] https://git.dpdk.org/dpdk/commit/?id=fb871d0a4dc1c038a381c524cdb86fe83d21d842
commit fb871d0a4dc1c038a381c524cdb86fe83d21d842
Author: Tetsuya Mukawa <mukawa@igel.co.jp>
Date:   Mon Mar 14 17:53:32 2016 +0900

    vhost: fix default value of kickfd and callfd
    
    Currently, default values of kickfd and callfd are -1.
    If the values are -1, current code guesses kickfd and callfd haven't
    been initialized yet. Then vhost library will guess the virtqueue isn't
    ready for processing.
    
    But callfd and kickfd will be set as -1 when "--enable-kvm"
    isn't specified in QEMU command line. It means we cannot treat -1 as
    uninitialized state.
    
    The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
    VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
    the default values of kickfd and callfd.
    
    Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
    Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>



> > > Cc: yuanhan.liu@linux.intel.com
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > > ---
> > >  lib/librte_vhost/vhost_user.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > > index 65ee33919..4508f697b 100644
> > > --- a/lib/librte_vhost/vhost_user.c
> > > +++ b/lib/librte_vhost/vhost_user.c
> > > @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
> > >  {
> > >  	return vq && vq->desc && vq->avail && vq->used &&
> > >  	       vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > > -	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> > > +	       vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > > +	       vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> > > +	       vq->callfd != VIRTIO_INVALID_EVENTFD;
> > >  }
> > > 
> > >  static int
> > > --
> > > 2.14.1
> > 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-27  6:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-09 17:34 [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor Tomasz Kulasek
2018-02-11  4:21 ` Tan, Jianfeng
2018-07-26 15:48   ` Thomas Monjalon
2018-07-27  6:11     ` Tiwei Bie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).