[ovs-dev] [PATCH 3/3] netdev-dpdk: Don't fail vHost port creation if server mode is unavailable.

Daniele Di Proietto diproiettod at ovn.org
Mon Sep 19 21:51:24 UTC 2016


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.maximets at 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.maximets at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list