[v3,6/8] vhost: add support for virtio get status message

Message ID 20200706112452.1474533-7-amorenoz@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: improve Vhost/vDPA device init |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Adrian Moreno July 6, 2020, 11:24 a.m. UTC
  This patch adds support to the new Virtio device get status
Vhost-user message.

The driver can send this new message to read the device status.

One of the uses of this message is to ensure the feature negotiation has
succeded.  According to the virtio spec, after completing the feature
negotiation, the driver sets the FEATURE_OK status bit and re-reads it
to ensure the device has accepted the features.

This patch also clears the FEATURE_OK status bit if the feature
negotiation has failed to let the driver know about his failure.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 lib/librte_vhost/vhost.h      |  2 ++
 lib/librte_vhost/vhost_user.c | 32 ++++++++++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.h |  1 +
 3 files changed, 35 insertions(+)
  

Comments

Chenbo Xia July 7, 2020, 3:22 a.m. UTC | #1
> -----Original Message-----
> From: Adrian Moreno <amorenoz@redhat.com>
> Sent: Monday, July 6, 2020 7:25 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Ye, Xiaolong
> <xiaolong.ye@intel.com>; shahafs@mellanox.com; matan@mellanox.com;
> maxime.coquelin@redhat.com; Wang, Xiao W <xiao.w.wang@intel.com>;
> viacheslavo@mellanox.com
> Cc: jasowang@redhat.com; lulu@redhat.com; Adrian Moreno
> <amorenoz@redhat.com>
> Subject: [PATCH v3 6/8] vhost: add support for virtio get status message
> 
> This patch adds support to the new Virtio device get status Vhost-user message.
> 
> The driver can send this new message to read the device status.
> 
> One of the uses of this message is to ensure the feature negotiation has
> succeded.  According to the virtio spec, after completing the feature negotiation,
> the driver sets the FEATURE_OK status bit and re-reads it to ensure the device
> has accepted the features.
> 
> This patch also clears the FEATURE_OK status bit if the feature negotiation has
> failed to let the driver know about his failure.
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  lib/librte_vhost/vhost.h      |  2 ++
>  lib/librte_vhost/vhost_user.c | 32 ++++++++++++++++++++++++++++++++
> lib/librte_vhost/vhost_user.h |  1 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h index
> 25d31c71b..e743821cc 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -32,6 +32,8 @@
>  #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
>  /* Used to indicate that the device has its own data path and configured */
> #define VIRTIO_DEV_VDPA_CONFIGURED 8
> +/* Used to indicate that the feature negotiation failed */ #define
> +VIRTIO_DEV_FEATURES_FAILED 16
> 
>  /* Backend value set by guest. */
>  #define VIRTIO_DEV_STOPPED -1
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index
> 8d3d13913..5b6f4fb62 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -88,6 +88,7 @@ static const char *vhost_message_str[VHOST_USER_MAX]
> = {
>  	[VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
>  	[VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
>  	[VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
> +	[VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
>  };
> 
>  static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg); @@ -339,6
> +340,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct
> VhostUserMsg *msg,
>  		VHOST_LOG_CONFIG(ERR,
>  			"(%d) received invalid negotiated features.\n",
>  			dev->vid);
> +		dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
> +		dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
> +
>  		return RTE_VHOST_MSG_RESULT_ERR;
>  	}
> 
> @@ -402,6 +406,7 @@ vhost_user_set_features(struct virtio_net **pdev, struct
> VhostUserMsg *msg,
>  	if (vdpa_dev)
>  		vdpa_dev->ops->set_features(dev->vid);
> 
> +	dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
>  	return RTE_VHOST_MSG_RESULT_OK;
>  }
> 
> @@ -2458,6 +2463,22 @@ vhost_user_postcopy_end(struct virtio_net **pdev,
> struct VhostUserMsg *msg,
>  	return RTE_VHOST_MSG_RESULT_REPLY;
>  }
> 
> +static int
> +vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
> +		      int main_fd __rte_unused)
> +{
> +	struct virtio_net *dev = *pdev;
> +
> +	if (validate_msg_fds(msg, 0) != 0)
> +		return RTE_VHOST_MSG_RESULT_ERR;
> +
> +	msg->payload.u64 = dev->status;
> +	msg->size = sizeof(msg->payload.u64);
> +	msg->fd_num = 0;
> +
> +	return RTE_VHOST_MSG_RESULT_REPLY;
> +}
> +
>  static int
>  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
>  			int main_fd __rte_unused)
> @@ -2476,6 +2497,16 @@ vhost_user_set_status(struct virtio_net **pdev,
> struct VhostUserMsg *msg,
> 
>  	dev->status = msg->payload.u64;
> 
> +	if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
> +	    (dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
> +		VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature
> negotiation failed\n");
> +		/*
> +		 * Clear the bit to let the driver know about the feature
> +		 * negotiation failure
> +		 */
> +		dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
> +	}
> +
>  	VHOST_LOG_CONFIG(INFO, "New device status(0x%08x):\n"
>  			"\t-ACKNOWLEDGE: %u\n"
>  			"\t-DRIVER: %u\n"
> @@ -2527,6 +2558,7 @@ static vhost_message_handler_t
> vhost_message_handlers[VHOST_USER_MAX] = {
>  	[VHOST_USER_GET_INFLIGHT_FD] = vhost_user_get_inflight_fd,
>  	[VHOST_USER_SET_INFLIGHT_FD] = vhost_user_set_inflight_fd,
>  	[VHOST_USER_SET_STATUS] = vhost_user_set_status,
> +	[VHOST_USER_GET_STATUS] = vhost_user_get_status,
>  };
> 
>  /* return bytes# of read on success or negative val on failure. */ diff --git
> a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h index
> 82885ab5e..16fe03f88 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -58,6 +58,7 @@ typedef enum VhostUserRequest {
>  	VHOST_USER_GET_INFLIGHT_FD = 31,
>  	VHOST_USER_SET_INFLIGHT_FD = 32,
>  	VHOST_USER_SET_STATUS = 39,
> +	VHOST_USER_GET_STATUS = 40,
>  	VHOST_USER_MAX = 41
>  } VhostUserRequest;
> 
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

Patch

diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 25d31c71b..e743821cc 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -32,6 +32,8 @@ 
 #define VIRTIO_DEV_BUILTIN_VIRTIO_NET 4
 /* Used to indicate that the device has its own data path and configured */
 #define VIRTIO_DEV_VDPA_CONFIGURED 8
+/* Used to indicate that the feature negotiation failed */
+#define VIRTIO_DEV_FEATURES_FAILED 16
 
 /* Backend value set by guest. */
 #define VIRTIO_DEV_STOPPED -1
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 8d3d13913..5b6f4fb62 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -88,6 +88,7 @@  static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_INFLIGHT_FD] = "VHOST_USER_GET_INFLIGHT_FD",
 	[VHOST_USER_SET_INFLIGHT_FD] = "VHOST_USER_SET_INFLIGHT_FD",
 	[VHOST_USER_SET_STATUS] = "VHOST_USER_SET_STATUS",
+	[VHOST_USER_GET_STATUS] = "VHOST_USER_GET_STATUS",
 };
 
 static int send_vhost_reply(int sockfd, struct VhostUserMsg *msg);
