All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.