QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support
@ 2015-09-14  8:58 Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 1/6] vhost-user: use VHOST_USER_XXX macro for switch statement Yuanhan Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

Hi,

Here is the updated patch set for enabling vhost-user multiple queue.

This patch set introduces 2 more vhost user messages: VHOST_USER_GET_QUEUE_NUM,
for querying how many queues the backend supports, and VHOST_USER_SET_VRING_ENABLE,
for enabling/disabling a specific virt queue.

Both of the two new messages are treated as vhost protocol extension,
and that's why Michaels's patch "vhost-user: add protocol feature
negotiation" is also included here.

Patch 1-4 are all prepare works for actually enabling multiple queue.

Patch 5 is the major patch for enabling multiple queue, which also tries
to address two major concerns from Michael: no feedback from backend if
it can't support # of requested queues, and all messages are sent N time.
It also fixes a hidden bug.

Patch 6 introduces the VHOST_USER_SET_VRING_ENABLE message, to enable
or disable a specific vring.

Note that I haven't done any formal test yet, it just passes build
test and basic functional test, such as it does exit when backend
doesn't support # of requested queues. Here I sent it out just for
more comments, and for avoiding spending too much effort on the wrong
track.


v8: - set vq_index for vhost-user inside vhost_net_init() based on the
      net->nc->queue_index field. Per Jason Wang suggested, we can't
      move the vhost_net_set_vq_index() ahead to inside of vhost_net_init(),
      as it will break vhost-kernel multiple queue support(with tap).
      And that's why it's one patch less in this version.


    - Rename VHOST_USER_SET_VRING_FLAG to VHOST_USER_SET_VRING_ENABLE.


Thanks.

    --yliu

---
Changchun Ouyang (2):
  vhost-user: add multiple queue support
  vhost-user: add a new message to disable/enable a specific virt queue.

Michael S. Tsirkin (1):
  vhost-user: add protocol feature negotiation

Yuanhan Liu (3):
  vhost-user: use VHOST_USER_XXX macro for switch statement
  vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE
  vhost-user: add VHOST_USER_GET_QUEUE_NUM message

 docs/specs/vhost-user.txt         |  75 +++++++++++++++++++-
 hw/net/vhost_net.c                |  34 ++++++++-
 hw/net/virtio-net.c               |   8 +++
 hw/virtio/vhost-user.c            | 140 ++++++++++++++++++++++++++++++++------
 include/hw/virtio/vhost-backend.h |   2 +
 include/hw/virtio/vhost.h         |   2 +
 include/net/net.h                 |   1 +
 include/net/vhost_net.h           |   3 +
 linux-headers/linux/vhost.h       |   2 +-
 net/vhost-user.c                  | 140 +++++++++++++++++++++++++++-----------
 qapi-schema.json                  |   6 +-
 qemu-options.hx                   |   5 +-
 tests/vhost-user-test.c           |   2 +-
 13 files changed, 350 insertions(+), 70 deletions(-)

-- 
1.9.0

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

* [Qemu-devel] [PATCH 1/6] vhost-user: use VHOST_USER_XXX macro for switch statement
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 2/6] vhost-user: add protocol feature negotiation Yuanhan Liu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

So that we could let vhost_user_call to handle extented requests,
such as VHOST_USER_GET/SET_PROTOCOL_FEATURES, instead of invoking
vhost_user_read/write and constructing the msg again by ourself.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 hw/virtio/vhost-user.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e7ab829..13677ac 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -193,27 +193,33 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
 
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
-    msg_request = vhost_user_request_translate(request);
+    /* only translate vhost ioctl requests */
+    if (request > VHOST_USER_MAX) {
+        msg_request = vhost_user_request_translate(request);
+    } else {
+        msg_request = request;
+    }
+
     msg.request = msg_request;
     msg.flags = VHOST_USER_VERSION;
     msg.size = 0;
 
