[dpdk-dev,v2] vhost: provide vhost API to unregister vhost unix domain socket

Message ID 1433474786-704-1-git-send-email-huawei.xie@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

Huawei Xie June 5, 2015, 3:26 a.m. UTC
  rte_vhost_driver_unregister will remove the listenfd from event list, and then close it.

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Signed-off-by: Peng Sun <peng.a.sun@intel.com>
---
 lib/librte_vhost/rte_virtio_net.h            |  3 ++
 lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
 lib/librte_vhost/vhost_user/vhost-net-user.c | 68 +++++++++++++++++++++++-----
 lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
 4 files changed, 69 insertions(+), 13 deletions(-)
  

Comments

Loftus, Ciara June 5, 2015, 9:04 a.m. UTC | #1
> -----Original Message-----
> From: Xie, Huawei
> Sent: Friday, June 05, 2015 4:26 AM
> To: dev@dpdk.org
> Cc: Loftus, Ciara; Xie, Huawei; Sun, Peng A
> Subject: [PATCH v2] vhost: provide vhost API to unregister vhost unix domain
> socket
> 
> rte_vhost_driver_unregister will remove the listenfd from event list, and
> then close it.
> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
> ---
>  lib/librte_vhost/rte_virtio_net.h            |  3 ++
>  lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>  lib/librte_vhost/vhost_user/vhost-net-user.c | 68
> +++++++++++++++++++++++-----
>  lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
>  4 files changed, 69 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/librte_vhost/rte_virtio_net.h
> b/lib/librte_vhost/rte_virtio_net.h
> index 5d38185..5630fbc 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -188,6 +188,9 @@ int rte_vhost_enable_guest_notification(struct
> virtio_net *dev, uint16_t queue_i
>  /* Register vhost driver. dev_name could be different for multiple instance
> support. */
>  int rte_vhost_driver_register(const char *dev_name);
> 
> +/* Unregister vhost driver. This is only meaningful to vhost user. */
> +int rte_vhost_driver_unregister(const char *dev_name);
> +
>  /* Register callbacks. */
>  int rte_vhost_driver_callback_register(struct virtio_net_device_ops const *
> const);
>  /* Start vhost driver session blocking loop. */
> diff --git a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
> b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
> index 6b68abf..1ae7c49 100644
> --- a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
> +++ b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
> @@ -405,6 +405,15 @@ rte_vhost_driver_register(const char *dev_name)
>  }
> 
>  /**
> + * An empty function for unregister
> + */
> +int
> +rte_vhost_driver_unregister(const char *dev_name __rte_unused)
> +{
> +	return 0;
> +}
> +
> +/**
>   * The CUSE session is launched allowing the application to receive open,
>   * release and ioctl calls.
>   */
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c
> b/lib/librte_vhost/vhost_user/vhost-net-user.c
> index 31f1215..87a4711 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.c
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
> @@ -66,6 +66,8 @@ struct connfd_ctx {
>  struct _vhost_server {
>  	struct vhost_server *server[MAX_VHOST_SERVER];
>  	struct fdset fdset;
> +	int vserver_cnt;
> +	pthread_mutex_t server_mutex;
>  };
> 
>  static struct _vhost_server g_vhost_server = {
> @@ -74,10 +76,10 @@ static struct _vhost_server g_vhost_server = {
>  		.fd_mutex = PTHREAD_MUTEX_INITIALIZER,
>  		.num = 0
>  	},
> +	.vserver_cnt = 0,
> +	.server_mutex = PTHREAD_MUTEX_INITIALIZER,
>  };
> 
> -static int vserver_idx;
> -
>  static const char *vhost_message_str[VHOST_USER_MAX] = {
>  	[VHOST_USER_NONE] = "VHOST_USER_NONE",
>  	[VHOST_USER_GET_FEATURES] = "VHOST_USER_GET_FEATURES",
> @@ -427,7 +429,6 @@ vserver_message_handler(int connfd, void *dat, int
> *remove)
>  	}
>  }
> 
> -
>  /**
>   * Creates and initialise the vhost server.
>   */
> @@ -436,34 +437,77 @@ rte_vhost_driver_register(const char *path)
>  {
>  	struct vhost_server *vserver;
> 
> -	if (vserver_idx == 0)
> +	pthread_mutex_lock(&g_vhost_server.server_mutex);
> +	if (ops == NULL)
>  		ops = get_virtio_net_callbacks();
> -	if (vserver_idx == MAX_VHOST_SERVER)
> +
> +	if (g_vhost_server.vserver_cnt == MAX_VHOST_SERVER) {
> +		RTE_LOG(ERR, VHOST_CONFIG,
> +			"error: the number of servers reaches maximum\n");
> +		pthread_mutex_unlock(&g_vhost_server.server_mutex);
>  		return -1;
> +	}
> 
>  	vserver = calloc(sizeof(struct vhost_server), 1);
> -	if (vserver == NULL)
> +	if (vserver == NULL) {
> +		pthread_mutex_unlock(&g_vhost_server.server_mutex);
>  		return -1;
> -
> -	unlink(path);
> +	}
> 
>  	vserver->listenfd = uds_socket(path);
>  	if (vserver->listenfd < 0) {
>  		free(vserver);
> +		pthread_mutex_unlock(&g_vhost_server.server_mutex);
>  		return -1;
>  	}
> -	vserver->path = path;
> +
> +	vserver->path = strdup(path);
> 
>  	fdset_add(&g_vhost_server.fdset, vserver->listenfd,
> -		vserver_new_vq_conn, NULL,
> -		vserver);
> +		vserver_new_vq_conn, NULL, vserver);
> 
> -	g_vhost_server.server[vserver_idx++] = vserver;
> +	g_vhost_server.server[g_vhost_server.vserver_cnt++] = vserver;
> +	pthread_mutex_unlock(&g_vhost_server.server_mutex);
> 
>  	return 0;
>  }
> 
> 
> +/**
> + * Unregister the specified vhost server
> + */
> +int
> +rte_vhost_driver_unregister(const char *path)
> +{
> +	int i;
> +	int count;
> +
> +	pthread_mutex_lock(&g_vhost_server.server_mutex);
> +
> +	for (i = 0; i < g_vhost_server.vserver_cnt; i++) {
> +		if (!strcmp(g_vhost_server.server[i]->path, path)) {
> +			fdset_del(&g_vhost_server.fdset,
> +				g_vhost_server.server[i]->listenfd);
> +
> +			close(g_vhost_server.server[i]->listenfd);
> +			free(g_vhost_server.server[i]->path);
> +			free(g_vhost_server.server[i]);
> +
> +			unlink(path);
> +
> +			count = --g_vhost_server.vserver_cnt;
> +			g_vhost_server.server[i] =
> g_vhost_server.server[count];
> +			g_vhost_server.server[count] = NULL;
> +
> 	pthread_mutex_unlock(&g_vhost_server.server_mutex);
> +
> +			return 0;
> +		}
> +	}
> +	pthread_mutex_unlock(&g_vhost_server.server_mutex);
> +
> +	return -1;
> +}
> +
>  int
>  rte_vhost_driver_session_start(void)
>  {
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h
> b/lib/librte_vhost/vhost_user/vhost-net-user.h
> index 1b6be6c..2e72f3c 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.h
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
> @@ -41,7 +41,7 @@
>  #include "fd_man.h"
> 
>  struct vhost_server {
> -	const char *path; /**< The path the uds is bind to. */
> +	char *path; /**< The path the uds is bind to. */
>  	int listenfd;     /**< The listener sockfd. */
>  };
> 
> --
> 1.8.1.4

Acked-by: Ciara Loftus <ciara.loftus@intel.com>
  
Huawei Xie June 8, 2015, 3:38 p.m. UTC | #2
On 6/5/2015 5:04 PM, Loftus, Ciara wrote:
>
>> -----Original Message-----
>> From: Xie, Huawei
>> Sent: Friday, June 05, 2015 4:26 AM
>> To: dev@dpdk.org
>> Cc: Loftus, Ciara; Xie, Huawei; Sun, Peng A
>> Subject: [PATCH v2] vhost: provide vhost API to unregister vhost unix domain
>> socket
>>
>> rte_vhost_driver_unregister will remove the listenfd from event list, and
>> then close it.
>>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
>> ---
>>  lib/librte_vhost/rte_virtio_net.h            |  3 ++
>>  lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>>  lib/librte_vhost/vhost_user/vhost-net-user.c | 68
>> +++++++++++++++++++++++-----
>>  lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
>>  4 files changed, 69 insertions(+), 13 deletions(-)
>>
>>
> Acked-by: Ciara Loftus <ciara.loftus@intel.com>
>
>
>
Thomas:
Comments to this patch?
This patch will remove the socket file and associated listen fd.
In future, I would also look at whether there is opportunity to attach a
id to each vhost user net interface from QEMU.
Currently DPDK OVS creates a socket file  for each virtio device and use
the file path as the id for the port.

/huawei
  
Thomas F Herbert June 8, 2015, 8:25 p.m. UTC | #3
On 6/8/15 11:38 AM, Xie, Huawei wrote:
> On 6/5/2015 5:04 PM, Loftus, Ciara wrote:
>>
>>> -----Original Message-----
>>> From: Xie, Huawei
>>> Sent: Friday, June 05, 2015 4:26 AM
>>> To: dev@dpdk.org
>>> Cc: Loftus, Ciara; Xie, Huawei; Sun, Peng A
>>> Subject: [PATCH v2] vhost: provide vhost API to unregister vhost unix domain
>>> socket
>>>
>>> rte_vhost_driver_unregister will remove the listenfd from event list, and
>>> then close it.
>>>
>>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>>> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
>>> ---
>>>   lib/librte_vhost/rte_virtio_net.h            |  3 ++
>>>   lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>>>   lib/librte_vhost/vhost_user/vhost-net-user.c | 68
>>> +++++++++++++++++++++++-----
>>>   lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
>>>   4 files changed, 69 insertions(+), 13 deletions(-)
>>>
>>>
>> Acked-by: Ciara Loftus <ciara.loftus@intel.com>
>>
>>
>>
> Thomas:
> Comments to this patch?
After reading the patch, it looks straight forward. I want to compile 
and run OVS/DPDK with vhost-user patch linked with DPDK with this patch 
applied first. I will respond when that is complete.
> This patch will remove the socket file and associated listen fd.
> In future, I would also look at whether there is opportunity to attach a
> id to each vhost user net interface from QEMU.
> Currently DPDK OVS creates a socket file  for each virtio device and use
> the file path as the id for the port.
>
> /huawei
>
>
  
Huawei Xie June 17, 2015, 3:33 a.m. UTC | #4
On 6/5/2015 5:04 PM, Loftus, Ciara wrote:
>
>> -----Original Message-----
>> From: Xie, Huawei
>> Sent: Friday, June 05, 2015 4:26 AM
>> To: dev@dpdk.org
>> Cc: Loftus, Ciara; Xie, Huawei; Sun, Peng A
>> Subject: [PATCH v2] vhost: provide vhost API to unregister vhost unix domain
>> socket
>>
>> rte_vhost_driver_unregister will remove the listenfd from event list, and
>> then close it.
>>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
>> ---
>>  lib/librte_vhost/rte_virtio_net.h            |  3 ++
>>  lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>>  lib/librte_vhost/vhost_user/vhost-net-user.c | 68
>> +++++++++++++++++++++++-----
>>  lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
>>  4 files changed, 69 insertions(+), 13 deletions(-)
>>
[...]
> Acked-by: Ciara Loftus <ciara.loftus@intel.com>
Thomas:
Comments to  this patch?
  
Thomas Monjalon June 17, 2015, 8:59 p.m. UTC | #5
2015-06-05 11:26, Huawei Xie:
> rte_vhost_driver_unregister will remove the listenfd from event list, and then close it.
> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
> ---
>  lib/librte_vhost/rte_virtio_net.h            |  3 ++
>  lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>  lib/librte_vhost/vhost_user/vhost-net-user.c | 68 +++++++++++++++++++++++-----
>  lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-

A function is added to the API without update of the .map file?

Could it be used in a test or an example? It would prevent to break
it or forget to add it in the .map.
  
Huawei Xie June 18, 2015, 1:40 a.m. UTC | #6
On 6/18/2015 5:02 AM, Thomas Monjalon wrote:
> 2015-06-05 11:26, Huawei Xie:
>> rte_vhost_driver_unregister will remove the listenfd from event list, and then close it.
>>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
>> ---
>>  lib/librte_vhost/rte_virtio_net.h            |  3 ++
>>  lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
>>  lib/librte_vhost/vhost_user/vhost-net-user.c | 68 +++++++++++++++++++++++-----
>>  lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
> A function is added to the API without update of the .map file?
Get the comment from Panu. Would submit v3 patch.

>
> Could it be used in a test or an example? It would prevent to break
> it or forget to add it in the .map.
>
Now in vhost example, we only register one socket and never unregister it.
Would evaluate calling this API in vhost example in future.
>
>
  
Huawei Xie June 18, 2015, 5:40 p.m. UTC | #7
vhost user could register multiple unix domain socket server, and use the path to identify the virtio device connecting to it.
rte_vhost_driver_unregister will clean up the unix domain socket for the specified path.

Huawei Xie (2):
  vhost socket cleanup
  update version map file for rte_vhost_driver_unregister API

 lib/librte_vhost/rte_vhost_version.map       |  8 ++++
 lib/librte_vhost/rte_virtio_net.h            |  3 ++
 lib/librte_vhost/vhost_cuse/vhost-net-cdev.c |  9 ++++
 lib/librte_vhost/vhost_user/vhost-net-user.c | 68 +++++++++++++++++++++++-----
 lib/librte_vhost/vhost_user/vhost-net-user.h |  2 +-
 5 files changed, 77 insertions(+), 13 deletions(-)
  

Patch

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 5d38185..5630fbc 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -188,6 +188,9 @@  int rte_vhost_enable_guest_notification(struct virtio_net *dev, uint16_t queue_i
 /* Register vhost driver. dev_name could be different for multiple instance support. */
 int rte_vhost_driver_register(const char *dev_name);
 
+/* Unregister vhost driver. This is only meaningful to vhost user. */
+int rte_vhost_driver_unregister(const char *dev_name);
+
 /* Register callbacks. */
 int rte_vhost_driver_callback_register(struct virtio_net_device_ops const * const);
 /* Start vhost driver session blocking loop. */
diff --git a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
index 6b68abf..1ae7c49 100644
--- a/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
+++ b/lib/librte_vhost/vhost_cuse/vhost-net-cdev.c
@@ -405,6 +405,15 @@  rte_vhost_driver_register(const char *dev_name)
 }
 
 /**
+ * An empty function for unregister
+ */
+int
+rte_vhost_driver_unregister(const char *dev_name __rte_unused)
+{
+	return 0;
+}
+
+/**
  * The CUSE session is launched allowing the application to receive open,
  * release and ioctl calls.
  */
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c
index 31f1215..87a4711 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -66,6 +66,8 @@  struct connfd_ctx {
 struct _vhost_server {
 	struct vhost_server *server[MAX_VHOST_SERVER];
 	struct fdset fdset;
+	int vserver_cnt;
+	pthread_mutex_t server_mutex;
 };
 
 static struct _vhost_server g_vhost_server = {
@@ -74,10 +76,10 @@  static struct _vhost_server g_vhost_server = {
 		.fd_mutex = PTHREAD_MUTEX_INITIALIZER,
 		.num = 0
 	},
+	.vserver_cnt = 0,
+	.server_mutex = PTHREAD_MUTEX_INITIALIZER,
 };
 
-static int vserver_idx;
-
 static const char *vhost_message_str[VHOST_USER_MAX] = {
 	[VHOST_USER_NONE] = "VHOST_USER_NONE",
 	[VHOST_USER_GET_FEATURES] = "VHOST_USER_GET_FEATURES",
@@ -427,7 +429,6 @@  vserver_message_handler(int connfd, void *dat, int *remove)
 	}
 }
 
-
 /**
  * Creates and initialise the vhost server.
  */
@@ -436,34 +437,77 @@  rte_vhost_driver_register(const char *path)
 {
 	struct vhost_server *vserver;
 
-	if (vserver_idx == 0)
+	pthread_mutex_lock(&g_vhost_server.server_mutex);
+	if (ops == NULL)
 		ops = get_virtio_net_callbacks();
-	if (vserver_idx == MAX_VHOST_SERVER)
+
+	if (g_vhost_server.vserver_cnt == MAX_VHOST_SERVER) {
+		RTE_LOG(ERR, VHOST_CONFIG,
+			"error: the number of servers reaches maximum\n");
+		pthread_mutex_unlock(&g_vhost_server.server_mutex);
 		return -1;
+	}
 
 	vserver = calloc(sizeof(struct vhost_server), 1);
-	if (vserver == NULL)
+	if (vserver == NULL) {
+		pthread_mutex_unlock(&g_vhost_server.server_mutex);
 		return -1;
-
-	unlink(path);
+	}
 
 	vserver->listenfd = uds_socket(path);
 	if (vserver->listenfd < 0) {
 		free(vserver);
+		pthread_mutex_unlock(&g_vhost_server.server_mutex);
 		return -1;
 	}
-	vserver->path = path;
+
+	vserver->path = strdup(path);
 
 	fdset_add(&g_vhost_server.fdset, vserver->listenfd,
-		vserver_new_vq_conn, NULL,
-		vserver);
+		vserver_new_vq_conn, NULL, vserver);
 
-	g_vhost_server.server[vserver_idx++] = vserver;
+	g_vhost_server.server[g_vhost_server.vserver_cnt++] = vserver;
+	pthread_mutex_unlock(&g_vhost_server.server_mutex);
 
 	return 0;
 }
 
 
