Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Don't fail vHost port creation if server mode is unavailable.
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 :
> 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
> ---
> 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(>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(_mutex);
> +
> +err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST);
> +if (err) {
> +goto out;
> +}
> +
> +ovs_mutex_lock(>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(>mutex);
> +out:
> ovs_mutex_unlock(_mutex);
> return err;
> }
> @@ -978,6 +1008,9 @@ netdev_dpdk_destruct(struct netdev *netdev)
> struct netdev_dpdk *dev =
[ovs-dev] [PATCH 3/3] netdev-dpdk: Don't fail vHost port creation if server mode is unavailable.
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
---
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(>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(_mutex);
+
+err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST);
+if (err) {
+goto out;
+}
+
+ovs_mutex_lock(>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(>mutex);
+out:
ovs_mutex_unlock(_mutex);
return err;
}
@@ -978,6 +1008,9 @@ netdev_dpdk_destruct(struct netdev *netdev)
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
ovs_mutex_lock(_mutex);
+ovs_list_remove(>list_node);
+ovs_mutex_unlock(_mutex);
+
ovs_mutex_lock(>mutex);
rte_eth_dev_stop(dev->port_id);
@@ -985,34 +1018,64 @@ netdev_dpdk_destruct(struct netdev *netdev)
>ingress_policer));
rte_free(dev->tx_q);
-ovs_list_remove(>list_node);
dpdk_mp_put(dev->dpdk_mp);
ovs_mutex_unlock(>mutex);
-ovs_mutex_unlock(_mutex);
}
/*
