From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout4.w1.samsung.com (mailout4.w1.samsung.com [210.118.77.14]) by dpdk.org (Postfix) with ESMTP id 410C93990 for ; Thu, 21 Jul 2016 10:33:17 +0200 (CEST) Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0OAN00E68P3FDF40@mailout4.w1.samsung.com> for dev@dpdk.org; Thu, 21 Jul 2016 09:33:15 +0100 (BST) X-AuditID: cbfec7f5-f792a6d000001302-39-579088cb3f9d Received: from eusync1.samsung.com ( [203.254.199.211]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 20.14.04866.BC880975; Thu, 21 Jul 2016 09:33:15 +0100 (BST) Received: from [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 <0OAN004KSP3ELF60@eusync1.samsung.com>; Thu, 21 Jul 2016 09:33:15 +0100 (BST) To: Yuanhan Liu References: <1469003563-27340-1-git-send-email-i.maximets@samsung.com> <20160721082419.GB28708@yliu-dev.sh.intel.com> Cc: dev@dpdk.org, Huawei Xie , Dyasly Sergey , Heetae Ahn , Thomas Monjalon From: Ilya Maximets Message-id: <579088C9.2030705@samsung.com> Date: Thu, 21 Jul 2016 11:33:13 +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: <20160721082419.GB28708@yliu-dev.sh.intel.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrBLMWRmVeSWpSXmKPExsVy+t/xy7qnOyaEG+x7x2Lx7tN2Jotpn2+z W7TPPMtkcaX9J7vF5NlSFl82TWezuD7hAqsDu8fF/juMHr8WLGX1WLznJZPHvJOBHn1bVjEG sEZx2aSk5mSWpRbp2yVwZWxcs5ixYL14xfWV11kbGPcJdTFyckgImEjc+3KUFcIWk7hwbz1b FyMXh5DAUkaJ9pUbmUESQgIvGCXOTnUAsYUFHCU2rO5hAbFFBHQlns5ZxwpRUy4xbetmJpBm ZoGNjBILpp5mA0mwCehInFp9hLGLkYODV0BL4v5UbZAwi4CqxL1Xu5lAbFGBCIlZ23+A2bwC ghI/Jt8Dm88pYC3x/dQqVpBWZgE9ifsXtUDCzALyEpvXvGWewCgwC0nHLISqWUiqFjAyr2IU TS1NLihOSs810itOzC0uzUvXS87P3cQICe+vOxiXHrM6xCjAwajEw7tjdX+4EGtiWXFl7iFG CQ5mJRHe/y0TwoV4UxIrq1KL8uOLSnNSiw8xSnOwKInzztz1PkRIID2xJDU7NbUgtQgmy8TB KdXAuJDLwdYk8Uhi++rk2hsGs+c8vzTfdBXHhIRPlq7BP6LmHPjMFspy58Lqlawy926cMhWo 0LwgIVvwZvIa6ysfp7X807hk7bJIkmfvjOxfT/UWTw+oOPi66ULwHhU5dQMbMYeVdy/VTXv6 3qpwXpHw8t9ddwIThNJW/Ty1XyB1922T75tV+mZy9iqxFGckGmoxFxUnAgDR2n1lawIAAA== Subject: Re: [dpdk-dev] [PATCH] 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 08:33:17 -0000 Thanks. Fixed. Best regards, Ilya Maximets. On 21.07.2016 11:24, Yuanhan Liu wrote: > On Wed, Jul 20, 2016 at 11:32:43AM +0300, 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") >> Signed-off-by: Ilya Maximets > > Again, thanks for the fix. > >> --- >> >> Patch prepared for master branch because dpdk-next-virtio doesn't contain >> commit acbff5c67ea7 ("vhost: fix crash when exceeding file descriptors"). >> Porting to dpdk-next-virtio/master is trivial and may be performed on >> demand. > > Yeah, my bad, I haven't updated it after rc2, since Thomas no longer > pull request from it. Anyway, you just remind me that I should have > done that. > >> /** >> * 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) { >> + (void) fdset_del(&vhost_user.fdset, >> + vsocket->listenfd); > > I would think the (void) cast is not neceessary here. > >> + 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); > > We should try not to break a log message into several lines, which > hurts "git grep". Here, it could be: > > RTE_LOG(INFO, VHOST_CONFIG, > "free connfd = %d for device '%s'\n", > vsocket->connfd, path); > > Besides the two minor nits, > > Acked-by: Yuanhan Liu > > --yliu > >