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
>
>
