[Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Zhiyong Yang]

   Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
to cause failures of new connection when negotiating to set MQ.
(one queue pair works well).
   Because there exist some bugs in qemu code when introducing
VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
for the second time, qemu indeed doesn't send the messge (The message
needs to be sent only once)but still will be waiting for dpdk's reply
ack, then, qemu is always freezing, while DPDK is always waiting for
next vhost message from qemu.
  The patch aims to fix the bug, MQ can work well.
  The same bug is found in function vhost_user_net_set_mtu, it is fixed
at the same time.
  DPDK related patch is as following:
  http://www.dpdk.org/dev/patchwork/patch/23955/

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 hw/virtio/vhost-user.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9334a8a..c2c54ce 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
     /*
      * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
      * we just need send it once in the first time. For later such
-     * request, we just ignore it.
+     * request, we just ignore it. In this case, return value is 1 which is
+     * different from 0 that stands for message written successfully.
      */
     if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
-        return 0;
+        return 1;
     }
 
     if (qemu_chr_fe_set_msgfds(chr, fds, fd_num) < 0) {
@@ -270,7 +271,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
                                     struct vhost_memory *mem)
 {
     int fds[VHOST_MEMORY_MAX_NREGIONS];
-    int i, fd;
+    int i, fd, ret;
     size_t fd_num = 0;
     bool reply_supported = virtio_has_feature(dev->protocol_features,
                                               VHOST_USER_PROTOCOL_F_REPLY_ACK);
@@ -315,11 +316,12 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
     msg.size += sizeof(msg.payload.memory.padding);
     msg.size += fd_num * sizeof(VhostUserMemoryRegion);
 
-    if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
+    ret = vhost_user_write(dev, &msg, fds, fd_num);
+    if (ret < 0) {
         return -1;
     }
 
-    if (reply_supported) {
+    if (reply_supported && (ret == 0)) {
         return process_message_reply(dev, msg.request);
     }
 
@@ -691,6 +693,7 @@ static bool vhost_user_can_merge(struct vhost_dev *dev,
 static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
 {
     VhostUserMsg msg;
+    int ret;
     bool reply_supported = virtio_has_feature(dev->protocol_features,
                                               VHOST_USER_PROTOCOL_F_REPLY_ACK);
 
@@ -706,12 +709,13 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
         msg.flags |= VHOST_USER_NEED_REPLY_MASK;
     }
 
-    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+    ret = vhost_user_write(dev, &msg, NULL, 0);
+    if (ret < 0) {
         return -1;
     }
 
     /* If reply_ack supported, slave has to ack specified MTU is valid */
-    if (reply_supported) {
+    if (reply_supported && (ret == 0)) {
         return process_message_reply(dev, msg.request);
     }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Maxime Coquelin]

On 04/28/2017 09:09 AM, Zhiyong Yang wrote:
>     Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
> to cause failures of new connection when negotiating to set MQ.
> (one queue pair works well).
>     Because there exist some bugs in qemu code when introducing
> VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
> is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
> for the second time, qemu indeed doesn't send the messge (The message
> needs to be sent only once)but still will be waiting for dpdk's reply
> ack, then, qemu is always freezing, while DPDK is always waiting for
> next vhost message from qemu.
>    The patch aims to fix the bug, MQ can work well.
>    The same bug is found in function vhost_user_net_set_mtu, it is fixed
> at the same time.
>    DPDK related patch is as following:
>    http://www.dpdk.org/dev/patchwork/patch/23955/
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
>   hw/virtio/vhost-user.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 9334a8a..c2c54ce 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
>       /*
>        * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
>        * we just need send it once in the first time. For later such
> -     * request, we just ignore it.
> +     * request, we just ignore it. In this case, return value is 1 which is
> +     * different from 0 that stands for message written successfully.
>        */
>       if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
> -        return 0;
> +        return 1;

I personally prefer the fix I suggested in the DPDK mail thread, as
returning a random positive value does look like a workaround:

"
I think the problem must be fixed generally and not per request.
Maybe in vhost_user_write() if one-time request, just clear the
VHOST_USER_NEED_REPLY flag. Then, in process_message_reply(), return
early if this flag isn't set.
"

Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Yang, Zhiyong]

Hi，Maxime:

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Tuesday, May 2, 2017 8:23 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; qemu-devel@nongnu.org
> Cc: mst@redhat.com
> Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when
> MQ
> 
> 
> 
> On 04/28/2017 09:09 AM, Zhiyong Yang wrote:
> >     Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together to
> > cause failures of new connection when negotiating to set MQ.
> > (one queue pair works well).
> >     Because there exist some bugs in qemu code when introducing
> > VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When
> vhost_user_set_mem_table
> > is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE for
> > the second time, qemu indeed doesn't send the messge (The message
> > needs to be sent only once)but still will be waiting for dpdk's reply
> > ack, then, qemu is always freezing, while DPDK is always waiting for
> > next vhost message from qemu.
> >    The patch aims to fix the bug, MQ can work well.
> >    The same bug is found in function vhost_user_net_set_mtu, it is
> > fixed at the same time.
> >    DPDK related patch is as following:
> >    http://www.dpdk.org/dev/patchwork/patch/23955/
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> >   hw/virtio/vhost-user.c | 18 +++++++++++-------
> >   1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > 9334a8a..c2c54ce 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev *dev,
> VhostUserMsg *msg,
> >       /*
> >        * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> >        * we just need send it once in the first time. For later such
> > -     * request, we just ignore it.
> > +     * request, we just ignore it. In this case, return value is 1 which is
> > +     * different from 0 that stands for message written successfully.
> >        */
> >       if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
> > -        return 0;
> > +        return 1;
> 
> I personally prefer the fix I suggested in the DPDK mail thread, as returning a
> random positive value does look like a workaround:

