Thanks for the patch and the review! Pushed to master, branch-2.5 and branch-2.4
On 22/03/2016 16:27, "Traynor, Kevin" <kevin.tray...@intel.com> wrote: >> -----Original Message----- >> From: Ilya Maximets [mailto:i.maxim...@samsung.com] >> Sent: Tuesday, March 22, 2016 12:42 PM >> To: dev@openvswitch.org; Daniele Di Proietto <diproiet...@vmware.com> >> Cc: Dyasly Sergey <s.dya...@samsung.com>; Ben Pfaff <b...@ovn.org>; >>Flavio >> Leitner <f...@sysclose.org>; Traynor, Kevin <kevin.tray...@intel.com>; >>Ilya >> Maximets <i.maxim...@samsung.com> >> Subject: [PATCH] netdev-dpdk: Fix crash when changing the vhost-user >>port. >> >> According to netdev-provider API: >> 'The "destruct" function is not allowed to fail.' >> >> netdev-dpdk breaks this restriction for vhost-user ports. >> This leads to SIGABRT or SIGSEGV in dpdk_watchdog thread >> because 'dealloc' will be called anyway indifferently >> to result of 'destruct'. >> >> For example, if we call >> # ovs-vsctl set interface vhost1 ofport_request=5 >> while QEMU still attached, we'll get: >> ------------------[cut]------------------ >> |dpdk|ERR|Can not remove port, vhost device still attached >> VHOST_CONFIG: socket created, fd:98 >> VHOST_CONFIG: fail to bind fd:98, remove file:/home/vhost1 and try >>again. >> |dpdk|ERR|vhost-user socket device setup failure for socket /home/vhost1 >> |bridge|WARN|could not open network device vhost1 (Unknown error -1) >> ovs-vswitchd(dpdk_watchdog1): lib/netdev-dpdk.c:532: ovs_mutex_lock_at() >> passed uninitialized ovs_mutex >> >> Program received signal SIGABRT, Aborted. >> ------------------[cut]------------------ >> >> Fix that by removing port anyway even when guest is still >> attached. Guest becomes an orphan in that case but OVS >> will not crash and will continue forwarding for other ports. >> VM restart required to restore connectivity. > >The issue in destruct was reported (without a crash) by Jan also. >http://openvswitch.org/pipermail/discuss/2016-February/020271.html > >I wanted to try and fold in re-add without restarting the guest along >with this. I got it working for most part but without vhost client mode >support in DPDK, it's hard to cover all possibilities and not at least >leak the sockets. I think a re-add without restarting the guest is >something to revisit when vhost pmd and vhost client mode are available >in DPDK. > >Acked-by: Kevin Traynor <kevin.tray...@intel.com> > >> >> Fixes: 58397e6c1e6c ("netdev-dpdk: add dpdk vhost-cuse ports") >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >> --- >> lib/netdev-dpdk.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 6ac0eec..f4ed210 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -869,10 +869,13 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev_) >> { >> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev_); >> >> - /* Can't remove a port while a guest is attached to it. */ >> + /* Guest becomes an orphan if still attached. */ >> if (netdev_dpdk_get_virtio(dev) != NULL) { >> - VLOG_ERR("Can not remove port, vhost device still attached"); >> - return; >> + VLOG_ERR("Removing port '%s' while vhost device still >>attached.", >> + netdev_->name); >> + VLOG_ERR("To restore connectivity after re-adding of port, VM >>on >> socket" >> + " '%s' must be restarted.", >> + dev->vhost_id); >> } >> >> if (rte_vhost_driver_unregister(dev->vhost_id)) { >> -- >> 2.5.0 > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
