I believe that this is not necessary anymore, since Ciara's change (which I just applied) 2d24d165d6a5("netdev-dpdk: Add new 'dpdkvhostuserclient' port type").
Thanks, Daniele 2016-09-05 6:36 GMT-07:00 Ilya Maximets <i.maxim...@samsung.com>: > Currently vHost client mode feature is broken in case where QEMU and OVS > are configured to create sockets in the same place. While adding the port, > OVS will try to register vHost driver in server mode and will fail because > socket already exist if QEMU was started first. > > In this situation port will not be created and will not be reconfigured > to client mode. > > To fix this let's not fail creation of vhost port if server mode is > unavailable. > > Fixes: c1ff66ac80b5 ("netdev-dpdk: vHost client mode and reconnect") > Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> > --- > lib/netdev-dpdk.c | 173 ++++++++++++++++++++++++++++++ > +++--------------------- > 1 file changed, 105 insertions(+), 68 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index be48218..2ba57d7 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -355,6 +355,8 @@ struct netdev_dpdk { > /* virtio identifier for vhost devices */ > ovsrcu_index vid; > > + bool vhost_driver_registered; > + > /* True if vHost device is 'up' and has been reconfigured at least > once */ > bool vhost_reconfigured; > > @@ -400,7 +402,8 @@ static bool dpdk_thread_is_pmd(void); > > static int netdev_dpdk_construct(struct netdev *); > > -int netdev_dpdk_get_vid(const struct netdev_dpdk *dev); > +static void netdev_dpdk_vhost_clear(struct netdev_dpdk *); > +static int netdev_dpdk_get_vid(const struct netdev_dpdk *); > > struct ingress_policer * > netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev); > @@ -828,6 +831,7 @@ netdev_dpdk_init(struct netdev *netdev, unsigned int > port_no, > dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu); > ovsrcu_index_init(&dev->vid, -1); > dev->vhost_reconfigured = false; > + dev->vhost_driver_registered = false; > /* initialise vHost port in server mode */ > dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT; > > @@ -903,6 +907,37 @@ get_vhost_id(struct netdev_dpdk *dev) > } > > static int > +netdev_dpdk_vhost_driver_register(struct netdev_dpdk *dev) > + OVS_REQUIRES(dev->mutex) > +{ > + int mode = !!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT); > + const char *str_mode[] = {"server", "client"}; > + const char *id = get_vhost_id(dev); > + int err; > + > + if (dev->vhost_driver_registered) { > + return 0; > + } > + > + err = rte_vhost_driver_register(id, dev->vhost_driver_flags); > + if (err) { > + VLOG_ERR("%s: Unable to register vhost driver in %s mode using > socket '" > + "%s'.\n", dev->up.name, str_mode[mode], id); > + return err; > + } > + > + dev->vhost_driver_registered = true; > + if (!mode) { > + /* OVS server mode - add this socket to list for deletion */ > + fatal_signal_add_file_to_unlink(id); > + } > + VLOG_INFO("%s: vhost driver registered in %s mode using socket > '%s'.\n", > + dev->up.name, str_mode[mode], id); > + > + return 0; > +} > + > +static int > netdev_dpdk_vhost_construct(struct netdev *netdev) > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > @@ -924,6 +959,13 @@ netdev_dpdk_vhost_construct(struct netdev *netdev) > } > > ovs_mutex_lock(&dpdk_mutex); > + > + err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST); > + if (err) { > + goto out; > + } > + > + ovs_mutex_lock(&dev->mutex); > /* Take the name of the vhost-user port and append it to the location > where > * the socket is to be created, then register the socket. Sockets are > * registered initially in 'server' mode. > @@ -931,21 +973,9 @@ netdev_dpdk_vhost_construct(struct netdev *netdev) > snprintf(dev->vhost_server_id, sizeof dev->vhost_server_id, "%s/%s", > vhost_sock_dir, name); > > - err = rte_vhost_driver_register(dev->vhost_server_id, > - dev->vhost_driver_flags); > - if (err) { > - VLOG_ERR("vhost-user socket device setup failure for socket %s\n", > - dev->vhost_server_id); > - } else { > - if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > - /* OVS server mode - add this socket to list for deletion */ > - fatal_signal_add_file_to_unlink(dev->vhost_server_id); > - VLOG_INFO("Socket %s created for vhost-user port %s\n", > - dev->vhost_server_id, name); > - } > - err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST); > - } > - > + netdev_dpdk_vhost_driver_register(dev); > + ovs_mutex_unlock(&dev->mutex); > +out: > ovs_mutex_unlock(&dpdk_mutex); > return err; > } > @@ -978,6 +1008,9 @@ netdev_dpdk_destruct(struct netdev *netdev) > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > > ovs_mutex_lock(&dpdk_mutex); > + ovs_list_remove(&dev->list_node); > + ovs_mutex_unlock(&dpdk_mutex); > + > ovs_mutex_lock(&dev->mutex); > > rte_eth_dev_stop(dev->port_id); > @@ -985,34 +1018,64 @@ netdev_dpdk_destruct(struct netdev *netdev) > &dev->ingress_policer)); > > rte_free(dev->tx_q); > - ovs_list_remove(&dev->list_node); > dpdk_mp_put(dev->dpdk_mp); > > ovs_mutex_unlock(&dev->mutex); > - ovs_mutex_unlock(&dpdk_mutex); > } > > /* rte_vhost_driver_unregister() can call back destroy_device(), which > will > - * try to acquire 'dpdk_mutex' and possibly 'dev->mutex'. To avoid a > - * deadlock, none of the mutexes must be held while calling this > function. */ > + * try to acquire 'dpdk_mutex'. To avoid a deadlock, 'dpdk_mutex' must > not > + * be held while calling this function. */ > static int > -dpdk_vhost_driver_unregister(struct netdev_dpdk *dev OVS_UNUSED, > - char *vhost_id) > +netdev_dpdk_vhost_driver_unregister(struct netdev_dpdk *dev) > + OVS_REQUIRES(dev->mutex) > OVS_EXCLUDED(dpdk_mutex) > - OVS_EXCLUDED(dev->mutex) > { > - return rte_vhost_driver_unregister(vhost_id); > + int mode = !!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT); > + const char *id = get_vhost_id(dev); > + int err; > + > + if (!dev->vhost_driver_registered) { > + return 0; > + } > + > + if (netdev_dpdk_get_vid(dev) >= 0) { > + /* Device still attached and may exist in 'dpdk_list'. Clear the > device > + * to avoid possible double locking of 'dev->mutex' inside the > + * destroy_device() callback. */ > + netdev_dpdk_vhost_clear(dev); > + } > + > + err = rte_vhost_driver_unregister(id); > + if (err) { > + VLOG_ERR("%s: Unable to unregister vhost driver for socket > '%s'.\n", > + dev->up.name, id); > + return err; > + } > + > + dev->vhost_driver_registered = false; > + if (!mode) { > + /* OVS server mode - remove this socket from list for deletion */ > + fatal_signal_remove_file_to_unlink(id); > + > + } > + > + VLOG_INFO("%s: vhost driver for socket '%s' unregistered.\n", > + dev->up.name, id); > + > + return 0; > } > > static void > netdev_dpdk_vhost_destruct(struct netdev *netdev) > { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > - char *vhost_id; > > ovs_mutex_lock(&dpdk_mutex); > - ovs_mutex_lock(&dev->mutex); > + ovs_list_remove(&dev->list_node); > + ovs_mutex_unlock(&dpdk_mutex); > > + ovs_mutex_lock(&dev->mutex); > /* Guest becomes an orphan if still attached. */ > if (netdev_dpdk_get_vid(dev) >= 0 > && !(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > @@ -1027,21 +1090,10 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev) > &dev->ingress_policer)); > > rte_free(dev->tx_q); > - ovs_list_remove(&dev->list_node); > dpdk_mp_put(dev->dpdk_mp); > > - vhost_id = xstrdup(get_vhost_id(dev)); > - > + netdev_dpdk_vhost_driver_unregister(dev); > ovs_mutex_unlock(&dev->mutex); > - ovs_mutex_unlock(&dpdk_mutex); > - > - if (dpdk_vhost_driver_unregister(dev, vhost_id)) { > - VLOG_ERR("Unable to remove vhost-user socket %s", vhost_id); > - } else if (!(dev->vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > - /* OVS server mode - remove this socket from list for deletion */ > - fatal_signal_remove_file_to_unlink(vhost_id); > - } > - free(vhost_id); > } > > static void > @@ -2403,6 +2455,14 @@ netdev_dpdk_txq_map_clear(struct netdev_dpdk *dev) > } > } > > +static void > +netdev_dpdk_vhost_clear(struct netdev_dpdk *dev) > + OVS_REQUIRES(dev->mutex) > +{ > + dev->vhost_reconfigured = false; > + ovsrcu_index_set(&dev->vid, -1); > + netdev_dpdk_txq_map_clear(dev); > +} > /* > * Remove a virtio-net device from the specific vhost port. Use > dev->remove > * flag to stop any more packets from being sent or received to/from a VM > and > @@ -2423,10 +2483,7 @@ destroy_device(int vid) > if (netdev_dpdk_get_vid(dev) == vid) { > > ovs_mutex_lock(&dev->mutex); > - dev->vhost_reconfigured = false; > - ovsrcu_index_set(&dev->vid, -1); > - netdev_dpdk_txq_map_clear(dev); > - > + netdev_dpdk_vhost_clear(dev); > netdev_change_seq_changed(&dev->up); > ovs_mutex_unlock(&dev->mutex); > exists = true; > @@ -2497,7 +2554,7 @@ vring_state_changed(int vid, uint16_t queue_id, int > enable) > return 0; > } > > -int > +static int > netdev_dpdk_get_vid(const struct netdev_dpdk *dev) > { > return ovsrcu_index_get(&dev->vid); > @@ -2999,36 +3056,16 @@ netdev_dpdk_vhost_reconfigure(struct netdev > *netdev) > && !(netdev_dpdk_get_vid(dev) >= 0) > && strlen(dev->vhost_client_id)) { > /* Unregister server-mode device */ > - char *vhost_id = xstrdup(get_vhost_id(dev)); > - > - ovs_mutex_unlock(&dev->mutex); > - err = dpdk_vhost_driver_unregister(dev, vhost_id); > - free(vhost_id); > - ovs_mutex_lock(&dev->mutex); > - if (err) { > - VLOG_ERR("Unable to remove vhost-user socket %s", > - get_vhost_id(dev)); > - } else { > - fatal_signal_remove_file_to_unlink(get_vhost_id(dev)); > - /* Register client-mode device */ > - err = rte_vhost_driver_register(dev->vhost_client_id, > - RTE_VHOST_USER_CLIENT); > - if (err) { > - VLOG_ERR("vhost-user device setup failure for device > %s\n", > - dev->vhost_client_id); > - } else { > - /* Configuration successful */ > - dev->vhost_driver_flags |= RTE_VHOST_USER_CLIENT; > - VLOG_INFO("vHost User device '%s' changed to 'client' > mode, " > - "using client socket '%s'", > - dev->up.name, get_vhost_id(dev)); > - } > + err = netdev_dpdk_vhost_driver_unregister(dev); > + if (!err) { > + dev->vhost_driver_flags |= RTE_VHOST_USER_CLIENT; > + err = netdev_dpdk_vhost_driver_register(dev); > } > } > > ovs_mutex_unlock(&dev->mutex); > > - return 0; > + return err; > } > > #define NETDEV_DPDK_CLASS(NAME, CONSTRUCT, DESTRUCT, \ > -- > 2.7.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev