From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id DD9F01F5 for ; Thu, 21 Jul 2016 14:57:09 +0200 (CEST) Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OAO009LO1B12WA0@mailout1.w1.samsung.com> for dev@dpdk.org; Thu, 21 Jul 2016 13:57:01 +0100 (BST) X-AuditID: cbfec7f5-f792a6d000001302-3c-5790c69d7963 Received: from eusync3.samsung.com ( [203.254.199.213]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 66.4E.04866.D96C0975; Thu, 21 Jul 2016 13:57:01 +0100 (BST) Received: from [106.109.129.180] by eusync3.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0OAO00DLC1B06L40@eusync3.samsung.com>; Thu, 21 Jul 2016 13:57:01 +0100 (BST) To: dev@dpdk.org, Huawei Xie , Yuanhan Liu References: <1469003563-27340-1-git-send-email-i.maximets@samsung.com> <1469089911-15480-1-git-send-email-i.maximets@samsung.com> Cc: Dyasly Sergey , Heetae Ahn , Thomas Monjalon From: Ilya Maximets Message-id: <5790C69C.8010603@samsung.com> Date: Thu, 21 Jul 2016 15:57:00 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <1469089911-15480-1-git-send-email-i.maximets@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrJLMWRmVeSWpSXmKPExsVy+t/xq7pzj00IN7j2VMbi3aftTBbTPt9m t2ifeZbJ4kr7T3aLybOlLL5sms5mcX3CBVYHdo+L/XcYPX4tWMrqsXjPSyaPeScDPfq2rGIM YI3isklJzcksSy3St0vgytjbMZGpoN284kF7B1MD407tLkZODgkBE4nL834yQ9hiEhfurWfr YuTiEBJYyijx/fcUFgjnBaPErFvzWLsYOTiEBVwklkwxAGkQEUiQOLL/NytETTOjRMusnywg CWaBBkaJyx9LQGw2AR2JU6uPMILYvAJaEndW/AbbxiKgKtG9/RSYLSoQITFr+w8miBpBiR+T 74HN4RRwl1i49RcjyF5mAT2J+xe1IMbLS2xe85Z5AqPALCQdsxCqZiGpWsDIvIpRNLU0uaA4 KT3XSK84Mbe4NC9dLzk/dxMjJMC/7mBceszqEKMAB6MSD++O1f3hQqyJZcWVuYcYJTiYlUR4 xQ9NCBfiTUmsrEotyo8vKs1JLT7EKM3BoiTOO3PX+xAhgfTEktTs1NSC1CKYLBMHp1QDY1mQ 7d6DbwLPZ0ubfqqbxqep73s47aPKpfu6G3bmmW7kqGXemFF/IWzWB93vH3dKK8bu+T7pb96N 6wfe1DoWvWn9J2Yy7frR7FKRlO5PQZ/1HxTohjEa9s/pi7H7+OPj9cLO2XbuIplKq/a0L607 HblgveX0x80HJueGhlxSudi6Zu6rQzV+s5RYijMSDbWYi4oTAQeeltZsAgAA Subject: Re: [dpdk-dev] [PATCH v2] 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: , X-List-Received-Date: Thu, 21 Jul 2016 12:57:10 -0000 I've fixed leak of file descriptors in 'vhost_user_remove_reconnect()' and sent v3. On 21.07.2016 11:31, Ilya Maximets wrote: > 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") > > Acked-by: Yuanhan Liu > Signed-off-by: Ilya Maximets > --- > 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 | 56 ++++++++++++++++++++++++---- > 3 files changed, 62 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..8c6a096 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,29 @@ 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); > + free(reconn); > + found = true; > + break; > + } > + } > + pthread_mutex_unlock(&reconn_list.mutex); > + return found; > +} > + > /** > * Unregister the specified vhost socket > */ > @@ -672,20 +700,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]; >