From patchwork Thu Jul 21 12:55:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 14959 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 060935424; Thu, 21 Jul 2016 14:55:48 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id CEB042B9D for ; Thu, 21 Jul 2016 14:55:46 +0200 (CEST) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout2.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OAO00HJX18XG3A0@mailout2.w1.samsung.com> for dev@dpdk.org; Thu, 21 Jul 2016 13:55:45 +0100 (BST) X-AuditID: cbfec7f4-f796c6d000001486-17-5790c65040c3 Received: from eusync1.samsung.com ( [203.254.199.211]) by eucpsbgm1.samsung.com (EUCPMTA) with SMTP id D0.D2.05254.056C0975; Thu, 21 Jul 2016 13:55:44 +0100 (BST) Received: from imaximets.rnd.samsung.ru ([106.109.129.180]) by eusync1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OAO008KR18PFQ70@eusync1.samsung.com>; Thu, 21 Jul 2016 13:55:44 +0100 (BST) From: Ilya Maximets To: dev@dpdk.org, Huawei Xie , Yuanhan Liu Cc: Dyasly Sergey , Heetae Ahn , Thomas Monjalon , Ilya Maximets Date: Thu, 21 Jul 2016 15:55:36 +0300 Message-id: <1469105736-413-1-git-send-email-i.maximets@samsung.com> X-Mailer: git-send-email 2.7.4 In-reply-to: <1469003563-27340-1-git-send-email-i.maximets@samsung.com> References: <1469003563-27340-1-git-send-email-i.maximets@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprMLMWRmVeSWpSXmKPExsVy+t/xy7oBxyaEG3y+YGPx7tN2Jotpn2+z W7TPPMtkcaX9J7vF5NlSFl82TWezuD7hAqsDu8fF/juMHr8WLGX1WLznJZPHvJOBHn1bVjEG sEZx2aSk5mSWpRbp2yVwZbQvPM5e8Ni44ua6kywNjDc0uxg5OSQETCQuXv7GBGGLSVy4t56t i5GLQ0hgKaPE0R07mSCcViaJGb272EGq2AR0JE6tPsIIYosIJEgc2f+bFaSIWWAVo8T1x5PA ioQFHCQuPVsCZrMIqEo0rrwEVMTBwSvgIrFkjR/ENjmJm+c6mUFsTgF3ie/3j7GB2EICbhKb jjQyTWDkXcDIsIpRNLU0uaA4KT3XUK84Mbe4NC9dLzk/dxMjJKi+7GBcfMzqEKMAB6MSD2/C yv5wIdbEsuLK3EOMEhzMSiK84ocmhAvxpiRWVqUW5ccXleakFh9ilOZgURLnnbvrfYiQQHpi SWp2ampBahFMlomDU6qBUeC3JMdS3ynOQt6PPuoY7n5a/+avBk+zy5q/+aVqqo07ehaKvHio dvfbZiUt1Y41wcun9q4vdCtsuPqlUG3i1cIZZv19fC+Olsyy/7hyxY5A94dZXuKSRbtEmcW2 mv949e/2inffgg7L8v6K3O52sqN8+8O8DeYqUzOe6TPqO4RwH/zi0uCpqcRSnJFoqMVcVJwI AIdHM/MmAgAA Subject: [dpdk-dev] [PATCH v3] vhost: fix driver unregister for client mode 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" Currently while calling of 'rte_vhost_driver_unregister()' connection to QEMU will not be closed. This leads to inability to register driver again and reconnect to same virtual machine. This scenario is reproducible with OVS. While executing of the following command vhost port will be re-created (will be executed 'rte_vhost_driver_register()' followed by 'rte_vhost_driver_unregister()') network will be broken and QEMU possibly will crash: ovs-vsctl set Interface vhost1 ofport_request=15 Fix this by closing all established connections on driver unregister and removing of pending connections from reconnection list. Fixes: 64ab701c3d1e ("vhost: add vhost-user client mode") Signed-off-by: Ilya Maximets Acked-by: Yuanhan Liu --- Version 3: * fixed leak of file descriptors by adding of 'close(reconn->fd)' to 'vhost_user_remove_reconnect()' Version 2: * style fixes. lib/librte_vhost/vhost_user/fd_man.c | 15 ++++++-- lib/librte_vhost/vhost_user/fd_man.h | 2 +- lib/librte_vhost/vhost_user/vhost-net-user.c | 57 ++++++++++++++++++++++++---- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c index c691339..2d3eeb7 100644 --- a/lib/librte_vhost/vhost_user/fd_man.c +++ b/lib/librte_vhost/vhost_user/fd_man.c @@ -132,8 +132,10 @@ fdset_init(struct fdset *pfdset) if (pfdset == NULL) return; - for (i = 0; i < MAX_FDS; i++) + for (i = 0; i < MAX_FDS; i++) { pfdset->fd[i].fd = -1; + pfdset->fd[i].dat = NULL; + } pfdset->num = 0; } @@ -166,14 +168,16 @@ fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat) /** * Unregister the fd from the fdset. + * Returns context of a given fd or NULL. */ -void +void * fdset_del(struct fdset *pfdset, int fd) { int i; + void *dat = NULL; if (pfdset == NULL || fd == -1) - return; + return NULL; do { pthread_mutex_lock(&pfdset->fd_mutex); @@ -181,13 +185,17 @@ fdset_del(struct fdset *pfdset, int fd) i = fdset_find_fd(pfdset, fd); if (i != -1 && pfdset->fd[i].busy == 0) { /* busy indicates r/wcb is executing! */ + dat = pfdset->fd[i].dat; pfdset->fd[i].fd = -1; pfdset->fd[i].rcb = pfdset->fd[i].wcb = NULL; + pfdset->fd[i].dat = NULL; pfdset->num--; i = -1; } pthread_mutex_unlock(&pfdset->fd_mutex); } while (i != -1); + + return dat; } /** @@ -203,6 +211,7 @@ fdset_del_slot(struct fdset *pfdset, int index) pfdset->fd[index].fd = -1; pfdset->fd[index].rcb = pfdset->fd[index].wcb = NULL; + pfdset->fd[index].dat = NULL; pfdset->num--; pthread_mutex_unlock(&pfdset->fd_mutex); diff --git a/lib/librte_vhost/vhost_user/fd_man.h b/lib/librte_vhost/vhost_user/fd_man.h index 74ecde2..bd66ed1 100644 --- a/lib/librte_vhost/vhost_user/fd_man.h +++ b/lib/librte_vhost/vhost_user/fd_man.h @@ -60,7 +60,7 @@ void fdset_init(struct fdset *pfdset); int fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat); -void fdset_del(struct fdset *pfdset, int fd); +void *fdset_del(struct fdset *pfdset, int fd); void fdset_event_dispatch(struct fdset *pfdset); diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c b/lib/librte_vhost/vhost_user/vhost-net-user.c index f0ea3a2..62c5ec3 100644 --- a/lib/librte_vhost/vhost_user/vhost-net-user.c +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c @@ -60,6 +60,7 @@ struct vhost_user_socket { char *path; int listenfd; + int connfd; bool is_server; bool reconnect; }; @@ -277,11 +278,13 @@ 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->vsocket = vsocket; conn->vid = vid; ret = fdset_add(&vhost_user.fdset, fd, vhost_user_msg_handler, NULL, conn); if (ret < 0) { + vsocket->connfd = -1; free(conn); close(fd); RTE_LOG(ERR, VHOST_CONFIG, @@ -329,6 +332,7 @@ vhost_user_msg_handler(int connfd, void *dat, int *remove) RTE_LOG(ERR, VHOST_CONFIG, "vhost read incorrect message\n"); + vsocket->connfd = -1; close(connfd); *remove = 1; free(conn); @@ -635,6 +639,7 @@ 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; if ((flags & RTE_VHOST_USER_CLIENT) != 0) { vsocket->reconnect = !(flags & RTE_VHOST_USER_NO_RECONNECT); @@ -664,6 +669,30 @@ out: return ret; } +static bool +vhost_user_remove_reconnect(struct vhost_user_socket *vsocket) +{ + int found = false; + struct vhost_user_reconnect *reconn, *next; + + pthread_mutex_lock(&reconn_list.mutex); + + for (reconn = TAILQ_FIRST(&reconn_list.head); + reconn != NULL; reconn = next) { + next = TAILQ_NEXT(reconn, next); + + if (reconn->vsocket == vsocket) { + TAILQ_REMOVE(&reconn_list.head, reconn, next); + close(reconn->fd); + free(reconn); + found = true; + break; + } + } + pthread_mutex_unlock(&reconn_list.mutex); + return found; +} + /** * Unregister the specified vhost socket */ @@ -672,20 +701,34 @@ rte_vhost_driver_unregister(const char *path) { int i; int count; + struct vhost_user_connection *conn; pthread_mutex_lock(&vhost_user.mutex); for (i = 0; i < vhost_user.vsocket_cnt; i++) { - if (!strcmp(vhost_user.vsockets[i]->path, path)) { - if (vhost_user.vsockets[i]->is_server) { - fdset_del(&vhost_user.fdset, - vhost_user.vsockets[i]->listenfd); - close(vhost_user.vsockets[i]->listenfd); + struct vhost_user_socket *vsocket = vhost_user.vsockets[i]; + + if (!strcmp(vsocket->path, path)) { + if (vsocket->is_server) { + fdset_del(&vhost_user.fdset, vsocket->listenfd); + close(vsocket->listenfd); unlink(path); + } else if (vsocket->reconnect) { + vhost_user_remove_reconnect(vsocket); + } + + conn = fdset_del(&vhost_user.fdset, vsocket->connfd); + if (conn) { + RTE_LOG(INFO, VHOST_CONFIG, + "free connfd = %d for device '%s'\n", + vsocket->connfd, path); + close(vsocket->connfd); + vhost_destroy_device(conn->vid); + free(conn); } - free(vhost_user.vsockets[i]->path); - free(vhost_user.vsockets[i]); + free(vsocket->path); + free(vsocket); count = --vhost_user.vsocket_cnt; vhost_user.vsockets[i] = vhost_user.vsockets[count];