-    switch (request) {
-    case VHOST_GET_FEATURES:
+    switch (msg_request) {
+    case VHOST_USER_GET_FEATURES:
         need_reply = 1;
         break;
 
-    case VHOST_SET_FEATURES:
-    case VHOST_SET_LOG_BASE:
+    case VHOST_USER_SET_FEATURES:
+    case VHOST_USER_SET_LOG_BASE:
         msg.u64 = *((__u64 *) arg);
         msg.size = sizeof(m.u64);
         break;
 
-    case VHOST_SET_OWNER:
-    case VHOST_RESET_OWNER:
+    case VHOST_USER_SET_OWNER:
+    case VHOST_USER_RESET_OWNER:
         break;
 
-    case VHOST_SET_MEM_TABLE:
+    case VHOST_USER_SET_MEM_TABLE:
         for (i = 0; i < dev->mem->nregions; ++i) {
             struct vhost_memory_region *reg = dev->mem->regions + i;
             ram_addr_t ram_addr;
@@ -246,30 +252,30 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
 
         break;
 
-    case VHOST_SET_LOG_FD:
+    case VHOST_USER_SET_LOG_FD:
         fds[fd_num++] = *((int *) arg);
         break;
 
-    case VHOST_SET_VRING_NUM:
-    case VHOST_SET_VRING_BASE:
+    case VHOST_USER_SET_VRING_NUM:
+    case VHOST_USER_SET_VRING_BASE:
         memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
         msg.size = sizeof(m.state);
         break;
 
-    case VHOST_GET_VRING_BASE:
+    case VHOST_USER_GET_VRING_BASE:
         memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
         msg.size = sizeof(m.state);
         need_reply = 1;
         break;
 
-    case VHOST_SET_VRING_ADDR:
+    case VHOST_USER_SET_VRING_ADDR:
         memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
         msg.size = sizeof(m.addr);
         break;
 
-    case VHOST_SET_VRING_KICK:
-    case VHOST_SET_VRING_CALL:
-    case VHOST_SET_VRING_ERR:
+    case VHOST_USER_SET_VRING_KICK:
+    case VHOST_USER_SET_VRING_CALL:
+    case VHOST_USER_SET_VRING_ERR:
         file = arg;
         msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
         msg.size = sizeof(m.u64);
-- 
1.9.0

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

* [Qemu-devel] [PATCH 2/6] vhost-user: add protocol feature negotiation
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 1/6] vhost-user: use VHOST_USER_XXX macro for switch statement Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 3/6] vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE Yuanhan Liu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

From: "Michael S. Tsirkin" <mst@redhat.com>

Support a separate bitmask for vhost-user protocol features,
and messages to get/set protocol features.

Invoke them at init.

No features are defined yet.

v2: leverage vhost_user_call for request handling -- Yuanhan Liu

Signed-off-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt | 37 +++++++++++++++++++++++++++++++++++++
 hw/net/vhost_net.c        |  2 ++
 hw/virtio/vhost-user.c    | 31 +++++++++++++++++++++++++++++++
 include/hw/virtio/vhost.h |  1 +
 4 files changed, 71 insertions(+)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 650bb18..70da3b1 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -113,6 +113,7 @@ message replies. Most of the requests don't require replies. Here is a list of
 the ones that do:
 
  * VHOST_GET_FEATURES
+ * VHOST_GET_PROTOCOL_FEATURES
  * VHOST_GET_VRING_BASE
 
 There are several messages that the master sends with file descriptors passed
@@ -127,6 +128,13 @@ in the ancillary data:
 If Master is unable to send the full message or receives a wrong reply it will
 close the connection. An optional reconnection mechanism can be implemented.
 
+Any protocol extensions are gated by protocol feature bits,
+which allows full backwards compatibility on both master
+and slave.
+As older slaves don't support negotiating protocol features,
+a feature bit was dedicated for this purpose:
+#define VHOST_USER_F_PROTOCOL_FEATURES 30
+
 Message types
 -------------
 
@@ -138,6 +146,8 @@ Message types
       Slave payload: u64
 
       Get from the underlying vhost implementation the features bitmask.
+      Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals slave support for
+      VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES.
 
  * VHOST_USER_SET_FEATURES
 
@@ -146,6 +156,33 @@ Message types
       Master payload: u64
 
       Enable features in the underlying vhost implementation using a bitmask.
+      Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals slave support for
+      VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES.
+
+ * VHOST_USER_GET_PROTOCOL_FEATURES
+
+      Id: 15
+      Equivalent ioctl: VHOST_GET_FEATURES
+      Master payload: N/A
+      Slave payload: u64
+
+      Get the protocol feature bitmask from the underlying vhost implementation.
+      Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+      VHOST_USER_GET_FEATURES.
+      Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support
+      this message even before VHOST_USER_SET_FEATURES was called.
+
+ * VHOST_USER_SET_PROTOCOL_FEATURES
+
+      Id: 16
+      Ioctl: VHOST_SET_FEATURES
+      Master payload: u64
+
+      Enable protocol features in the underlying vhost implementation.
+      Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+      VHOST_USER_GET_FEATURES.
+      Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support
+      this message even before VHOST_USER_SET_FEATURES was called.
 
  * VHOST_USER_SET_OWNER
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 5c1d11f..c864237 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -152,8 +152,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
         net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
             ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
         net->backend = r;
+        net->dev.protocol_features = 0;
     } else {
         net->dev.backend_features = 0;
+        net->dev.protocol_features = 0;
         net->backend = -1;
     }
     net->nc = options->net_backend;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 13677ac..43169a1 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -24,6 +24,8 @@
 #include <linux/vhost.h>
 
 #define VHOST_MEMORY_MAX_NREGIONS    8
+#define VHOST_USER_F_PROTOCOL_FEATURES 30
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL
 
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
@@ -41,6 +43,8 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_KICK = 12,
     VHOST_USER_SET_VRING_CALL = 13,
     VHOST_USER_SET_VRING_ERR = 14,
+    VHOST_USER_GET_PROTOCOL_FEATURES = 15,
+    VHOST_USER_SET_PROTOCOL_FEATURES = 16,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -206,11 +210,13 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
 
     switch (msg_request) {
     case VHOST_USER_GET_FEATURES:
+    case VHOST_USER_GET_PROTOCOL_FEATURES:
         need_reply = 1;
         break;
 
     case VHOST_USER_SET_FEATURES:
     case VHOST_USER_SET_LOG_BASE:
+    case VHOST_USER_SET_PROTOCOL_FEATURES:
         msg.u64 = *((__u64 *) arg);
         msg.size = sizeof(m.u64);
         break;
@@ -308,6 +314,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
 
         switch (msg_request) {
         case VHOST_USER_GET_FEATURES:
+        case VHOST_USER_GET_PROTOCOL_FEATURES:
             if (msg.size != sizeof(m.u64)) {
                 error_report("Received bad msg size.");
                 return -1;
@@ -333,10 +340,34 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
 
 static int vhost_user_init(struct vhost_dev *dev, void *opaque)
 {
+    unsigned long long features;
+    int err;
+
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
     dev->opaque = opaque;
 
+    err = vhost_user_call(dev, VHOST_USER_GET_FEATURES, &features);
+    if (err < 0) {
+        return err;
+    }
+
+    if (__virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
+        dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
+
+        err = vhost_user_call(dev, VHOST_USER_GET_PROTOCOL_FEATURES, &features);
+        if (err < 0) {
+            return err;
+        }
+
+        dev->protocol_features = features & VHOST_USER_PROTOCOL_FEATURE_MASK;
+        err = vhost_user_call(dev, VHOST_USER_SET_PROTOCOL_FEATURES,
+                              &dev->protocol_features);
+        if (err < 0) {
+            return err;
+        }
+    }
+
     return 0;
 }
 
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index dd51050..6467c73 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -47,6 +47,7 @@ struct vhost_dev {
     unsigned long long features;
     unsigned long long acked_features;
     unsigned long long backend_features;
+    unsigned long long protocol_features;
     bool started;
     bool log_enabled;
     unsigned long long log_size;
-- 
1.9.0

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

* [Qemu-devel] [PATCH 3/6] vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 1/6] vhost-user: use VHOST_USER_XXX macro for switch statement Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 2/6] vhost-user: add protocol feature negotiation Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 4/6] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

Quote from Michael:

    We really should rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c          | 2 +-
 hw/virtio/vhost-user.c      | 6 +++---
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c     | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 70da3b1..ccbbcbb 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -194,10 +194,10 @@ Message types
       as an owner of the session. This can be used on the Slave as a
       "session start" flag.
 
- * VHOST_USER_RESET_OWNER
+ * VHOST_USER_RESET_DEVICE
 
       Id: 4
-      Equivalent ioctl: VHOST_RESET_OWNER
+      Equivalent ioctl: VHOST_RESET_DEVICE
       Master payload: N/A
 
       Issued when a new connection is about to be closed. The Master will no
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index c864237..33b0e97 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -287,7 +287,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
     } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
