* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-02 1:50 [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Huawei Xie
@ 2015-06-03 9:42 ` Loftus, Ciara
2015-06-03 12:55 ` Ouyang, Changchun
` (2 subsequent siblings)
3 siblings, 0 replies; 31+ messages in thread
From: Loftus, Ciara @ 2015-06-03 9:42 UTC (permalink / raw)
To: Xie, Huawei, dev; +Cc: Sun, Peng A
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 02, 2015 2:50 AM
> To: dev@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@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 | 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);
>
> 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>
I have validated this and it works with dpdk vhost-user in OVS.
Thanks,
Ciara
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-02 1:50 [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Huawei Xie
2015-06-03 9:42 ` Loftus, Ciara
@ 2015-06-03 12:55 ` Ouyang, Changchun
2015-06-03 19:03 ` Xie, Huawei
2015-06-05 3:26 ` [dpdk-dev] [PATCH v2] " Huawei Xie
2015-06-17 4:17 ` [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Tetsuya Mukawa
3 siblings, 1 reply; 31+ messages in thread
From: Ouyang, Changchun @ 2015-06-03 12:55 UTC (permalink / raw)
To: Xie, Huawei, dev; +Cc: Sun, Peng A
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 2, 2015 9:50 AM
> To: dev@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@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 | 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?
>
> 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.
> + 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
> + 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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-03 12:55 ` Ouyang, Changchun
@ 2015-06-03 19:03 ` Xie, Huawei
0 siblings, 0 replies; 31+ messages in thread
From: Xie, Huawei @ 2015-06-03 19:03 UTC (permalink / raw)
To: Ouyang, Changchun, dev; +Cc: Sun, Peng A
On 6/3/2015 8:55 PM, Ouyang, Changchun wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
>> Sent: Tuesday, June 2, 2015 9:50 AM
>> To: dev@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@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 | 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
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-02 1:50 [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Huawei Xie
2015-06-03 9:42 ` Loftus, Ciara
2015-06-03 12:55 ` Ouyang, Changchun
@ 2015-06-05 3:26 ` Huawei Xie
2015-06-05 9:04 ` Loftus, Ciara
` (2 more replies)
2015-06-17 4:17 ` [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Tetsuya Mukawa
3 siblings, 3 replies; 31+ messages in thread
From: Huawei Xie @ 2015-06-05 3:26 UTC (permalink / raw)
To: dev
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-05 3:26 ` [dpdk-dev] [PATCH v2] " Huawei Xie
@ 2015-06-05 9:04 ` Loftus, Ciara
2015-06-08 15:38 ` Xie, Huawei
2015-06-17 3:33 ` Xie, Huawei
2015-06-17 20:59 ` Thomas Monjalon
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 0/2] vhost: vhost unix domain socket cleanup Huawei Xie
2 siblings, 2 replies; 31+ messages in thread
From: Loftus, Ciara @ 2015-06-05 9:04 UTC (permalink / raw)
To: Xie, Huawei, dev
> -----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>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-05 9:04 ` Loftus, Ciara
@ 2015-06-08 15:38 ` Xie, Huawei
2015-06-08 20:25 ` Thomas F Herbert
2015-06-17 8:10 ` Panu Matilainen
2015-06-17 3:33 ` Xie, Huawei
1 sibling, 2 replies; 31+ messages in thread
From: Xie, Huawei @ 2015-06-08 15:38 UTC (permalink / raw)
To: Loftus, Ciara, dev
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-08 15:38 ` Xie, Huawei
@ 2015-06-08 20:25 ` Thomas F Herbert
2015-06-17 8:10 ` Panu Matilainen
1 sibling, 0 replies; 31+ messages in thread
From: Thomas F Herbert @ 2015-06-08 20:25 UTC (permalink / raw)
To: dev
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
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-08 15:38 ` Xie, Huawei
2015-06-08 20:25 ` Thomas F Herbert
@ 2015-06-17 8:10 ` Panu Matilainen
1 sibling, 0 replies; 31+ messages in thread
From: Panu Matilainen @ 2015-06-17 8:10 UTC (permalink / raw)
To: Xie, Huawei, Loftus, Ciara, dev
On 06/08/2015 06:38 PM, 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(-)
>>>
You need to update the symbol version map when adding new public APIs.
Something like:
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -13,3 +13,11 @@ DPDK_2.0 {
local: *;
};
+
+DPDK_2.1 {
+ global:
+
+ rte_vhost_driver_unregister;
+
+ local: *;
+};
- Panu -
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-05 9:04 ` Loftus, Ciara
2015-06-08 15:38 ` Xie, Huawei
@ 2015-06-17 3:33 ` Xie, Huawei
1 sibling, 0 replies; 31+ messages in thread
From: Xie, Huawei @ 2015-06-17 3:33 UTC (permalink / raw)
To: Loftus, Ciara, dev, Thomas Monjalon
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?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-05 3:26 ` [dpdk-dev] [PATCH v2] " Huawei Xie
2015-06-05 9:04 ` Loftus, Ciara
@ 2015-06-17 20:59 ` Thomas Monjalon
2015-06-18 1:40 ` Xie, Huawei
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 0/2] vhost: vhost unix domain socket cleanup Huawei Xie
2 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2015-06-17 20:59 UTC (permalink / raw)
To: Huawei Xie; +Cc: dev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v2] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-17 20:59 ` Thomas Monjalon
@ 2015-06-18 1:40 ` Xie, Huawei
0 siblings, 0 replies; 31+ messages in thread
From: Xie, Huawei @ 2015-06-18 1:40 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
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.
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v3 0/2] vhost: vhost unix domain socket cleanup
2015-06-05 3:26 ` [dpdk-dev] [PATCH v2] " Huawei Xie
2015-06-05 9:04 ` Loftus, Ciara
2015-06-17 20:59 ` Thomas Monjalon
@ 2015-06-18 17:40 ` Huawei Xie
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 1/2] " Huawei Xie
2015-06-18 17:41 ` [dpdk-dev] [PATCH v3 2/2] vhost: version map file update Huawei Xie
2 siblings, 2 replies; 31+ messages in thread
From: Huawei Xie @ 2015-06-18 17:40 UTC (permalink / raw)
To: dev
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(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v3 1/2] vhost: vhost unix domain socket cleanup
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 0/2] vhost: vhost unix domain socket cleanup Huawei Xie
@ 2015-06-18 17:40 ` Huawei Xie
2015-06-29 18:28 ` Xie, Huawei
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
2015-06-18 17:41 ` [dpdk-dev] [PATCH v3 2/2] vhost: version map file update Huawei Xie
1 sibling, 2 replies; 31+ messages in thread
From: Huawei Xie @ 2015-06-18 17:40 UTC (permalink / raw)
To: dev
rte_vhost_driver_unregister API will remove the listenfd for the specified path from event processing list, and then close it.
v2 changes:
-minor code style fix: remove unnecessary new line
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/2] vhost: vhost unix domain socket cleanup
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 1/2] " Huawei Xie
@ 2015-06-29 18:28 ` Xie, Huawei
2015-06-29 21:02 ` Thomas Monjalon
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
1 sibling, 1 reply; 31+ messages in thread
From: Xie, Huawei @ 2015-06-29 18:28 UTC (permalink / raw)
To: dev
On 6/19/2015 1:40 AM, Huawei Xie wrote:
rte_vhost_driver_unregister API will remove the listenfd for the specified path from event processing list, and then close it.
v2 changes:
-minor code style fix: remove unnecessary new line
Signed-off-by: Huawei Xie <huawei.xie@intel.com><mailto:huawei.xie@intel.com>
Signed-off-by: Peng Sun <peng.a.sun@intel.com><mailto: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);
In fd_man.c, in the event handler for connection fd, the fd could be closed when receives no data.
Before the following code snippet, as it isn't protected, there is chance we register the listenfd with the value of the just closed fd.
so the following fdset_del could wrongly remove the new listenfd.
would use fdset_del_slot to delete entry at fixed slot.
if (remove1 || remove2)
fdset_del(pfdset, fd);
another thing is when select is blocked, rte_vhost_driver_unregister/register could remove/refill entries of some listenfd(s!!!).
There is potential unwanted call on new listenfds, it is not a issue. would add comment to emphasize that.
- 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. */
};
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/2] vhost: vhost unix domain socket cleanup
2015-06-29 18:28 ` Xie, Huawei
@ 2015-06-29 21:02 ` Thomas Monjalon
2015-06-30 6:19 ` Xie, Huawei
0 siblings, 1 reply; 31+ messages in thread
From: Thomas Monjalon @ 2015-06-29 21:02 UTC (permalink / raw)
To: Xie, Huawei; +Cc: dev
Huawei,
I don't understand this reply. You forgot quoting, you didn't remove useless lines,
and you seem to reply to yourself.
Should this patch be applied?
2015-06-29 18:28, Xie, Huawei:
> On 6/19/2015 1:40 AM, Huawei Xie wrote:
>
> rte_vhost_driver_unregister API will remove the listenfd for the specified path from event processing list, and then close it.
>
> v2 changes:
> -minor code style fix: remove unnecessary new line
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com><mailto:huawei.xie@intel.com>
> Signed-off-by: Peng Sun <peng.a.sun@intel.com><mailto: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);
>
>
> In fd_man.c, in the event handler for connection fd, the fd could be closed when receives no data.
> Before the following code snippet, as it isn't protected, there is chance we register the listenfd with the value of the just closed fd.
> so the following fdset_del could wrongly remove the new listenfd.
> would use fdset_del_slot to delete entry at fixed slot.
>
> if (remove1 || remove2)
> fdset_del(pfdset, fd);
>
>
> another thing is when select is blocked, rte_vhost_driver_unregister/register could remove/refill entries of some listenfd(s!!!).
> There is potential unwanted call on new listenfds, it is not a issue. would add comment to emphasize that.
>
>
>
>
>
>
> - 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. */
> };
>
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v3 1/2] vhost: vhost unix domain socket cleanup
2015-06-29 21:02 ` Thomas Monjalon
@ 2015-06-30 6:19 ` Xie, Huawei
0 siblings, 0 replies; 31+ messages in thread
From: Xie, Huawei @ 2015-06-30 6:19 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On 6/30/2015 5:04 AM, Thomas Monjalon wrote:
> Huawei,
> I don't understand this reply. You forgot quoting, you didn't remove useless lines,
> and you seem to reply to yourself.
> Should this patch be applied?
>
Thomas:
Oh, here i remove useless lines. I am sending a new patch to fix a
potential issue.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v4 0/4] vhost: vhost unix domain socket cleanup
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 1/2] " Huawei Xie
2015-06-29 18:28 ` Xie, Huawei
@ 2015-06-30 9:20 ` Huawei Xie
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd Huawei Xie
` (4 more replies)
1 sibling, 5 replies; 31+ messages in thread
From: Huawei Xie @ 2015-06-30 9:20 UTC (permalink / raw)
To: dev
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.
v2 changes:
-minor code style fix, remove unnecessary new line
v3 changes:
update version map file
v4 changes:
-add comment for potential unwanted callback on listenfds
-call fdset_del_slot to remove connection fd
Huawei Xie (4):
fdset_del_slot
vhost socket cleanup
version map file update
add comment for potential unwanted call on listenfds
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/fd_man.c | 34 +++++++++++++-
lib/librte_vhost/vhost_user/vhost-net-user.c | 68 +++++++++++++++++++++++-----
lib/librte_vhost/vhost_user/vhost-net-user.h | 2 +-
6 files changed, 110 insertions(+), 14 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread* [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
@ 2015-06-30 9:20 ` Huawei Xie
2015-07-01 2:14 ` Ouyang, Changchun
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup Huawei Xie
` (3 subsequent siblings)
4 siblings, 1 reply; 31+ messages in thread
From: Huawei Xie @ 2015-06-30 9:20 UTC (permalink / raw)
To: dev
In the event handler of connection fd, the connection fd could be possibly
closed. The event dispatch loop would then try to remove the fd from fdset.
Between these two actions, another thread might register a new listenfd
reusing the val of just closed fd, so we couldn't call fdset_del which would
wrongly clean up the new listenfd. A new function fdset_del_slot is provided
to cleanup the fd at the specified location.
v4 changes:
- call fdset_del_slot to remove connection fd
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
lib/librte_vhost/vhost_user/fd_man.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c
index 831c9c1..bd30f8d 100644
--- a/lib/librte_vhost/vhost_user/fd_man.c
+++ b/lib/librte_vhost/vhost_user/fd_man.c
@@ -188,6 +188,24 @@ fdset_del(struct fdset *pfdset, int fd)
}
/**
+ * Unregister the fd at the specified slot from the fdset.
+ */
+static void
+fdset_del_slot(struct fdset *pfdset, int index)
+{
+ if (pfdset == NULL || index < 0 || index >= MAX_FDS)
+ return;
+
+ pthread_mutex_lock(&pfdset->fd_mutex);
+
+ pfdset->fd[index].fd = -1;
+ pfdset->fd[index].rcb = pfdset->fd[index].wcb = NULL;
+ pfdset->num--;
+
+ pthread_mutex_unlock(&pfdset->fd_mutex);
+}
+
+/**
* This functions runs in infinite blocking loop until there is no fd in
* pfdset. It calls corresponding r/w handler if there is event on the fd.
*
@@ -248,8 +266,15 @@ fdset_event_dispatch(struct fdset *pfdset)
* We don't allow fdset_del to be called in callback
* directly.
*/
+ /*
+ * When we are to clean up the fd from fdset,
+ * because the fd is closed in the cb,
+ * the old fd val could be reused by when creates new
+ * listen fd in another thread, we couldn't call
+ * fd_set_del.
+ */
if (remove1 || remove2)
- fdset_del(pfdset, fd);
+ fdset_del_slot(pfdset, i);
}
}
}
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd Huawei Xie
@ 2015-07-01 2:14 ` Ouyang, Changchun
0 siblings, 0 replies; 31+ messages in thread
From: Ouyang, Changchun @ 2015-07-01 2:14 UTC (permalink / raw)
To: Xie, Huawei, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 30, 2015 5:21 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove
> connection fd
>
> In the event handler of connection fd, the connection fd could be possibly
> closed. The event dispatch loop would then try to remove the fd from fdset.
> Between these two actions, another thread might register a new listenfd
> reusing the val of just closed fd, so we couldn't call fdset_del which would
> wrongly clean up the new listenfd. A new function fdset_del_slot is provided
> to cleanup the fd at the specified location.
>
> v4 changes:
> - call fdset_del_slot to remove connection fd
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
> lib/librte_vhost/vhost_user/fd_man.c | 27
> ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost_user/fd_man.c
> b/lib/librte_vhost/vhost_user/fd_man.c
> index 831c9c1..bd30f8d 100644
> --- a/lib/librte_vhost/vhost_user/fd_man.c
> +++ b/lib/librte_vhost/vhost_user/fd_man.c
> @@ -188,6 +188,24 @@ fdset_del(struct fdset *pfdset, int fd) }
>
> /**
> + * Unregister the fd at the specified slot from the fdset.
> + */
> +static void
> +fdset_del_slot(struct fdset *pfdset, int index) {
> + if (pfdset == NULL || index < 0 || index >= MAX_FDS)
> + return;
> +
> + pthread_mutex_lock(&pfdset->fd_mutex);
> +
> + pfdset->fd[index].fd = -1;
> + pfdset->fd[index].rcb = pfdset->fd[index].wcb = NULL;
> + pfdset->num--;
> +
> + pthread_mutex_unlock(&pfdset->fd_mutex);
> +}
> +
> +/**
> * This functions runs in infinite blocking loop until there is no fd in
> * pfdset. It calls corresponding r/w handler if there is event on the fd.
> *
> @@ -248,8 +266,15 @@ fdset_event_dispatch(struct fdset *pfdset)
> * We don't allow fdset_del to be called in callback
> * directly.
> */
> + /*
> + * When we are to clean up the fd from fdset,
> + * because the fd is closed in the cb,
> + * the old fd val could be reused by when creates
> new
> + * listen fd in another thread, we couldn't call
> + * fd_set_del.
> + */
> if (remove1 || remove2)
> - fdset_del(pfdset, fd);
> + fdset_del_slot(pfdset, i);
> }
> }
> }
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd Huawei Xie
@ 2015-06-30 9:20 ` Huawei Xie
2015-07-01 2:14 ` Ouyang, Changchun
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 3/4] vhost: version map file update Huawei Xie
` (2 subsequent siblings)
4 siblings, 1 reply; 31+ messages in thread
From: Huawei Xie @ 2015-06-30 9:20 UTC (permalink / raw)
To: dev
rte_vhost_driver_unregister API will remove the listenfd from event list,
and then close it.
v2 changes:
-minor code style fix, remove unnecessary new line
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup Huawei Xie
@ 2015-07-01 2:14 ` Ouyang, Changchun
0 siblings, 0 replies; 31+ messages in thread
From: Ouyang, Changchun @ 2015-07-01 2:14 UTC (permalink / raw)
To: Xie, Huawei, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 30, 2015 5:21 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup
>
> rte_vhost_driver_unregister API will remove the listenfd from event list, and
> then close it.
>
> v2 changes:
> -minor code style fix, remove unnecessary new line
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> Signed-off-by: Peng Sun <peng.a.sun@intel.com>
Acked-by: Changchun Ouyang <changchun.ouyang@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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v4 3/4] vhost: version map file update
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 1/4] vhost: call fdset_del_slot to remove connection fd Huawei Xie
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 2/4] vhost: vhost unix domain socket cleanup Huawei Xie
@ 2015-06-30 9:20 ` Huawei Xie
2015-07-01 2:15 ` Ouyang, Changchun
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential unwanted callback on listenfds Huawei Xie
2015-06-30 15:55 ` [dpdk-dev] [PATCH v4 0/4] vhost: vhost unix domain socket cleanup Thomas Monjalon
4 siblings, 1 reply; 31+ messages in thread
From: Huawei Xie @ 2015-06-30 9:20 UTC (permalink / raw)
To: dev
update version map file for rte_vhost_driver_unregister API
v3 changes:
update version map file
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
lib/librte_vhost/rte_vhost_version.map | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 163dde0..fb6bb9e 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -13,3 +13,11 @@ DPDK_2.0 {
local: *;
};
+
+DPDK_2.1 {
+ global:
+
+ rte_vhost_driver_unregister;
+
+ local: *;
+} DPDK_2.0;
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v4 3/4] vhost: version map file update
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 3/4] vhost: version map file update Huawei Xie
@ 2015-07-01 2:15 ` Ouyang, Changchun
0 siblings, 0 replies; 31+ messages in thread
From: Ouyang, Changchun @ 2015-07-01 2:15 UTC (permalink / raw)
To: Xie, Huawei, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 30, 2015 5:21 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 3/4] vhost: version map file update
>
> update version map file for rte_vhost_driver_unregister API
>
> v3 changes:
> update version map file
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
> lib/librte_vhost/rte_vhost_version.map | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/lib/librte_vhost/rte_vhost_version.map
> b/lib/librte_vhost/rte_vhost_version.map
> index 163dde0..fb6bb9e 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -13,3 +13,11 @@ DPDK_2.0 {
>
> local: *;
> };
> +
> +DPDK_2.1 {
> + global:
> +
> + rte_vhost_driver_unregister;
> +
> + local: *;
> +} DPDK_2.0;
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential unwanted callback on listenfds
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
` (2 preceding siblings ...)
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 3/4] vhost: version map file update Huawei Xie
@ 2015-06-30 9:20 ` Huawei Xie
2015-07-01 2:15 ` Ouyang, Changchun
2015-06-30 15:55 ` [dpdk-dev] [PATCH v4 0/4] vhost: vhost unix domain socket cleanup Thomas Monjalon
4 siblings, 1 reply; 31+ messages in thread
From: Huawei Xie @ 2015-06-30 9:20 UTC (permalink / raw)
To: dev
add comment for potential unwanted callback on listenfds
v4 changes:
add comment for potential unwanted callback on listenfds
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
lib/librte_vhost/vhost_user/fd_man.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c
index bd30f8d..d68b270 100644
--- a/lib/librte_vhost/vhost_user/fd_man.c
+++ b/lib/librte_vhost/vhost_user/fd_man.c
@@ -242,6 +242,13 @@ fdset_event_dispatch(struct fdset *pfdset)
pthread_mutex_unlock(&pfdset->fd_mutex);
+ /*
+ * When select is blocked, other threads might unregister
+ * listenfds from and register new listenfds into fdset.
+ * When select returns, the entries for listenfds in the fdset
+ * might have been updated. It is ok if there is unwanted call
+ * for new listenfds.
+ */
ret = select(maxfds + 1, &rfds, &wfds, NULL, &tv);
if (ret <= 0)
continue;
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential unwanted callback on listenfds
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential unwanted callback on listenfds Huawei Xie
@ 2015-07-01 2:15 ` Ouyang, Changchun
0 siblings, 0 replies; 31+ messages in thread
From: Ouyang, Changchun @ 2015-07-01 2:15 UTC (permalink / raw)
To: Xie, Huawei, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Huawei Xie
> Sent: Tuesday, June 30, 2015 5:21 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential
> unwanted callback on listenfds
>
> add comment for potential unwanted callback on listenfds
>
> v4 changes:
> add comment for potential unwanted callback on listenfds
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
> lib/librte_vhost/vhost_user/fd_man.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lib/librte_vhost/vhost_user/fd_man.c
> b/lib/librte_vhost/vhost_user/fd_man.c
> index bd30f8d..d68b270 100644
> --- a/lib/librte_vhost/vhost_user/fd_man.c
> +++ b/lib/librte_vhost/vhost_user/fd_man.c
> @@ -242,6 +242,13 @@ fdset_event_dispatch(struct fdset *pfdset)
>
> pthread_mutex_unlock(&pfdset->fd_mutex);
>
> + /*
> + * When select is blocked, other threads might unregister
> + * listenfds from and register new listenfds into fdset.
> + * When select returns, the entries for listenfds in the fdset
> + * might have been updated. It is ok if there is unwanted call
> + * for new listenfds.
> + */
> ret = select(maxfds + 1, &rfds, &wfds, NULL, &tv);
> if (ret <= 0)
> continue;
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH v4 0/4] vhost: vhost unix domain socket cleanup
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 0/4] " Huawei Xie
` (3 preceding siblings ...)
2015-06-30 9:20 ` [dpdk-dev] [PATCH v4 4/4] vhost: add comment for potential unwanted callback on listenfds Huawei Xie
@ 2015-06-30 15:55 ` Thomas Monjalon
4 siblings, 0 replies; 31+ messages in thread
From: Thomas Monjalon @ 2015-06-30 15:55 UTC (permalink / raw)
To: Huawei Xie; +Cc: dev
2015-06-30 17:20, Huawei Xie:
> 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.
>
> v2 changes:
> -minor code style fix, remove unnecessary new line
>
> v3 changes:
> update version map file
>
> v4 changes:
> -add comment for potential unwanted callback on listenfds
> -call fdset_del_slot to remove connection fd
>
> Huawei Xie (4):
> fdset_del_slot
> vhost socket cleanup
> version map file update
> add comment for potential unwanted call on listenfds
Applied, thanks
^ permalink raw reply [flat|nested] 31+ messages in thread
* [dpdk-dev] [PATCH v3 2/2] vhost: version map file update
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 0/2] vhost: vhost unix domain socket cleanup Huawei Xie
2015-06-18 17:40 ` [dpdk-dev] [PATCH v3 1/2] " Huawei Xie
@ 2015-06-18 17:41 ` Huawei Xie
1 sibling, 0 replies; 31+ messages in thread
From: Huawei Xie @ 2015-06-18 17:41 UTC (permalink / raw)
To: dev
update version map file for rte_vhost_driver_unregister API
v3 changes:
update version map file
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
lib/librte_vhost/rte_vhost_version.map | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/rte_vhost_version.map
index 163dde0..fb6bb9e 100644
--- a/lib/librte_vhost/rte_vhost_version.map
+++ b/lib/librte_vhost/rte_vhost_version.map
@@ -13,3 +13,11 @@ DPDK_2.0 {
local: *;
};
+
+DPDK_2.1 {
+ global:
+
+ rte_vhost_driver_unregister;
+
+ local: *;
+} DPDK_2.0;
--
1.8.1.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-02 1:50 [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Huawei Xie
` (2 preceding siblings ...)
2015-06-05 3:26 ` [dpdk-dev] [PATCH v2] " Huawei Xie
@ 2015-06-17 4:17 ` Tetsuya Mukawa
2015-06-17 11:05 ` Xie, Huawei
3 siblings, 1 reply; 31+ messages in thread
From: Tetsuya Mukawa @ 2015-06-17 4:17 UTC (permalink / raw)
To: Huawei Xie, dev
On 2015/06/02 10:50, Huawei Xie wrote:
> 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>
> ---
>
> +/**
> + * 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;
> +}
> +
>
Hi Xie,
It seems vserver_cnt is incremented when socket is registered, and
decremented when unregistered.
And this value is used for index value of g_vhost_server.server[ ], when
a new socket is registered.
So I have a question about below case.
Step1. socket0 is registered.
Step2: scoekt1 is registered.
Step3. socket0 is unregistered.
Step4. socket2 is registered.
After above steps, are socket1 and socket2 still registered?
Thanks,
Tetsuya
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-17 4:17 ` [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket Tetsuya Mukawa
@ 2015-06-17 11:05 ` Xie, Huawei
2015-06-18 1:00 ` Tetsuya Mukawa
0 siblings, 1 reply; 31+ messages in thread
From: Xie, Huawei @ 2015-06-17 11:05 UTC (permalink / raw)
To: Tetsuya Mukawa; +Cc: dev
On 6/17/2015 12:17 PM, Tetsuya Mukawa wrote:
> On 2015/06/02 10:50, Huawei Xie wrote:
>> 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>
>> ---
>>
>> +/**
>> + * 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;
>> +}
>> +
>>
> Hi Xie,
>
> It seems vserver_cnt is incremented when socket is registered, and
> decremented when unregistered.
> And this value is used for index value of g_vhost_server.server[ ], when
> a new socket is registered.
When we unregister a server at index x, we will move the server at the
tail of the array to the location x.
> So I have a question about below case.
>
> Step1. socket0 is registered.
> Step2: scoekt1 is registered.
> Step3. socket0 is unregistered.
When socket0 is unregistered, socket1 will be moved to location at index 0.
> Step4. socket2 is registered.
socket2 is registered at index 1.
>
> After above steps, are socket1 and socket2 still registered?
>
> Thanks,
> Tetsuya
>
>
What is your concern here?
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [dpdk-dev] [PATCH] vhost: provide vhost API to unregister vhost unix domain socket
2015-06-17 11:05 ` Xie, Huawei
@ 2015-06-18 1:00 ` Tetsuya Mukawa
0 siblings, 0 replies; 31+ messages in thread
From: Tetsuya Mukawa @ 2015-06-18 1:00 UTC (permalink / raw)
To: Xie, Huawei; +Cc: dev
On 2015/06/17 20:05, Xie, Huawei wrote:
> On 6/17/2015 12:17 PM, Tetsuya Mukawa wrote:
>> On 2015/06/02 10:50, Huawei Xie wrote:
>>> 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>
>>> ---
>>>
>>> +/**
>>> + * 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;
>>> +}
>>> +
>>>
>> Hi Xie,
>>
>> It seems vserver_cnt is incremented when socket is registered, and
>> decremented when unregistered.
>> And this value is used for index value of g_vhost_server.server[ ], when
>> a new socket is registered.
> When we unregister a server at index x, we will move the server at the
> tail of the array to the location x.
>> So I have a question about below case.
>>
>> Step1. socket0 is registered.
>> Step2: scoekt1 is registered.
>> Step3. socket0 is unregistered.
> When socket0 is unregistered, socket1 will be moved to location at index 0.
Thanks for explanation, I overlooked this behavior.
Now I don't have any concerns.
Thanks,
Tetsuya
^ permalink raw reply [flat|nested] 31+ messages in thread