[v4] vhost: add config change slave msg support

Message ID 20191219085406.16524-1-fengli@smartx.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series [v4] vhost: add config change slave msg support |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail apply issues

Commit Message

Li Feng Dec. 19, 2019, 8:54 a.m. UTC
This msg is used to notify qemu that should get the config of backend.

For example, vhost-user-blk uses this msg to notify guest os the
capacity of backend has changed.

The need_reply flag is not mandatory because it will block the sender
thread and master process will send get_config message to fetch the
configuration, this need an extra thread to process the vhost message.

Signed-off-by: Li Feng <fengli@smartx.com>
---
v4:
* Fix type and code style.
* Add need_reply support.

v3:
* Move the declare to rte_vhost.h
* Add the symbol in rte_vhost_version.map

v2:
* Fix a little log typo.

 lib/librte_vhost/rte_vhost.h           | 15 +++++++++++++++
 lib/librte_vhost/rte_vhost_version.map |  1 +
 lib/librte_vhost/vhost_user.c          | 35 ++++++++++++++++++++++++++++++++++
 lib/librte_vhost/vhost_user.h          |  1 +
 4 files changed, 52 insertions(+)
  

Comments

Tiwei Bie Dec. 20, 2019, 8:04 a.m. UTC | #1
On Thu, Dec 19, 2019 at 04:54:06PM +0800, Li Feng wrote:
> This msg is used to notify qemu that should get the config of backend.
> 
> For example, vhost-user-blk uses this msg to notify guest os the
> capacity of backend has changed.
> 
> The need_reply flag is not mandatory because it will block the sender
> thread and master process will send get_config message to fetch the
> configuration, this need an extra thread to process the vhost message.
> 
> Signed-off-by: Li Feng <fengli@smartx.com>
> ---
> v4:
> * Fix type and code style.
> * Add need_reply support.
> 
> v3:
> * Move the declare to rte_vhost.h
> * Add the symbol in rte_vhost_version.map
> 
> v2:
> * Fix a little log typo.
> 
>  lib/librte_vhost/rte_vhost.h           | 15 +++++++++++++++
>  lib/librte_vhost/rte_vhost_version.map |  1 +
>  lib/librte_vhost/vhost_user.c          | 35 ++++++++++++++++++++++++++++++++++
>  lib/librte_vhost/vhost_user.h          |  1 +
>  4 files changed, 52 insertions(+)
> 
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index 7b5dc87c2..c7b619ae0 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -10,6 +10,7 @@
>   * Interface to vhost-user
>   */
>  
> +#include <stdbool.h>
>  #include <stdint.h>
>  #include <sys/eventfd.h>
>  
> @@ -977,6 +978,20 @@ __rte_experimental
>  int
>  rte_vhost_get_vdpa_device_id(int vid);
>  
> +/**
> + * Notify the guest that should get virtio configuration space from backend.
> + *
> + * @param vid
> + *  vhost device ID
> + * @param need_reply
> + *  wait for the master response the status of this operation
> + * @return
> + *  0 on success, < 0 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_slave_config_change(int vid, bool need_reply);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
> index c512377fe..051d08c12 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -65,4 +65,5 @@ EXPERIMENTAL {
>  	rte_vhost_clr_inflight_desc_packed;
>  	rte_vhost_get_vhost_ring_inflight;
>  	rte_vhost_get_vring_base_from_inflight;
> +	rte_vhost_slave_config_change;
>  };
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 0cfb8b792..aed1210b1 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -2840,6 +2840,41 @@ vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm)
>  	return 0;
>  }
>  
> +static int
> +vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply)
> +{
> +	int ret;
> +	uint32_t flags = VHOST_USER_VERSION;
> +	if (need_reply)
> +		flags |= VHOST_USER_NEED_REPLY;
> +	struct VhostUserMsg msg = {
> +		.request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
> +		.flags = flags,
> +		.size = 0,
> +	};

Normally, we don't mix the declarations and code.
Something like this looks better to me:

	struct VhostUserMsg msg = {
		.request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
		.flags = VHOST_USER_VERSION,
	};

	if (need_reply)
		msg.flags |= VHOST_USER_NEED_REPLY;

> +
> +	ret = send_vhost_slave_message(dev, &msg);
> +	if (ret < 0) {
> +		RTE_LOG(ERR, VHOST_CONFIG,
> +			"Failed to send config change (%d)\n",
> +			ret);
> +		return ret;
> +	}
> +	if (need_reply)
> +		return process_slave_message_reply(dev, &msg);

We can call process_slave_message_reply() unconditionally.
https://github.com/DPDK/dpdk/blob/f26c2b39b271/lib/librte_vhost/vhost_user.c#L2794-L2795

> +	return 0;
> +}
> +
> +int
> +rte_vhost_slave_config_change(int vid, bool need_reply)
> +{
> +	struct virtio_net *dev;

Looks better to add an empty line here.

> +	dev = get_device(vid);
> +	if (!dev)
> +		return -ENODEV;
> +	return vhost_user_slave_config_change(dev, need_reply);
> +}
> +
>  static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
>  						    int index, int fd,
>  						    uint64_t offset,
> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
> index 6563f7315..86c364a93 100644
> --- a/lib/librte_vhost/vhost_user.h
> +++ b/lib/librte_vhost/vhost_user.h
> @@ -62,6 +62,7 @@ typedef enum VhostUserRequest {
>  typedef enum VhostUserSlaveRequest {
>  	VHOST_USER_SLAVE_NONE = 0,
>  	VHOST_USER_SLAVE_IOTLB_MSG = 1,
> +	VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2,
>  	VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3,
>  	VHOST_USER_SLAVE_MAX
>  } VhostUserSlaveRequest;
> -- 
> 2.11.0
> 
> 
> -- 
> The SmartX email address is only for business purpose. Any sent message 
> that is not related to the business is not authorized or permitted by 
> SmartX.
> 本邮箱为北京志凌海纳科技有限公司(SmartX)工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
> 
>
  

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 7b5dc87c2..c7b619ae0 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -10,6 +10,7 @@ 
  * Interface to vhost-user
  */
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <sys/eventfd.h>
 