+/**
+ * Unregister the specified vhost server
+ */
+int
+rte_vhost_driver_unregister(const char *path)
+{
+	int i;
+	int count;
+
+	pthread_mutex_lock(&g_vhost_server.server_mutex);
+
+	for (i = 0; i < g_vhost_server.vserver_cnt; i++) {
+		if (!strcmp(g_vhost_server.server[i]->path, path)) {
+			fdset_del(&g_vhost_server.fdset,
+				g_vhost_server.server[i]->listenfd);
+
+			close(g_vhost_server.server[i]->listenfd);
+			free(g_vhost_server.server[i]->path);
+			free(g_vhost_server.server[i]);
+
+			unlink(path);
+
+			count = --g_vhost_server.vserver_cnt;
+			g_vhost_server.server[i] = g_vhost_server.server[count];
+			g_vhost_server.server[count] = NULL;
+			pthread_mutex_unlock(&g_vhost_server.server_mutex);
+
+			return 0;
+		}
+	}
+	pthread_mutex_unlock(&g_vhost_server.server_mutex);
+
+	return -1;
+}
+
 int
 rte_vhost_driver_session_start(void)
 {
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.h b/lib/librte_vhost/vhost_user/vhost-net-user.h
index 1b6be6c..2e72f3c 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.h
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.h
@@ -41,7 +41,7 @@ 
 #include "fd_man.h"
 
 struct vhost_server {
-	const char *path; /**< The path the uds is bind to. */
+	char *path; /**< The path the uds is bind to. */
 	int listenfd;     /**< The listener sockfd. */
 };