+            int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
                                           NULL);
             assert(r >= 0);
         }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 43169a1..0ac8252 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -32,7 +32,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_FEATURES = 1,
     VHOST_USER_SET_FEATURES = 2,
     VHOST_USER_SET_OWNER = 3,
-    VHOST_USER_RESET_OWNER = 4,
+    VHOST_USER_RESET_DEVICE = 4,
     VHOST_USER_SET_MEM_TABLE = 5,
     VHOST_USER_SET_LOG_BASE = 6,
     VHOST_USER_SET_LOG_FD = 7,
@@ -98,7 +98,7 @@ static unsigned long int ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
     VHOST_GET_FEATURES,     /* VHOST_USER_GET_FEATURES */
     VHOST_SET_FEATURES,     /* VHOST_USER_SET_FEATURES */
     VHOST_SET_OWNER,        /* VHOST_USER_SET_OWNER */
-    VHOST_RESET_OWNER,      /* VHOST_USER_RESET_OWNER */
+    VHOST_RESET_DEVICE,      /* VHOST_USER_RESET_DEVICE */
     VHOST_SET_MEM_TABLE,    /* VHOST_USER_SET_MEM_TABLE */
     VHOST_SET_LOG_BASE,     /* VHOST_USER_SET_LOG_BASE */
     VHOST_SET_LOG_FD,       /* VHOST_USER_SET_LOG_FD */
@@ -222,7 +222,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         break;
 
     case VHOST_USER_SET_OWNER:
