From patchwork Mon Jun 29 18:28:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Huawei Xie X-Patchwork-Id: 5951 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 3BB8CC41C; Mon, 29 Jun 2015 20:28:44 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id AAEFBC3FE for ; Mon, 29 Jun 2015 20:28:42 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 29 Jun 2015 11:28:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,371,1432623600"; d="scan'208";a="752684859" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by fmsmga002.fm.intel.com with ESMTP; 29 Jun 2015 11:28:35 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.224.2; Tue, 30 Jun 2015 02:28:33 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.246]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.46]) with mapi id 14.03.0224.002; Tue, 30 Jun 2015 02:28:26 +0800 From: "Xie, Huawei" To: "dev@dpdk.org" Thread-Topic: [PATCH v3 1/2] vhost: vhost unix domain socket cleanup Thread-Index: AdCymWKipkVWETvHS8G1a54FpsHPuw== Date: Mon, 29 Jun 2015 18:28:25 +0000 Message-ID: References: <1433474786-704-1-git-send-email-huawei.xie@intel.com> <1434649260-26317-1-git-send-email-huawei.xie@intel.com> <1434649260-26317-2-git-send-email-huawei.xie@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/2] vhost: vhost unix domain socket cleanup X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "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 Signed-off-by: Peng Sun --- 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. */ };