I think that for vhost_user_write(), it's behaving in a different way for some specific vhost messages.  
So, it should not return the same returen value 0 which stands for success. 

> 
> "
> I think the problem must be fixed generally and not per request.
> Maybe in vhost_user_write() if one-time request, just clear the
> VHOST_USER_NEED_REPLY flag. Then, in process_message_reply(), return early
> if this flag isn't set.
> "
It's another choise. Either this one nor that one,  not a big deal. :)
Fixing these  existing bugs is the most important.

thanks
Zhiyong Yang

Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Marc-André Lureau]

Hi

On Wed, May 3, 2017 at 7:09 AM Yang, Zhiyong <zhiyong.yang@intel.com> wrote:

> Hi，Maxime:
>
> > -----Original Message-----
> > From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> > Sent: Tuesday, May 2, 2017 8:23 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>; qemu-devel@nongnu.org
> > Cc: mst@redhat.com
> > Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to
> startup when
> > MQ
> >
> >
> >
> > On 04/28/2017 09:09 AM, Zhiyong Yang wrote:
> > >     Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together to
> > > cause failures of new connection when negotiating to set MQ.
> > > (one queue pair works well).
> > >     Because there exist some bugs in qemu code when introducing
> > > VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When
> > vhost_user_set_mem_table
> > > is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE for
> > > the second time, qemu indeed doesn't send the messge (The message
> > > needs to be sent only once)but still will be waiting for dpdk's reply
> > > ack, then, qemu is always freezing, while DPDK is always waiting for
> > > next vhost message from qemu.
> > >    The patch aims to fix the bug, MQ can work well.
> > >    The same bug is found in function vhost_user_net_set_mtu, it is
> > > fixed at the same time.
> > >    DPDK related patch is as following:
> > >    http://www.dpdk.org/dev/patchwork/patch/23955/
> > >
> > > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > > ---
> > >   hw/virtio/vhost-user.c | 18 +++++++++++-------
> > >   1 file changed, 11 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > > 9334a8a..c2c54ce 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev
> *dev,
> > VhostUserMsg *msg,
> > >       /*
> > >        * For non-vring specific requests, like
> VHOST_USER_SET_MEM_TABLE,
> > >        * we just need send it once in the first time. For later such
> > > -     * request, we just ignore it.
> > > +     * request, we just ignore it. In this case, return value is 1
> which is
> > > +     * different from 0 that stands for message written successfully.
> > >        */
> > >       if (vhost_user_one_time_request(msg->request) && dev->vq_index
> != 0) {
> > > -        return 0;
> > > +        return 1;
> >
> > I personally prefer the fix I suggested in the DPDK mail thread, as
> returning a
> > random positive value does look like a workaround:
>
> I think that for vhost_user_write(), it's behaving in a different way for
> some specific vhost messages.
> So, it should not return the same returen value 0 which stands for success.
>

But you need to do the special handling for every caller.


> >
> > "
> > I think the problem must be fixed generally and not per request.
> > Maybe in vhost_user_write() if one-time request, just clear the
> > VHOST_USER_NEED_REPLY flag. Then, in process_message_reply(), return
> early
> > if this flag isn't set.
> > "
> It's another choise. Either this one nor that one,  not a big deal. :)
> Fixing these  existing bugs is the most important.
>
>
While the suggestion from Maxime would work transparently, similar to
one-time request are transparent to caller today. I also prefer that
solution.

thanks
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

[Yang, Zhiyong]

Hi,  Maxime, Marc-André Lureau:

Thank you a lot for your suggestions and I’m agree with you. I will send V2 later according to
Maxime’s suggested change.

Thanks
Zhiyong

From: Marc-André Lureau [mailto:marcandre.lureau@gmail.com]
Sent: Wednesday, May 3, 2017 8:37 PM
To: Yang, Zhiyong <zhiyong.yang@intel.com>; Maxime Coquelin <maxime.coquelin@redhat.com>; qemu-devel@nongnu.org
Cc: Liu, Yuanhan <yuanhan.liu@intel.com>; mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ

Hi

On Wed, May 3, 2017 at 7:09 AM Yang, Zhiyong <zhiyong.yang@intel.com<mailto:zhiyong.yang@intel.com>> wrote:
Hi，Maxime:

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com<mailto:maxime.coquelin@redhat.com>]
> Sent: Tuesday, May 2, 2017 8:23 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com<mailto:zhiyong.yang@intel.com>>; qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>
> Cc: mst@redhat.com<mailto:mst@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when
> MQ
>
>
>
> On 04/28/2017 09:09 AM, Zhiyong Yang wrote:
> >     Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together to
> > cause failures of new connection when negotiating to set MQ.
> > (one queue pair works well).
> >     Because there exist some bugs in qemu code when introducing
> > VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When
> vhost_user_set_mem_table
> > is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE for
> > the second time, qemu indeed doesn't send the messge (The message
> > needs to be sent only once)but still will be waiting for dpdk's reply
> > ack, then, qemu is always freezing, while DPDK is always waiting for
> > next vhost message from qemu.
> >    The patch aims to fix the bug, MQ can work well.
> >    The same bug is found in function vhost_user_net_set_mtu, it is
> > fixed at the same time.
> >    DPDK related patch is as following:
> >    http://www.dpdk.org/dev/patchwork/patch/23955/
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com<mailto:zhiyong.yang@intel.com>>
> > ---
> >   hw/virtio/vhost-user.c | 18 +++++++++++-------
> >   1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > 9334a8a..c2c54ce 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev *dev,
> VhostUserMsg *msg,
> >       /*
> >        * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> >        * we just need send it once in the first time. For later such
> > -     * request, we just ignore it.
> > +     * request, we just ignore it. In this case, return value is 1 which is
> > +     * different from 0 that stands for message written successfully.
> >        */
> >       if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
> > -        return 0;
> > +        return 1;
>
> I personally prefer the fix I suggested in the DPDK mail thread, as returning a
> random positive value does look like a workaround:

