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

Xie, Huawei huawei.xie at intel.com
Wed Jun 3 21:03:49 CEST 2015


On 6/3/2015 8:55 PM, Ouyang, Changchun wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie
>> Sent: Tuesday, June 2, 2015 9:50 AM
>> To: dev at dpdk.org
>> Cc: Sun, Peng A
>> Subject: [dpdk-dev] [PATCH] 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 at intel.com>
>> Signed-off-by: Peng Sun <peng.a.sun at 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 | 70
>> +++++++++++++++++++++++-----  lib/librte_vhost/vhost_user/vhost-net-
>> user.h |  2 +-
>>  4 files changed, 71 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..dff46ee 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,79 @@ 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);
> Do we need check if there is existing server which has same path  with the new one?

This is considered. If there is existing server, binding will fail.
>
>>  	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);
> Besides listenfd, do we need check other fd's status, all fds should be closed before driver unregistered. 
No, we don't need. Why do we check other fds? Here we are to unregister 
the 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];
> it is better to remove the unnecessary new line for easy to read, so does the next 2 lines
Thanks.
>> +			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;
>> +}
> Do we have test case cover this new function?
>
>> +
>>  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
>
>



More information about the dev mailing list