[ovs-dev] [PATCH] vswitchd: Always cleanup userspace datapath.

[Ilya Maximets]

'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.

This 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 at samsung.com>
---
 NEWS                       | 2 ++
 lib/dpif-netdev.c          | 1 +
 lib/dpif-netlink.c         | 1 +
 lib/dpif-provider.h        | 6 ++++++
 lib/dpif.c                 | 7 +++++++
 lib/dpif.h                 | 2 ++
 ofproto/ofproto-dpif.c     | 4 +++-
 vswitchd/ovs-vswitchd.8.in | 3 +++
 8 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index af1659ce8..dbbc3827b 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,8 @@ 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 userspace datapath cleanup regardless
+       of '--cleanup' option.
    - 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 4e73f96fb..ad6ff5338 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7411,6 +7411,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 ba80a0079..985a28426 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3384,6 +3384,7 @@ probe_broken_meters(struct dpif *dpif)
 
 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 b2a4dff96..f503d116a 100644
--- 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 must be destroyed on ofproto destruction.
+     *
+     * This is used by the vswitch at exit, so that it can delete 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 a18fb1b02..c88b2106f 100644
--- 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 883472a59..289d574a0 100644
--- 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 cbeb6776f..957f70cfa 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -678,7 +678,7 @@ close_dpif_backer(struct dpif_backer *backer, bool del)
     shash_find_and_delete(&all_dpif_backers, backer->type);
     free(backer->type);
     free(backer->dp_version_string);
-    if (del) {
+    if (del || dpif_cleanup_required(backer->dpif)) {
         dpif_delete(backer->dpif);
     }
     dpif_close(backer->dpif);
@@ -1973,6 +1973,8 @@ port_destruct(struct ofport *port_, bool del)
     xlate_ofport_remove(port);
     xlate_txn_commit();
 
+    del = 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 fcf22244a..b5e1c1a8f 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -110,6 +110,9 @@ 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.
 .
 .IP "\fBqos/show-types\fR \fIinterface\fR"
 Queries the interface for a list of Quality of Service types that are
-- 
2.17.1