I think that for vhost_user_write(), it's behaving in a different way for some specific vhost messages.
So, it should not return the same returen value 0 which stands for success.

But you need to do the special handling for every caller.

>
> "
> I think the problem must be fixed generally and not per request.
> Maybe in vhost_user_write() if one-time request, just clear the
> VHOST_USER_NEED_REPLY flag. Then, in process_message_reply(), return early
> if this flag isn't set.
> "
It's another choise. Either this one nor that one,  not a big deal. :)
Fixing these  existing bugs is the most important.

While the suggestion from Maxime would work transparently, similar to one-time request are transparent to caller today. I also prefer that solution.
thanks
--
Marc-André Lureau

[Qemu-devel] [PATCH v2] hw/virtio: fix vhost user fails to startup when MQ

[Zhiyong Yang]

 Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
to cause failures of new connection when negotiating to set MQ.
(one queue pair works well).
   Because there exist some bugs in qemu code when introducing
VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
for the second time, qemu indeed doesn't send the messge (The message
needs to be sent only once)but still will be waiting for dpdk's reply
ack, then, qemu is always freezing, while DPDK is always waiting for
next vhost message from qemu.
  The patch aims to fix the bug, MQ can work well.
  The same bug is found in function vhost_user_net_set_mtu, it is fixed
at the same time.
  DPDK related patch is as following:
  http://www.dpdk.org/dev/patchwork/patch/23955/

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---

Changes in V2:
Thanks for Maxime's suggestion, if one-time request, clear the
VHOST_USER_NEED_REPLY flag in function vhost_user_write,
in process_message_reply(), return early, if this flag isn't set.

 hw/virtio/vhost-user.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9334a8a..32a95a8 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -163,22 +163,26 @@ fail:
 }
 
 static int process_message_reply(struct vhost_dev *dev,
-                                 VhostUserRequest request)
+                                 VhostUserMsg msg)
 {
-    VhostUserMsg msg;
+    VhostUserMsg msg_reply;
 
-    if (vhost_user_read(dev, &msg) < 0) {
+    if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
+        return 0;
+    }
+
+    if (vhost_user_read(dev, &msg_reply) < 0) {
         return -1;
     }
 
-    if (msg.request != request) {
+    if (msg_reply.request != msg.request) {
         error_report("Received unexpected msg type."
                      "Expected %d received %d",
-                     request, msg.request);
+                     msg.request, msg_reply.request);
         return -1;
     }
 
-    return msg.payload.u64 ? -1 : 0;
+    return msg_reply.payload.u64 ? -1 : 0;
 }
 
 static bool vhost_user_one_time_request(VhostUserRequest request)
@@ -208,6 +212,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
      * request, we just ignore it.
      */
     if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
+        msg->flags &= ~VHOST_USER_NEED_REPLY_MASK;
         return 0;
     }
 
@@ -320,7 +325,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
     }
 
     if (reply_supported) {
-        return process_message_reply(dev, msg.request);
+        return process_message_reply(dev, msg);
     }
 
     return 0;
@@ -712,7 +717,7 @@ static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
 
     /* If reply_ack supported, slave has to ack specified MTU is valid */
     if (reply_supported) {
-        return process_message_reply(dev, msg.request);
+        return process_message_reply(dev, msg);
     }
 
     return 0;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] hw/virtio: fix vhost user fails to startup when MQ

[Marc-André Lureau]

On Thu, May 4, 2017 at 8:30 PM Zhiyong Yang <zhiyong.yang@intel.com> wrote:

>  Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
> to cause failures of new connection when negotiating to set MQ.
> (one queue pair works well).
>    Because there exist some bugs in qemu code when introducing
> VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
> is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
> for the second time, qemu indeed doesn't send the messge (The message
> needs to be sent only once)but still will be waiting for dpdk's reply
> ack, then, qemu is always freezing, while DPDK is always waiting for
> next vhost message from qemu.
>   The patch aims to fix the bug, MQ can work well.
>   The same bug is found in function vhost_user_net_set_mtu, it is fixed
> at the same time.
>   DPDK related patch is as following:
>   http://www.dpdk.org/dev/patchwork/patch/23955/
>
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


---
>
> Changes in V2:
> Thanks for Maxime's suggestion, if one-time request, clear the
> VHOST_USER_NEED_REPLY flag in function vhost_user_write,
> in process_message_reply(), return early, if this flag isn't set.
>
>  hw/virtio/vhost-user.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 9334a8a..32a95a8 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -163,22 +163,26 @@ fail:
>  }
>
>  static int process_message_reply(struct vhost_dev *dev,
> -                                 VhostUserRequest request)
> +                                 VhostUserMsg msg)
>  {
> -    VhostUserMsg msg;
> +    VhostUserMsg msg_reply;
>
> -    if (vhost_user_read(dev, &msg) < 0) {
> +    if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
> +        return 0;
> +    }
> +
> +    if (vhost_user_read(dev, &msg_reply) < 0) {
>          return -1;
>      }
>
> -    if (msg.request != request) {
> +    if (msg_reply.request != msg.request) {
>          error_report("Received unexpected msg type."
>                       "Expected %d received %d",
> -                     request, msg.request);
> +                     msg.request, msg_reply.request);
>          return -1;
>      }
>
> -    return msg.payload.u64 ? -1 : 0;
> +    return msg_reply.payload.u64 ? -1 : 0;
>  }
>
>  static bool vhost_user_one_time_request(VhostUserRequest request)
> @@ -208,6 +212,7 @@ static int vhost_user_write(struct vhost_dev *dev,
> VhostUserMsg *msg,
>       * request, we just ignore it.
>       */
>      if (vhost_user_one_time_request(msg->request) && dev->vq_index != 0) {
> +        msg->flags &= ~VHOST_USER_NEED_REPLY_MASK;
>          return 0;
>      }
>
> @@ -320,7 +325,7 @@ static int vhost_user_set_mem_table(struct vhost_dev
> *dev,
>      }
>
>      if (reply_supported) {
> -        return process_message_reply(dev, msg.request);
> +        return process_message_reply(dev, msg);
>      }
>
>      return 0;
> @@ -712,7 +717,7 @@ static int vhost_user_net_set_mtu(struct vhost_dev
> *dev, uint16_t mtu)
>
>      /* If reply_ack supported, slave has to ack specified MTU is valid */
>      if (reply_supported) {
> -        return process_message_reply(dev, msg.request);
> +        return process_message_reply(dev, msg);
>      }
>
>      return 0;
> --
> 2.7.4
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2] hw/virtio: fix vhost user fails to startup when MQ