@@ -339,6 +340,9 @@  vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 		VHOST_LOG_CONFIG(ERR,
 			"(%d) received invalid negotiated features.\n",
 			dev->vid);
+		dev->flags |= VIRTIO_DEV_FEATURES_FAILED;
+		dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
+
 		return RTE_VHOST_MSG_RESULT_ERR;
 	}
 
@@ -402,6 +406,7 @@  vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	if (vdpa_dev)
 		vdpa_dev->ops->set_features(dev->vid);
 
+	dev->flags &= ~VIRTIO_DEV_FEATURES_FAILED;
 	return RTE_VHOST_MSG_RESULT_OK;
 }
 
@@ -2458,6 +2463,22 @@  vhost_user_postcopy_end(struct virtio_net **pdev, struct VhostUserMsg *msg,
 	return RTE_VHOST_MSG_RESULT_REPLY;
 }
 
+static int
+vhost_user_get_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
+		      int main_fd __rte_unused)
+{
+	struct virtio_net *dev = *pdev;
+
+	if (validate_msg_fds(msg, 0) != 0)
+		return RTE_VHOST_MSG_RESULT_ERR;
+
+	msg->payload.u64 = dev->status;
+	msg->size = sizeof(msg->payload.u64);
+	msg->fd_num = 0;
+
+	return RTE_VHOST_MSG_RESULT_REPLY;
+}
+
 static int
 vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
@@ -2476,6 +2497,16 @@  vhost_user_set_status(struct virtio_net **pdev, struct VhostUserMsg *msg,
 
 	dev->status = msg->payload.u64;
 
+	if ((dev->status & VIRTIO_DEVICE_STATUS_FEATURES_OK) &&
+	    (dev->flags & VIRTIO_DEV_FEATURES_FAILED)) {
+		VHOST_LOG_CONFIG(ERR, "FEATURES_OK bit is set but feature negotiation failed\n");
+		/*
+		 * Clear the bit to let the driver know about the feature
+		 * negotiation failure
+		 */
+		dev->status &= ~VIRTIO_DEVICE_STATUS_FEATURES_OK;
+	}
+
 	VHOST_LOG_CONFIG(INFO, "New device status(0x%08x):\n"
 			"\t-ACKNOWLEDGE: %u\n"
 			"\t-DRIVER: %u\n"
@@ -2527,6 +2558,7 @@  static vhost_message_handler_t vhost_message_handlers[VHOST_USER_MAX] = {
 	[VHOST_USER_GET_INFLIGHT_FD] = vhost_user_get_inflight_fd,
 	[VHOST_USER_SET_INFLIGHT_FD] = vhost_user_set_inflight_fd,
 	[VHOST_USER_SET_STATUS] = vhost_user_set_status,
+	[VHOST_USER_GET_STATUS] = vhost_user_get_status,
 };
 
 /* return bytes# of read on success or negative val on failure. */
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 82885ab5e..16fe03f88 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -58,6 +58,7 @@  typedef enum VhostUserRequest {
 	VHOST_USER_GET_INFLIGHT_FD = 31,
 	VHOST_USER_SET_INFLIGHT_FD = 32,
 	VHOST_USER_SET_STATUS = 39,
+	VHOST_USER_GET_STATUS = 40,
 	VHOST_USER_MAX = 41
 } VhostUserRequest;