Hi, Ciara. I'm suggesting also following change: --------------------------------------------------------------------------------- diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 57dc437..f092fa2 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -959,7 +963,8 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev) struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); /* Guest becomes an orphan if still attached. */ - if (netdev_dpdk_get_vid(dev) >= 0) { + if (netdev_dpdk_get_vid(dev) >= 0 + && !(vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { 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" ---------------------------------------------------------------------------------
Few comments inline. On 04.08.2016 13:42, Ciara Loftus wrote: > A new other_config DB option has been added called 'vhost-driver-mode'. > By default this is set to 'server' which is the mode of operation OVS > with DPDK has used up until this point - whereby OVS creates and manages > vHost user sockets. > > If set to 'client', OVS will act as the vHost client and connect to > sockets created and managed by QEMU which acts as the server. This mode > allows for reconnect capability, which allows vHost ports to resume > normal connectivity in event of switch reset. > > QEMU v2.7.0+ is required when using OVS in client mode and QEMU in > server mode. > > Signed-off-by: Ciara Loftus <ciara.lof...@intel.com> > --- > INSTALL.DPDK-ADVANCED.md | 27 +++++++++++++++++++++++++++ > NEWS | 1 + > lib/netdev-dpdk.c | 28 +++++++++++++++++++++------- > vswitchd/vswitch.xml | 13 +++++++++++++ > 4 files changed, 62 insertions(+), 7 deletions(-) > > diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md > index f9587b5..a773533 100755 > --- a/INSTALL.DPDK-ADVANCED.md > +++ b/INSTALL.DPDK-ADVANCED.md > @@ -483,6 +483,33 @@ For users wanting to do packet forwarding using kernel > stack below are the steps > where `-L`: Changes the numbers of channels of the specified network > device > and `combined`: Changes the number of multi-purpose channels. > > + 4. Enable OVS vHost client-mode & vHost reconnect (OPTIONAL) > + > + By default, OVS DPDK acts as the vHost socket server and QEMU the > + client. In QEMU v2.7 the option is available for QEMU to act as the > + server. In order for this to work, OVS DPDK must be switched to > 'client' > + mode. This is possible by setting the 'vhost-driver-mode' DB entry to > + 'client' like so: > + > + ``` > + ovs-vsctl set Open_vSwitch . other_config:vhost-driver-mode="client" > + ``` > + > + This must be done before the switch is launched. It cannot sucessfully > + be changed after switch has launched. > + > + One must also append ',server' to the 'chardev' arguments on the QEMU > + command line, to instruct QEMU to use vHost server mode, like so: > + > + ```` > + -chardev > socket,id=char0,path=/usr/local/var/run/openvswitch/vhost0,server > + ```` > + > + One benefit of using this mode is the ability for vHost ports to > + 'reconnect' in event of the switch crashing or being brought down. > Once > + it is brought back up, the vHost ports will reconnect automatically > and > + normal service will resume. > + > - VM Configuration with libvirt > > * change the user/group, access control policty and restart libvirtd. > diff --git a/NEWS b/NEWS > index 9f09e1c..99412ba 100644 > --- a/NEWS > +++ b/NEWS > @@ -70,6 +70,7 @@ Post-v2.5.0 > fragmentation or NAT support yet) > * Support for DPDK 16.07 > * Remove dpdkvhostcuse port type. > + * OVS client mode for vHost and vHost reconnect (Requires QEMU 2.7) > - Increase number of registers to 16. > - ovs-benchmark: This utility has been removed due to lack of use and > bitrot. > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 7692cc8..c528cb4 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -136,7 +136,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / > ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF)) > #define OVS_VHOST_QUEUE_DISABLED (-2) /* Queue was disabled by guest and > not > * yet mapped to another queue. */ > > -static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */ > +static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */ > +static uint64_t vhost_driver_flags = 0; /* Denote whether client/server mode > */ > > #define VHOST_ENQ_RETRY_NUM 8 > #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) > @@ -833,7 +834,6 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev) > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > const char *name = netdev->name; > int err; > - uint64_t flags = 0; > > /* 'name' is appended to 'vhost_sock_dir' and used to create a socket in > * the file system. '/' or '\' would traverse directories, so they're not > @@ -856,14 +856,17 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev) > snprintf(dev->vhost_id, sizeof(dev->vhost_id), "%s/%s", > vhost_sock_dir, name); > > - err = rte_vhost_driver_register(dev->vhost_id, flags); > + err = rte_vhost_driver_register(dev->vhost_id, vhost_driver_flags); > if (err) { > VLOG_ERR("vhost-user socket device setup failure for socket %s\n", > dev->vhost_id); > } else { > - fatal_signal_add_file_to_unlink(dev->vhost_id); > - VLOG_INFO("Socket %s created for vhost-user port %s\n", > - dev->vhost_id, name); > + if (!(vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > + /* OVS server mode - OVS may delete the socket */ > + fatal_signal_add_file_to_unlink(dev->vhost_id); > + VLOG_INFO("Socket %s created for vhost-user port %s\n", > + dev->vhost_id, name); > + } > err = netdev_dpdk_init(netdev, -1, DPDK_DEV_VHOST); > } > > @@ -927,7 +930,8 @@ netdev_dpdk_vhost_destruct(struct netdev *netdev) > > if (rte_vhost_driver_unregister(dev->vhost_id)) { > VLOG_ERR("Unable to remove vhost-user socket %s", dev->vhost_id); > - } else { > + } else if (!(vhost_driver_flags & RTE_VHOST_USER_CLIENT)) { > + /* OVS server mode - OVS may delete the socket */ This comment is a little confusing because following function will not remove the socket. It will remove socket's name from the list for deletion. Socket actually was already deleted inside 'rte_vhost_driver_unregister()'. > fatal_signal_remove_file_to_unlink(dev->vhost_id); > } > > @@ -3181,6 +3185,7 @@ dpdk_init__(const struct smap *ovs_other_config) > int err = 0; > cpu_set_t cpuset; > char *sock_dir_subcomponent; > + const char *val; > > if (!smap_get_bool(ovs_other_config, "dpdk-init", false)) { > VLOG_INFO("DPDK Disabled - to change this requires a restart.\n"); > @@ -3211,6 +3216,15 @@ dpdk_init__(const struct smap *ovs_other_config) > } else { > vhost_sock_dir = sock_dir_subcomponent; > } > + val = smap_get(ovs_other_config, "vhost-driver-mode"); > + if (val != NULL && strncmp(val, "client", strlen(val)) == 0) { Why 'strncmp' instead 'strcmp'? It will not protect anything because you're using 'strlen(val)' to get the length. Also, I suggest replace checking for 'NULL' or '0' with '!' notation, e.g 'if (val && !strcmp(val, "client"))'. > + vhost_driver_flags |= RTE_VHOST_USER_CLIENT; > + VLOG_INFO("OVS client mode (QEMU server mode) selected for vHost"); > + } else { > + /* default case */ Generally, default case should go in 'if', not in 'else'. > + vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT; > + VLOG_INFO("OVS server mode (QEMU client mode) selected for vHost"); > + } > > argv = grow_argv(&argv, 0, 1); > argc = 1; > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index 63f0d89..41807b3 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -299,6 +299,19 @@ > </p> > </column> > > + <column name="other_config" key="vhost-driver-mode" > + type='{"type": "string", "enum": ["set", ["server", > "client"]]}'> > + <p> > + Specifies which mode OVS will use for vHost. In 'server' mode, OVS > + creates and destroys the vHost User sockets. In 'client' mode, OVS > + attaches to sockets created by QEMU. > + </p> > + <p> > + Defaults to 'server' mode. Changing this value requires restarting > + the daemon. > + </p> > + </column> > + > <column name="other_config" key="n-handler-threads" > type='{"type": "integer", "minInteger": 1}'> > <p> > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev