qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v11 2/7] vhost-user: add protocol feature negoti


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH v11 2/7] vhost-user: add protocol feature negotiation
Date: Thu, 24 Sep 2015 13:13:24 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 09/23/2015 07:19 AM, Yuanhan Liu wrote:
From: "Michael S. Tsirkin" <address@hidden>

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.

[ leverage vhost_user_call for request handling -- Yuanhan Liu ]

Signed-off-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Yuanhan Liu <address@hidden>
---
  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

VHOST_USER_GET_PROTOCOL_FEATURES does not have an equivalent ioctl

+      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

Same here

+      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 1d76b94..9d32d76 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;
      }

Maybe protocol_features assignment should be outside the if clause.
(assigned to 0 in both cases)

      net->nc = options->net_backend;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 13677ac..7fe35c6 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;


The above comments can be addressed on top of this series because
does not affect the correctness of the patch.

--
Reviewed-by: Marcel Apfelbaum <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]