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

Reply via email to