vswitchd: Always cleanup userspace datapath.

'netdev' datapath is implemented within ovs-vswitchd process and can
not exist without it, so it should be gracefully terminated with a
full cleanup of resources upon ovs-vswitchd exit.

This change forces dpif cleanup for 'netdev' datapath regardless of
passing '--cleanup' to 'ovs-appctl exit'. Such solution allowes to
not pass this additional option everytime for userspace datapath
installations and also allowes to not terminate system datapath in
setups where both datapaths runs at the same time.

The main part is that dpif_port_del() will lead to netdev_close()
and subsequent netdev_class->destroy(dev) which will stop HW NICs
and free their resources. For vhost-user interfaces it will invoke
vhost driver unregistering with a properly closed vhost-user
connection. For upcoming AF_XDP netdev this will allow to gracefully
destroy xdp sockets and unload xdp programs from linux interfaces.
Another important thing is that port deletion will also trigger
flushing of flows offloaded to HW NICs.

Exception made for 'internal' ports that could have user ip/route
configuration. These ports will not be removed without '--cleanup'.

This change fixes OVS disappearing from the DPDK point of view
(keeping HW NICs improperly configured, sudden closing of vhost-user
connections) and will help with linux devices clearing with upcoming
AF_XDP netdev support.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Tested-by: William Tu <u9012063@gmail.com>
Acked-by: Flavio Leitner <fbl@sysclose.org>
Acked-by: Ben Pfaff <blp@ovn.org>

diff --git a/NEWS b/NEWS
index 8ad2f6a2464a2525c5be6d56f2668ebe1a5e895e..097b1528ef22eaf033439bdbadc64c241f54b86c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,9 @@ Post-v2.11.0
      * New action "check_pkt_len".
      * Port configuration with "other-config:priority-tags" now has a mode
        that retains the 802.1Q header even if VLAN and priority are both zero.
+     * 'ovs-appctl exit' now implies cleanup of non-internal ports in userspace
+       datapath regardless of '--cleanup' option. Use '--cleanup' to remove
+       internal ports too.
    - OVSDB:
      * OVSDB clients can now resynchronize with clustered servers much more
        quickly after a brief disconnection, saving bandwidth and CPU time.

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d1b05f0b4936459d2934672e59946a27967fa689..f4b59e41b1697d5bee30cb75e9c35ab783b50bc7 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7437,6 +7437,7 @@ dpif_netdev_ipf_dump_done(struct dpif *dpif OVS_UNUSED, void *ipf_dump_ctx)
 
 const struct dpif_class dpif_netdev_class = {
     "netdev",
+    true,                       /* cleanup_required */
     dpif_netdev_init,
     dpif_netdev_enumerate,
     dpif_netdev_port_open_type,

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ba80a0079181bbc264a8aaa6d82f56225f0a643e..985a284267f5751cbaa8b09369f735d16d37cf9c 100644 (file)
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3384,6 +3384,7 @@ probe_broken_meters(struct dpif *dpif)
 \f
 const struct dpif_class dpif_netlink_class = {
     "system",
+    false,                      /* cleanup_required */
     NULL,                       /* init */
     dpif_netlink_enumerate,
     NULL,

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index b2a4dff9645c74699bcc854a3a8406d85507e480..12898b9e3c6d0adfeaecf7a3937dfd5da6cbbd63 100644 (file)
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -119,6 +119,12 @@ struct dpif_class {
      * the type assumed if no type is specified when opening a dpif. */
     const char *type;
 
+    /* If 'true', datapath ports should be destroyed on ofproto destruction.
+     *
+     * This is used by the vswitch at exit, so that it can clean any
+     * datapaths that can not exist without it (e.g. netdev datapath).  */
+    bool cleanup_required;
+
     /* Called when the dpif provider is registered, typically at program
      * startup.  Returning an error from this function will prevent any
      * datapath with this class from being created.

diff --git a/lib/dpif.c b/lib/dpif.c
index a18fb1b02cd4f3dd0af9063d4077fd026e846592..c88b2106f0860aa2265144eb87b32e701c895e19 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -498,6 +498,13 @@ dpif_type(const struct dpif *dpif)
     return dpif->dpif_class->type;
 }
 
+/* Checks if datapath 'dpif' requires cleanup. */
+bool
+dpif_cleanup_required(const struct dpif *dpif)
+{
+    return dpif->dpif_class->cleanup_required;
+}
+
 /* Returns the fully spelled out name for the given datapath 'type'.
  *
  * Normalized type string can be compared with strcmp().  Unnormalized type

diff --git a/lib/dpif.h b/lib/dpif.h
index 883472a59b070c3cc3136d89fbfa47b99fa36f52..289d574a01f8e077393c1aaca18587ebc0fc1745 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -419,6 +419,8 @@ const char *dpif_name(const struct dpif *);
 const char *dpif_base_name(const struct dpif *);
 const char *dpif_type(const struct dpif *);
 
+bool dpif_cleanup_required(const struct dpif *);
+
 int dpif_delete(struct dpif *);
 
 /* Statistics for a dpif as a whole. */

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cbeb6776f99d258c80e7add26c3ace9eb43c85b0..751535249e21b5a5f735a7b8e8a6b593cff5aa14 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1965,6 +1965,7 @@ port_destruct(struct ofport *port_, bool del)
     struct ofport_dpif *port = ofport_dpif_cast(port_);
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
     const char *devname = netdev_get_name(port->up.netdev);
+    const char *netdev_type = netdev_get_type(port->up.netdev);
     char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
     const char *dp_port_name;
 
@@ -1973,6 +1974,13 @@ port_destruct(struct ofport *port_, bool del)
     xlate_ofport_remove(port);
     xlate_txn_commit();
 
+    if (!del && strcmp(netdev_type,
+                       ofproto_port_open_type(port->up.ofproto, "internal"))) {
+        /* Check if datapath requires removal of attached ports.  Avoid
+         * removal of 'internal' ports to preserve user ip/route settings. */
+        del = dpif_cleanup_required(ofproto->backer->dpif);
+    }
+
     dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
                                               sizeof namebuf);
     if (del && dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {

diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index fcf22244a60fe8f25d620f471463a22b5e2eabbb..a5cdc3aa49913ce6f420ce4b39f61ed866213389 100644 (file)
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -110,6 +110,10 @@ how to configure Open vSwitch.
 Causes \fBovs\-vswitchd\fR to gracefully terminate. If \fI--cleanup\fR
 is specified, release datapath resources configured by \fBovs\-vswitchd\fR.
 Otherwise, datapath flows and other resources remains undeleted.
+Resources of datapaths that are integrated into \fBovs\-vswitchd\fR (e.g.
+the \fBnetdev\fR datapath type) are always released regardless of
+\fI--cleanup\fR except for ports with \fBinternal\fR type. Use \fI--cleanup\fR
+to release \fBinternal\fR ports too.
 .
 .IP "\fBqos/show-types\fR \fIinterface\fR"
 Queries the interface for a list of Quality of Service types that are