@@ -977,6 +978,20 @@  __rte_experimental
 int
 rte_vhost_get_vdpa_device_id(int vid);
 
+/**
+ * Notify the guest that should get virtio configuration space from backend.
+ *
+ * @param vid
+ *  vhost device ID
+ * @param need_reply
+ *  wait for the master response the status of this operation
+ * @return
+ *  0 on success, < 0 on failure
+ */
+__rte_experimental
+int
+rte_vhost_slave_config_change(int vid, bool need_reply);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index c512377fe..051d08c12 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -65,4 +65,5 @@  EXPERIMENTAL {
 	rte_vhost_clr_inflight_desc_packed;
 	rte_vhost_get_vhost_ring_inflight;
 	rte_vhost_get_vring_base_from_inflight;
+	rte_vhost_slave_config_change;
 };
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 0cfb8b792..aed1210b1 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -2840,6 +2840,41 @@  vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm)
 	return 0;
 }
 
+static int
+vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply)
+{
+	int ret;
+	uint32_t flags = VHOST_USER_VERSION;
+	if (need_reply)
+		flags |= VHOST_USER_NEED_REPLY;
+	struct VhostUserMsg msg = {
+		.request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG,
+		.flags = flags,
+		.size = 0,
+	};
+
+	ret = send_vhost_slave_message(dev, &msg);
+	if (ret < 0) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"Failed to send config change (%d)\n",
+			ret);
+		return ret;
+	}
+	if (need_reply)
+		return process_slave_message_reply(dev, &msg);
+	return 0;
+}
+
+int
+rte_vhost_slave_config_change(int vid, bool need_reply)
+{
+	struct virtio_net *dev;
+	dev = get_device(vid);
+	if (!dev)
+		return -ENODEV;
+	return vhost_user_slave_config_change(dev, need_reply);
+}
+
 static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev,
 						    int index, int fd,
 						    uint64_t offset,
diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h
index 6563f7315..86c364a93 100644
--- a/lib/librte_vhost/vhost_user.h
+++ b/lib/librte_vhost/vhost_user.h
@@ -62,6 +62,7 @@  typedef enum VhostUserRequest {
 typedef enum VhostUserSlaveRequest {
 	VHOST_USER_SLAVE_NONE = 0,
 	VHOST_USER_SLAVE_IOTLB_MSG = 1,
+	VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2,
 	VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3,
 	VHOST_USER_SLAVE_MAX
 } VhostUserSlaveRequest;