From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 648D32C5 for ; Fri, 7 Apr 2017 10:15:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1491552929; x=1523088929; h=from:to:cc:subject:date:message-id:in-reply-to: references; bh=QvirorT0XLgbw53CV6ciOyo10cKv32feyH5eDj1yc0w=; b=aK3S8bUnwFHueO1UK0+Bh5mRwkkiPy9CuoqGUoKPRBrEDodq1pvk241T bHpa9Icx8o7fLHOxhSx+Ax3k/QbsCQ==; Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Apr 2017 01:15:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,164,1488873600"; d="scan'208";a="953273619" Received: from yliu-dev.sh.intel.com ([10.239.67.162]) by orsmga003.jf.intel.com with ESMTP; 07 Apr 2017 01:15:28 -0700 From: Yuanhan Liu To: Yuanhan Liu Cc: Ilya Maximets , dpdk stable Date: Fri, 7 Apr 2017 16:11:57 +0800 Message-Id: <1491552724-3034-40-git-send-email-yuanhan.liu@linux.intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1491552724-3034-1-git-send-email-yuanhan.liu@linux.intel.com> References: <1491552724-3034-1-git-send-email-yuanhan.liu@linux.intel.com> Subject: [dpdk-stable] patch 'vhost: fix fd leaks for vhost-user server mode' has been queued to LTS release 16.11.2 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Apr 2017 08:15:29 -0000 Hi, FYI, your patch has been queued to LTS release 16.11.2 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 04/11/17. So please shout if anyone has objections. Thanks. --yliu --- >>From 62c9de08e2969c996ecae47954741004db1de3ae Mon Sep 17 00:00:00 2001 From: Yuanhan Liu Date: Mon, 27 Mar 2017 16:52:15 +0800 Subject: [PATCH] vhost: fix fd leaks for vhost-user server mode [ upstream commit 65388b43f592e16497af48db69ad5063d6824daf ] A vhost-user server socket could have many connections, thus many connfd. However, we currently just use one single int var to store it. Meaning, it will get overwritten every time a new connection is created. While this will not create fatal issue as it sounds (since the correct connfd is closured to the event loop thread by fdset_add), it may cause fd leaks if a user invokes rte_vhost_driver_unregister before shutting down all connections: it just closes the recent connfd. A simple example that should be able to reproduce this leaks issues is, del the ovs vhost-user port while the connected VMs are still alive. (Note that it's suggested to use one socket for one VM, which makes the issue not that fatal as it sounds again). Since we already use a struct "vhost_user_connection" to track all info about one connection, it's obvious that we should put the connfd there. Then we could build a connection list inside the vhost_user_socket struct, to represent all connections belong that socket file. Fixes: 164fd396788d ("vhost: fix unregistering in client mode") Cc: Ilya Maximets Signed-off-by: Yuanhan Liu --- lib/librte_vhost/socket.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 6a30a31..2afde98 100644 --- a/lib/librte_vhost/socket.c +++ b/lib/librte_vhost/socket.c @@ -52,14 +52,18 @@ #include "vhost.h" #include "vhost_user.h" + +TAILQ_HEAD(vhost_user_connection_list, vhost_user_connection); + /* * Every time rte_vhost_driver_register() is invoked, an associated * vhost_user_socket struct will be created. */ struct vhost_user_socket { + struct vhost_user_connection_list conn_list; + pthread_mutex_t conn_mutex; char *path; int listenfd; - int connfd; bool is_server; bool reconnect; bool dequeue_zero_copy; @@ -67,7 +71,10 @@ struct vhost_user_socket { struct vhost_user_connection { struct vhost_user_socket *vsocket; + int connfd; int vid; + + TAILQ_ENTRY(vhost_user_connection) next; }; #define MAX_VHOST_SOCKET 1024 @@ -209,19 +216,23 @@ vhost_user_add_connection(int fd, struct vhost_user_socket *vsocket) RTE_LOG(INFO, VHOST_CONFIG, "new device, handle is %d\n", vid); - vsocket->connfd = fd; + conn->connfd = fd; conn->vsocket = vsocket; conn->vid = vid; ret = fdset_add(&vhost_user.fdset, fd, vhost_user_read_cb, NULL, conn); if (ret < 0) { - vsocket->connfd = -1; + conn->connfd = -1; free(conn); close(fd); RTE_LOG(ERR, VHOST_CONFIG, "failed to add fd %d into vhost server fdset\n", fd); } + + pthread_mutex_lock(&vsocket->conn_mutex); + TAILQ_INSERT_TAIL(&vsocket->conn_list, conn, next); + pthread_mutex_unlock(&vsocket->conn_mutex); } /* call back when there is new vhost-user connection from client */ @@ -247,10 +258,14 @@ vhost_user_read_cb(int connfd, void *dat, int *remove) ret = vhost_user_msg_handler(conn->vid, connfd); if (ret < 0) { - vsocket->connfd = -1; close(connfd); *remove = 1; vhost_destroy_device(conn->vid); + + pthread_mutex_lock(&vsocket->conn_mutex); + TAILQ_REMOVE(&vsocket->conn_list, conn, next); + pthread_mutex_unlock(&vsocket->conn_mutex); + free(conn); if (vsocket->reconnect) @@ -502,7 +517,8 @@ rte_vhost_driver_register(const char *path, uint64_t flags) goto out; memset(vsocket, 0, sizeof(struct vhost_user_socket)); vsocket->path = strdup(path); - vsocket->connfd = -1; + TAILQ_INIT(&vsocket->conn_list); + pthread_mutex_init(&vsocket->conn_mutex, NULL); vsocket->dequeue_zero_copy = flags & RTE_VHOST_USER_DEQUEUE_ZERO_COPY; if ((flags & RTE_VHOST_USER_CLIENT) != 0) { @@ -565,7 +581,7 @@ rte_vhost_driver_unregister(const char *path) { int i; int count; - struct vhost_user_connection *conn; + struct vhost_user_connection *conn, *next; pthread_mutex_lock(&vhost_user.mutex); @@ -581,15 +597,22 @@ rte_vhost_driver_unregister(const char *path) vhost_user_remove_reconnect(vsocket); } - conn = fdset_del(&vhost_user.fdset, vsocket->connfd); - if (conn) { + pthread_mutex_lock(&vsocket->conn_mutex); + for (conn = TAILQ_FIRST(&vsocket->conn_list); + conn != NULL; + conn = next) { + next = TAILQ_NEXT(conn, next); + + fdset_del(&vhost_user.fdset, conn->connfd); RTE_LOG(INFO, VHOST_CONFIG, "free connfd = %d for device '%s'\n", - vsocket->connfd, path); - close(vsocket->connfd); + conn->connfd, path); + close(conn->connfd); vhost_destroy_device(conn->vid); + TAILQ_REMOVE(&vsocket->conn_list, conn, next); free(conn); } + pthread_mutex_unlock(&vsocket->conn_mutex); free(vsocket->path); free(vsocket); -- 1.9.0