-    case VHOST_USER_RESET_OWNER:
+    case VHOST_USER_RESET_DEVICE:
         break;
 
     case VHOST_USER_SET_MEM_TABLE:
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index ead86db..14a0160 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -78,7 +78,7 @@ struct vhost_memory {
 #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
 /* Give up ownership, and reset the device to default values.
  * Allows subsequent call to VHOST_OWNER_SET to succeed. */
-#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
+#define VHOST_RESET_DEVICE _IO(VHOST_VIRTIO, 0x02)
 
 /* Set up/modify memory layout */
 #define VHOST_SET_MEM_TABLE	_IOW(VHOST_VIRTIO, 0x03, struct vhost_memory)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 75fedf0..e301db7 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -58,7 +58,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_FEATURES = 1,
     VHOST_USER_SET_FEATURES = 2,
     VHOST_USER_SET_OWNER = 3,
-    VHOST_USER_RESET_OWNER = 4,
+    VHOST_USER_RESET_DEVICE = 4,
     VHOST_USER_SET_MEM_TABLE = 5,
     VHOST_USER_SET_LOG_BASE = 6,
     VHOST_USER_SET_LOG_FD = 7,
-- 
1.9.0

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

* [Qemu-devel] [PATCH 4/6] vhost-user: add VHOST_USER_GET_QUEUE_NUM message
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
                   ` (2 preceding siblings ...)
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 3/6] vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support Yuanhan Liu
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 6/6] vhost-user: add a new message to disable/enable a specific virt queue Yuanhan Liu
  5 siblings, 0 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

This is for querying how many queues the backend supports if it has mq
support(when VHOST_USER_PROTOCOL_F_MQ flag is set from the quried
protocol features).

vhost_net_get_max_queues() is the interface to export that value, and
to tell if the backend supports # of queues user requested, which is
done in the following patch.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt | 11 +++++++++++
 hw/net/vhost_net.c        |  7 +++++++
 hw/virtio/vhost-user.c    | 15 ++++++++++++++-
 include/hw/virtio/vhost.h |  1 +
 include/net/vhost_net.h   |  1 +
 5 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index ccbbcbb..43db9b4 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -301,3 +301,14 @@ Message types
       Bits (0-7) of the payload contain the vring index. Bit 8 is the
       invalid FD flag. This flag is set when there is no file descriptor
       in the ancillary data.
+
+ * VHOST_USER_GET_QUEUE_NUM
+
+      Id: 17
+      Equivalent ioctl: N/A
+      Master payload: N/A
+      Slave payload: u64
+
+      Query how many queues the backend supports. This request should be
+      sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
+      features by VHOST_USER_GET_PROTOCOL_FEATURES.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 33b0e97..f9441e9 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -122,6 +122,11 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
     vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features);
 }
 
+uint64_t vhost_net_get_max_queues(VHostNetState *net)
+{
+    return net->dev.max_queues;
+}
+
 static int vhost_net_get_fd(NetClientState *backend)
 {
     switch (backend->info->type) {
@@ -144,6 +149,8 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
         goto fail;
     }
 
+    net->dev.max_queues = 1;
+
     if (backend_kernel) {
         r = vhost_net_get_fd(options->net_backend);
         if (r < 0) {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 0ac8252..8b4b36b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -25,7 +25,9 @@
 
 #define VHOST_MEMORY_MAX_NREGIONS    8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+
+#define VHOST_USER_PROTOCOL_F_MQ    0
 
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
@@ -45,6 +47,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ERR = 14,
     VHOST_USER_GET_PROTOCOL_FEATURES = 15,
     VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+    VHOST_USER_GET_QUEUE_NUM = 17,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -211,6 +214,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
     switch (msg_request) {
     case VHOST_USER_GET_FEATURES:
     case VHOST_USER_GET_PROTOCOL_FEATURES:
+    case VHOST_USER_GET_QUEUE_NUM:
         need_reply = 1;
         break;
 
@@ -315,6 +319,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         switch (msg_request) {
         case VHOST_USER_GET_FEATURES:
         case VHOST_USER_GET_PROTOCOL_FEATURES:
+        case VHOST_USER_GET_QUEUE_NUM:
             if (msg.size != sizeof(m.u64)) {
                 error_report("Received bad msg size.");
                 return -1;
@@ -366,6 +371,14 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
         if (err < 0) {
             return err;
         }
+
+        /* query the max queues we support if backend supports Multiple Queue */
+        if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
+            err = vhost_user_call(dev, VHOST_USER_GET_QUEUE_NUM, &dev->max_queues);
+            if (err < 0) {
+                return err;
+            }
+        }
     }
 
     return 0;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 6467c73..c3758f3 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -48,6 +48,7 @@ struct vhost_dev {
     unsigned long long acked_features;
     unsigned long long backend_features;
     unsigned long long protocol_features;
+    unsigned long long max_queues;
     bool started;
     bool log_enabled;
     unsigned long long log_size;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 840d4b1..8db723e 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -13,6 +13,7 @@ typedef struct VhostNetOptions {
     void *opaque;
 } VhostNetOptions;
 
+uint64_t vhost_net_get_max_queues(VHostNetState *net);
 struct vhost_net *vhost_net_init(VhostNetOptions *options);
 
 int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int total_queues);
-- 
1.9.0

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

* [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
                   ` (3 preceding siblings ...)
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 4/6] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  2015-09-14 10:06   ` Jason Wang
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 6/6] vhost-user: add a new message to disable/enable a specific virt queue Yuanhan Liu
  5 siblings, 1 reply; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

From: Changchun Ouyang <changchun.ouyang@intel.com>

This patch is initially based a patch from Nikolay Nikolaev.

Here is the latest version for adding vhost-user multiple queue support,
by creating a nc and vhost_net pair for each queue.

What differs from last version is that this patch addresses two major
concerns from Michael, and fixes one hidden bug.

- Concern #1: no feedback when the backend can't support # of
  requested queues(by providing queues=# option).

  Here we address this issue by querying VHOST_USER_PROTOCOL_F_MQ
  protocol features first, if not set, it means the backend don't
  support mq feature, and let max_queues be 1. Otherwise, we send
  another message, VHOST_USER_GET_QUEUE_NUM, for getting the max_queues
  the backend supports.

  At vhost-user initiation stage(net_vhost_user_start), we then initiate
  one queue first, which, in the meantime, also gets the max_queues.
  We then do a simple compare: if requested_queues > max_queues, we
  exit(I guess it's safe to exit here, as the vm is not running yet).

- Concern #2: some messages are sent more times than necessary.

  We came an agreement with Michael that we could categorize vhost
  user messages to 2 types: none-vring specific messages, which should
  be sent only once, and vring specific messages, which should be sent
  per queue.

  Here I introduced a helper function vhost_user_one_time_request(),
  which lists following messages as none-vring specific messages:

        VHOST_USER_GET_FEATURES
        VHOST_USER_SET_FEATURES
        VHOST_USER_GET_PROTOCOL_FEATURES
        VHOST_USER_SET_PROTOCOL_FEATURES
        VHOST_USER_SET_OWNER
        VHOST_USER_RESET_DEVICE
        VHOST_USER_SET_MEM_TABLE
        VHOST_USER_GET_QUEUE_NUM

  For above messages, we simply ignore them when they are not sent the first
  time.

I also observed a hidden bug from last version. We register the char dev
event handler N times, which is not necessary, as well as buggy: A later
register overwrites the former one, as qemu_chr_add_handlers() will not
chain those handlers, but instead overwrites the old one. So, in theory,
invoking qemu_chr_add_handlers N times will not end up with calling the
handler N times.

However, the reason the handler is invoked N times is because we start
the backend(the connection server) first, and hence when net_vhost_user_init()
is executed, the connection is already established, and qemu_chr_add_handlers()
then invoke the handler immediately, which just looks like we invoke the
handler(net_vhost_user_event) directly from net_vhost_user_init().

The solution I came up with is to make VhostUserState as an upper level
structure, making it includes N nc and vhost_net pairs:

   typedef struct VhostUserNetPair {
       NetClientState *nc;
       VHostNetState  *vhost_net;
   } VhostUserNetPair;

   typedef struct VhostUserState {
       CharDriverState *chr;
       bool running;
       int queues;
       struct VhostUserNetPair pairs[];
   } VhostUserState;

v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
    value from net->nc->queue_index.

Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt |  13 +++++
 hw/net/vhost_net.c        |   5 +-
 hw/virtio/vhost-user.c    |  32 ++++++++++-
 include/net/net.h         |   1 +
 net/vhost-user.c          | 140 +++++++++++++++++++++++++++++++++-------------
 qapi-schema.json          |   6 +-
 qemu-options.hx           |   5 +-
 7 files changed, 157 insertions(+), 45 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 43db9b4..acf5708 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -135,6 +135,19 @@ As older slaves don't support negotiating protocol features,
 a feature bit was dedicated for this purpose:
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+Multiple queue support
+-------------------
+Multiple queue is treated as a protocol extension, hence the slave has to
+implement protocol features first. Multiple queues is supported only when
+the protocol feature VHOST_USER_PROTOCOL_F_MQ(bit 0) is set.
+
+The max number of queues the slave supports can be queried with message
+VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
+requested queues is bigger than that.
+
+As all queues share one connection, the master uses a unique index for each
+queue in the sent message to identify a specified queue.
+
 Message types
 -------------
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f9441e9..0eda2ed 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -148,6 +148,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
         fprintf(stderr, "vhost-net requires net backend to be setup\n");
         goto fail;
     }
+    net->nc = options->net_backend;
 
     net->dev.max_queues = 1;
 
@@ -164,8 +165,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
         net->dev.backend_features = 0;
         net->dev.protocol_features = 0;
         net->backend = -1;
+
+        /* vhost-user needs vq_index to initiate a specific queue pair */
+        net->dev.vq_index = net->nc->queue_index * 2;
     }
-    net->nc = options->net_backend;
 
     net->dev.nvqs = 2;
     net->dev.vqs = net->vqs;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8b4b36b..7922eb7 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -187,6 +187,23 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
             0 : -1;
 }
 
+static bool vhost_user_one_time_request(VhostUserRequest request)
+{
+    switch (request) {
+    case VHOST_USER_GET_FEATURES:
+    case VHOST_USER_SET_FEATURES:
+    case VHOST_USER_GET_PROTOCOL_FEATURES:
+    case VHOST_USER_SET_PROTOCOL_FEATURES:
+    case VHOST_USER_SET_OWNER:
+    case VHOST_USER_RESET_DEVICE:
+    case VHOST_USER_SET_MEM_TABLE:
+    case VHOST_USER_GET_QUEUE_NUM:
+        return true;
+    default:
+        return false;
+    }
+}
+
 static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         void *arg)
 {
@@ -207,6 +224,15 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         msg_request = request;
     }
 
+    /*
+     * For none-vring specific requests, like VHOST_USER_GET_FEATURES,
+     * we just need send it once in the first time. For later such
+     * request, we just ignore it.
+     */
+    if (vhost_user_one_time_request(msg_request) && dev->vq_index != 0) {
+        return 0;
+    }
+
     msg.request = msg_request;
     msg.flags = VHOST_USER_VERSION;
     msg.size = 0;
@@ -269,17 +295,20 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
     case VHOST_USER_SET_VRING_NUM:
     case VHOST_USER_SET_VRING_BASE:
         memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
+        msg.addr.index += dev->vq_index;
         msg.size = sizeof(m.state);
         break;
 
     case VHOST_USER_GET_VRING_BASE:
         memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
+        msg.addr.index += dev->vq_index;
         msg.size = sizeof(m.state);
         need_reply = 1;
         break;
 
     case VHOST_USER_SET_VRING_ADDR:
         memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
+        msg.addr.index += dev->vq_index;
         msg.size = sizeof(m.addr);
         break;
 
@@ -287,7 +316,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
     case VHOST_USER_SET_VRING_CALL:
     case VHOST_USER_SET_VRING_ERR:
         file = arg;
-        msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
+        msg.u64 = (file->index + dev->vq_index) & VHOST_USER_VRING_IDX_MASK;
         msg.size = sizeof(m.u64);
         if (ioeventfd_enabled() && file->fd > 0) {
             fds[fd_num++] = file->fd;
@@ -331,6 +360,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
                 error_report("Received bad msg size.");
                 return -1;
             }
+            msg.state.index -= dev->vq_index;
             memcpy(arg, &msg.state, sizeof(struct vhost_vring_state));
             break;
         default:
diff --git a/include/net/net.h b/include/net/net.h
index 6a6cbef..6f20656 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -92,6 +92,7 @@ struct NetClientState {
     NetClientDestructor *destructor;
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
+    void *opaque;
 };
 
 typedef struct NICState {
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 93dcecd..13539b3 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -15,10 +15,16 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 
+typedef struct VhostUserNetPair {
+    NetClientState *nc;
+    VHostNetState  *vhost_net;
+} VhostUserNetPair;
+
 typedef struct VhostUserState {
-    NetClientState nc;
     CharDriverState *chr;
-    VHostNetState *vhost_net;
+    bool running;
+    int queues;
+    VhostUserNetPair pairs[];
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -29,48 +35,81 @@ typedef struct VhostUserChardevProps {
 
 VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
 {
-    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    VhostUserState *s = nc->opaque;
     assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
-    return s->vhost_net;
-}
-
-static int vhost_user_running(VhostUserState *s)
-{
-    return (s->vhost_net) ? 1 : 0;
+    return s->pairs[nc->queue_index].vhost_net;
 }
 
 static int vhost_user_start(VhostUserState *s)
 {
     VhostNetOptions options;
+    VHostNetState *vhost_net;
+    int max_queues;
+    int i = 0;
 
-    if (vhost_user_running(s)) {
+    if (s->running) {
         return 0;
     }
 
     options.backend_type = VHOST_BACKEND_TYPE_USER;
-    options.net_backend = &s->nc;
     options.opaque = s->chr;
 
-    s->vhost_net = vhost_net_init(&options);
+    options.net_backend = s->pairs[i].nc;
+    vhost_net = s->pairs[i++].vhost_net = vhost_net_init(&options);
+
+    max_queues = vhost_net_get_max_queues(vhost_net);
+    if (s->queues > max_queues) {
+        error_report("you are asking more queues than supported: %d\n",
+                     max_queues);
+        return -1;
+    }
+
+    for (; i < s->queues; i++) {
+        options.net_backend = s->pairs[i].nc;
+
+        s->pairs[i].vhost_net = vhost_net_init(&options);
+        if (!s->pairs[i].vhost_net) {
+            return -1;
+        }
+    }
+    s->running = true;
 
-    return vhost_user_running(s) ? 0 : -1;
+    return 0;
 }
 
 static void vhost_user_stop(VhostUserState *s)
 {
-    if (vhost_user_running(s)) {
-        vhost_net_cleanup(s->vhost_net);
+    int i;
+    VHostNetState *vhost_net;
+
+    if (!s->running) {
+        return;
     }
 
-    s->vhost_net = 0;
+    for (i = 0;  i < s->queues; i++) {
+        vhost_net = s->pairs[i].vhost_net;
+        if (vhost_net) {
+            vhost_net_cleanup(vhost_net);
+        }
+    }
+
+    s->running = false;
 }
 
 static void vhost_user_cleanup(NetClientState *nc)
 {
-    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    VhostUserState *s = nc->opaque;
+    VHostNetState *vhost_net = s->pairs[nc->queue_index].vhost_net;
+
+    if (vhost_net) {
+        vhost_net_cleanup(vhost_net);
+    }
 
-    vhost_user_stop(s);
     qemu_purge_queued_packets(nc);
+
+    if (nc->queue_index == s->queues - 1) {
+        free(s);
+    }
 }
 
 static bool vhost_user_has_vnet_hdr(NetClientState *nc)
@@ -89,7 +128,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
 
 static NetClientInfo net_vhost_user_info = {
         .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
-        .size = sizeof(VhostUserState),
+        .size = sizeof(NetClientState),
         .cleanup = vhost_user_cleanup,
         .has_vnet_hdr = vhost_user_has_vnet_hdr,
         .has_ufo = vhost_user_has_ufo,
@@ -97,18 +136,25 @@ static NetClientInfo net_vhost_user_info = {
 
 static void net_vhost_link_down(VhostUserState *s, bool link_down)
 {
-    s->nc.link_down = link_down;
+    NetClientState *nc;
+    int i;
 
-    if (s->nc.peer) {
-        s->nc.peer->link_down = link_down;
-    }
+    for (i = 0; i < s->queues; i++) {
+        nc = s->pairs[i].nc;
 
-    if (s->nc.info->link_status_changed) {
-        s->nc.info->link_status_changed(&s->nc);
-    }
+        nc->link_down = link_down;
+
+        if (nc->peer) {
+            nc->peer->link_down = link_down;
+        }
 
-    if (s->nc.peer && s->nc.peer->info->link_status_changed) {
-        s->nc.peer->info->link_status_changed(s->nc.peer);
+        if (nc->info->link_status_changed) {
+            nc->info->link_status_changed(nc);
+        }
+
+        if (nc->peer && nc->peer->info->link_status_changed) {
+            nc->peer->info->link_status_changed(nc->peer);
+        }
     }
 }
 
@@ -118,7 +164,9 @@ static void net_vhost_user_event(void *opaque, int event)
 
     switch (event) {
     case CHR_EVENT_OPENED:
-        vhost_user_start(s);
+        if (vhost_user_start(s) < 0) {
+            exit(1);
+        }
         net_vhost_link_down(s, false);
         error_report("chardev \"%s\" went up", s->chr->label);
         break;
@@ -131,23 +179,28 @@ static void net_vhost_user_event(void *opaque, int event)
 }
 
 static int net_vhost_user_init(NetClientState *peer, const char *device,
-                               const char *name, CharDriverState *chr)
+                               const char *name, VhostUserState *s)
 {
     NetClientState *nc;
-    VhostUserState *s;
+    CharDriverState *chr = s->chr;
+    int i;
 
-    nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
+    for (i = 0; i < s->queues; i++) {
+        nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
 
-    snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to %s",
-             chr->label);
+        snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to %s",
+                 i, chr->label);
 
-    s = DO_UPCAST(VhostUserState, nc, nc);
+        /* We don't provide a receive callback */
+        nc->receive_disabled = 1;
 
-    /* We don't provide a receive callback */
-    s->nc.receive_disabled = 1;
-    s->chr = chr;
+        nc->queue_index = i;
+        nc->opaque      = s;
 
-    qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
+        s->pairs[i].nc = nc;
+    }
+
+    qemu_chr_add_handlers(chr, NULL, NULL, net_vhost_user_event, s);
 
     return 0;
 }
@@ -226,8 +279,10 @@ static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 int net_init_vhost_user(const NetClientOptions *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
+    int queues;
     const NetdevVhostUserOptions *vhost_user_opts;
     CharDriverState *chr;
+    VhostUserState *s;
 
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     vhost_user_opts = opts->vhost_user;
@@ -243,6 +298,11 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
         return -1;
     }
 
+    queues = vhost_user_opts->has_queues ? vhost_user_opts->queues : 1;
+    s = g_malloc0(sizeof(VhostUserState) +
+                  queues * sizeof(VhostUserNetPair));
+    s->queues = queues;
+    s->chr    = chr;
 
-    return net_vhost_user_init(peer, "vhost_user", name, chr);
+    return net_vhost_user_init(peer, "vhost_user", name, s);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index 67fef37..55c33db 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2480,12 +2480,16 @@
 #
 # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
 #
+# @queues: #optional number of queues to be created for multiqueue vhost-user
+#          (default: 1) (Since 2.5)
+#
 # Since 2.1
 ##
 { 'struct': 'NetdevVhostUserOptions',
   'data': {
     'chardev':        'str',
-    '*vhostforce':    'bool' } }
+    '*vhostforce':    'bool',
+    '*queues':        'int' } }
 
 ##
 # @NetClientOptions
diff --git a/qemu-options.hx b/qemu-options.hx
index 77f5853..5bfa7a3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1963,13 +1963,14 @@ The hubport netdev lets you connect a NIC to a QEMU "vlan" instead of a single
 netdev.  @code{-net} and @code{-device} with parameter @option{vlan} create the
 required hub automatically.
 
-@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off]
+@item -netdev vhost-user,chardev=@var{id}[,vhostforce=on|off][,queues=n]
 
 Establish a vhost-user netdev, backed by a chardev @var{id}. The chardev should
 be a unix domain socket backed one. The vhost-user uses a specifically defined
 protocol to pass vhost ioctl replacement messages to an application on the other
 end of the socket. On non-MSIX guests, the feature can be forced with
-@var{vhostforce}.
+@var{vhostforce}. Use 'queues=@var{n}' to specify the number of queues to
+be created for multiqueue vhost-user.
 
 Example:
 @example
-- 
1.9.0

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

* [Qemu-devel] [PATCH 6/6] vhost-user: add a new message to disable/enable a specific virt queue.
  2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
                   ` (4 preceding siblings ...)
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support Yuanhan Liu
@ 2015-09-14  8:58 ` Yuanhan Liu
  5 siblings, 0 replies; 8+ messages in thread
From: Yuanhan Liu @ 2015-09-14  8:58 UTC (permalink / raw
  To: qemu-devel; +Cc: jasowang, changchun.ouyang, Yuanhan Liu, mst

From: Changchun Ouyang <changchun.ouyang@intel.com>

Add a new message, VHOST_USER_SET_VRING_ENABLE, to enable or disable
a specific virt queue, which is similar to attach/detach queue for
tap device.

virtio driver on guest doesn't have to use max virt queue pair, it
could enable any number of virt queue ranging from 1 to max virt
queue pair.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 docs/specs/vhost-user.txt         | 12 +++++++++++-
 hw/net/vhost_net.c                | 18 ++++++++++++++++++
 hw/net/virtio-net.c               |  8 ++++++++
 hw/virtio/vhost-user.c            | 22 ++++++++++++++++++++--
 include/hw/virtio/vhost-backend.h |  2 ++
 include/net/vhost_net.h           |  2 ++
 6 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index acf5708..9c12875 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -146,7 +146,9 @@ VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
 requested queues is bigger than that.
 
 As all queues share one connection, the master uses a unique index for each
-queue in the sent message to identify a specified queue.
+queue in the sent message to identify a specified queue. One queue pairs
+is enabled initially. More queues are enabled dynamically, by sending
+message VHOST_USER_SET_VRING_ENABLE.
 
 Message types
 -------------
@@ -325,3 +327,11 @@ Message types
       Query how many queues the backend supports. This request should be
       sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
       features by VHOST_USER_GET_PROTOCOL_FEATURES.
+
+ * VHOST_USER_SET_VRING_ENABLE
+
+      Id: 18
+      Equivalent ioctl: N/A
+      Master payload: vring state description
+
+      Signal slave to enable or disable corresponding vring.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 0eda2ed..01b9c18 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -423,6 +423,19 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 
     return vhost_net;
 }
+
+int vhost_set_vring_enable(NetClientState *nc, int enable)
+{
+    VHostNetState *net = get_vhost_net(nc);
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+
+    if (vhost_ops->vhost_backend_set_vring_enable) {
+        return vhost_ops->vhost_backend_set_vring_enable(&net->dev, enable);
+    }
+
+    return 0;
+}
+
 #else
 struct vhost_net *vhost_net_init(VhostNetOptions *options)
 {
@@ -468,4 +481,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 {
     return 0;
 }
+
+int vhost_set_vring_enable(NetClientState *nc, int enable)
+{
+    return 0;
+}
 #endif
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8d28e45..3d41e22 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -406,6 +406,10 @@ static int peer_attach(VirtIONet *n, int index)
         return 0;
     }
 
+    if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+        vhost_set_vring_enable(nc->peer, 1);
+    }
+
     if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) {
         return 0;
     }
@@ -421,6 +425,10 @@ static int peer_detach(VirtIONet *n, int index)
         return 0;
     }
 
+    if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+        vhost_set_vring_enable(nc->peer, 0);
+    }
+
     if (nc->peer->info->type !=  NET_CLIENT_OPTIONS_KIND_TAP) {
         return 0;
     }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 7922eb7..842dd37 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -48,6 +48,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_PROTOCOL_FEATURES = 15,
     VHOST_USER_SET_PROTOCOL_FEATURES = 16,
     VHOST_USER_GET_QUEUE_NUM = 17,
+    VHOST_USER_SET_VRING_ENABLE = 18,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -298,6 +299,11 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
         msg.addr.index += dev->vq_index;
         msg.size = sizeof(m.state);
         break;
+    case VHOST_USER_SET_VRING_ENABLE:
+        msg.state.index = dev->vq_index;
+        msg.state.num = *(int*)arg;
+        msg.size = sizeof(msg.state);
+        break;
 
     case VHOST_USER_GET_VRING_BASE:
         memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
@@ -414,6 +420,17 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
     return 0;
 }
 
+static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
+{
+    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+    if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) {
+        return -1;
+    }
+
+    return vhost_user_call(dev, VHOST_USER_SET_VRING_ENABLE, &enable);
+}
+
 static int vhost_user_cleanup(struct vhost_dev *dev)
 {
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
@@ -427,5 +444,6 @@ const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_call = vhost_user_call,
         .vhost_backend_init = vhost_user_init,
-        .vhost_backend_cleanup = vhost_user_cleanup
-        };
+        .vhost_backend_cleanup = vhost_user_cleanup,
+        .vhost_backend_set_vring_enable = vhost_user_set_vring_enable,
+};
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index e472f29..d3b5a6c 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request,
              void *arg);
 typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
 typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
+typedef int (*vhost_backend_set_vring_enable)(struct vhost_dev *dev, int enable);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
     vhost_call vhost_call;
     vhost_backend_init vhost_backend_init;
     vhost_backend_cleanup vhost_backend_cleanup;
+    vhost_backend_set_vring_enable vhost_backend_set_vring_enable;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 8db723e..0188c4d 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -28,4 +28,6 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n);
 void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
                               int idx, bool mask);
 VHostNetState *get_vhost_net(NetClientState *nc);
+
+int vhost_set_vring_enable(NetClientState * nc, int enable);
 #endif
-- 
1.9.0

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

* Re: [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support
  2015-09-14  8:58 ` [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support Yuanhan Liu
@ 2015-09-14 10:06   ` Jason Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2015-09-14 10:06 UTC (permalink / raw
  To: Yuanhan Liu, qemu-devel; +Cc: mst, changchun.ouyang



On 09/14/2015 04:58 PM, Yuanhan Liu wrote:
> From: Changchun Ouyang <changchun.ouyang@intel.com>
>
> This patch is initially based a patch from Nikolay Nikolaev.
>
> Here is the latest version for adding vhost-user multiple queue support,
> by creating a nc and vhost_net pair for each queue.
>
> What differs from last version is that this patch addresses two major
> concerns from Michael, and fixes one hidden bug.
>
> - Concern #1: no feedback when the backend can't support # of
>   requested queues(by providing queues=# option).
>
>   Here we address this issue by querying VHOST_USER_PROTOCOL_F_MQ
>   protocol features first, if not set, it means the backend don't
>   support mq feature, and let max_queues be 1. Otherwise, we send
>   another message, VHOST_USER_GET_QUEUE_NUM, for getting the max_queues
>   the backend supports.
>
>   At vhost-user initiation stage(net_vhost_user_start), we then initiate
>   one queue first, which, in the meantime, also gets the max_queues.
>   We then do a simple compare: if requested_queues > max_queues, we
>   exit(I guess it's safe to exit here, as the vm is not running yet).
>
> - Concern #2: some messages are sent more times than necessary.
>
>   We came an agreement with Michael that we could categorize vhost
>   user messages to 2 types: none-vring specific messages, which should
>   be sent only once, and vring specific messages, which should be sent
>   per queue.
>
>   Here I introduced a helper function vhost_user_one_time_request(),
>   which lists following messages as none-vring specific messages:
>
>         VHOST_USER_GET_FEATURES
>         VHOST_USER_SET_FEATURES
>         VHOST_USER_GET_PROTOCOL_FEATURES
>         VHOST_USER_SET_PROTOCOL_FEATURES
>         VHOST_USER_SET_OWNER
>         VHOST_USER_RESET_DEVICE
>         VHOST_USER_SET_MEM_TABLE
>         VHOST_USER_GET_QUEUE_NUM
>
>   For above messages, we simply ignore them when they are not sent the first
>   time.
>
> I also observed a hidden bug from last version. We register the char dev
> event handler N times, which is not necessary, as well as buggy: A later
> register overwrites the former one, as qemu_chr_add_handlers() will not
> chain those handlers, but instead overwrites the old one. So, in theory,
> invoking qemu_chr_add_handlers N times will not end up with calling the
> handler N times.
>
> However, the reason the handler is invoked N times is because we start
> the backend(the connection server) first, and hence when net_vhost_user_init()
> is executed, the connection is already established, and qemu_chr_add_handlers()
> then invoke the handler immediately, which just looks like we invoke the
> handler(net_vhost_user_event) directly from net_vhost_user_init().
>
> The solution I came up with is to make VhostUserState as an upper level
> structure, making it includes N nc and vhost_net pairs:
>
>    typedef struct VhostUserNetPair {
>        NetClientState *nc;
>        VHostNetState  *vhost_net;
>    } VhostUserNetPair;
>
>    typedef struct VhostUserState {
>        CharDriverState *chr;
>        bool running;
>        int queues;
>        struct VhostUserNetPair pairs[];
>    } VhostUserState;
>
> v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
>     value from net->nc->queue_index.
>
> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  docs/specs/vhost-user.txt |  13 +++++
>  hw/net/vhost_net.c        |   5 +-
>  hw/virtio/vhost-user.c    |  32 ++++++++++-
>  include/net/net.h         |   1 +
>  net/vhost-user.c          | 140 +++++++++++++++++++++++++++++++++-------------
>  qapi-schema.json          |   6 +-
>  qemu-options.hx           |   5 +-
>  7 files changed, 157 insertions(+), 45 deletions(-)

Saw this version too late. I've some comments for last version and looks
like more of them still apply here. Please see there.

Thanks

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

end of thread, other threads:[~2015-09-14 10:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-14  8:58 [Qemu-devel] [PATCH 0/6 v8] vhost-user multiple queue support Yuanhan Liu
2015-09-14  8:58 ` [Qemu-devel] [PATCH 1/6] vhost-user: use VHOST_USER_XXX macro for switch statement Yuanhan Liu
2015-09-14  8:58 ` [Qemu-devel] [PATCH 2/6] vhost-user: add protocol feature negotiation Yuanhan Liu
2015-09-14  8:58 ` [Qemu-devel] [PATCH 3/6] vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE Yuanhan Liu
2015-09-14  8:58 ` [Qemu-devel] [PATCH 4/6] vhost-user: add VHOST_USER_GET_QUEUE_NUM message Yuanhan Liu
2015-09-14  8:58 ` [Qemu-devel] [PATCH 5/6] vhost-user: add multiple queue support Yuanhan Liu
2015-09-14 10:06   ` Jason Wang
2015-09-14  8:58 ` [Qemu-devel] [PATCH 6/6] vhost-user: add a new message to disable/enable a specific virt queue Yuanhan Liu

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 read-only IMAP folder(s) and NNTP newsgroup(s).