[Jens Freimann]

On Fri, May 05, 2017 at 12:25:36AM +0800, Zhiyong Yang wrote:
>  Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
> to cause failures of new connection when negotiating to set MQ.
> (one queue pair works well).
>    Because there exist some bugs in qemu code when introducing
> VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
> is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
> for the second time, qemu indeed doesn't send the messge (The message
> needs to be sent only once)but still will be waiting for dpdk's reply
> ack, then, qemu is always freezing, while DPDK is always waiting for
> next vhost message from qemu.
>   The patch aims to fix the bug, MQ can work well.
>   The same bug is found in function vhost_user_net_set_mtu, it is fixed
> at the same time.
>   DPDK related patch is as following:
>   http://www.dpdk.org/dev/patchwork/patch/23955/
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>

Tested-by: Jens Freimann <jfreiman@redhat.com>

Re: [Qemu-devel] [PATCH v2] hw/virtio: fix vhost user fails to startup when MQ

[Maxime Coquelin]

Hi,

On 05/04/2017 06:25 PM, Zhiyong Yang wrote:
>   Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
> to cause failures of new connection when negotiating to set MQ.
> (one queue pair works well).
>     Because there exist some bugs in qemu code when introducing
> VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
> is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
> for the second time, qemu indeed doesn't send the messge (The message
> needs to be sent only once)but still will be waiting for dpdk's reply
> ack, then, qemu is always freezing, while DPDK is always waiting for
> next vhost message from qemu.
>    The patch aims to fix the bug, MQ can work well.
>    The same bug is found in function vhost_user_net_set_mtu, it is fixed
> at the same time.
>    DPDK related patch is as following:
>    http://www.dpdk.org/dev/patchwork/patch/23955/
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
> 
> Changes in V2:
> Thanks for Maxime's suggestion, if one-time request, clear the
> VHOST_USER_NEED_REPLY flag in function vhost_user_write,
> in process_message_reply(), return early, if this flag isn't set.
> 
>   hw/virtio/vhost-user.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 

Thanks for the quick fix, it looks good to me.
I forgot to ask you to cc qemu-stable, and reference the faulty commit:

Cc: qemu-stable@nongnu.org
Fixes: ca525ce5618b ("vhost-user: Introduce a new protocol feature 
REPLY_ACK.")

Maybe this can be amended when applied. Michael?

Other than that, feel free to add my:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [Qemu-devel] [PATCH v2] hw/virtio: fix vhost user fails to startup when MQ

[Michael S. Tsirkin]

On Fri, May 05, 2017 at 09:34:14AM +0200, Maxime Coquelin wrote:
> Hi,
> 
> On 05/04/2017 06:25 PM, Zhiyong Yang wrote:
> >   Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
> > to cause failures of new connection when negotiating to set MQ.
> > (one queue pair works well).
> >     Because there exist some bugs in qemu code when introducing
> > VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
> > is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
> > for the second time, qemu indeed doesn't send the messge (The message
> > needs to be sent only once)but still will be waiting for dpdk's reply
> > ack, then, qemu is always freezing, while DPDK is always waiting for
> > next vhost message from qemu.
> >    The patch aims to fix the bug, MQ can work well.
> >    The same bug is found in function vhost_user_net_set_mtu, it is fixed
> > at the same time.
> >    DPDK related patch is as following:
> >    http://www.dpdk.org/dev/patchwork/patch/23955/
> > 
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> > 
> > Changes in V2:
> > Thanks for Maxime's suggestion, if one-time request, clear the
> > VHOST_USER_NEED_REPLY flag in function vhost_user_write,
> > in process_message_reply(), return early, if this flag isn't set.
> > 
> >   hw/virtio/vhost-user.c | 21 +++++++++++++--------
> >   1 file changed, 13 insertions(+), 8 deletions(-)
> > 
> 
> Thanks for the quick fix, it looks good to me.
> I forgot to ask you to cc qemu-stable, and reference the faulty commit:
> 
> Cc: qemu-stable@nongnu.org
> Fixes: ca525ce5618b ("vhost-user: Introduce a new protocol feature
> REPLY_ACK.")
> 
> Maybe this can be amended when applied. Michael?

Sure. np.

> Other than that, feel free to add my:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime