vswitchd: Add --cleanup option to the 'appctl exit' command

'appctl exit' stops the running vswitchd daemon, without releasing
the datapath resources (such as bridges and ports) that vswitchd
has created.  This is expected when vswitchd is to be relaunched, to
reduce the perturbation of exiting traffic and connections.

However, when vswitchd is intended to be shutdown permanently, it
is desirable not to leak datapath resources.  In theory, this can be
achieved by removing the corresponding configurations from
OVSDB before shutting down vswitchd. However it is not always
possible in practice. Sometimes it is convenient and robust for
vswitchd to release all datapath resources that it has configured.
Add 'appctl exit --cleanup' option for this use case.

Signed-off-by: Andy Zhou <azhou@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>

diff --git a/NEWS b/NEWS
index 4d03ca529fa6883b6f5decb0ce3795b438b2b5c9..bd43788f14f156be0bcf455331cb14189ed63b43 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,7 @@ Post-v2.7.0
      * The port status bit OFPPS_LIVE now reflects link aliveness.
    - Fedora Packaging:
      * OVN services are no longer restarted automatically after upgrade.
+   - Add --cleanup option to command 'ovs-appctl exit' (see ovs-vswitchd(8)).
 
 v2.7.0 - 21 Feb 2017
 ---------------------

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bd45be5ac5c305df3b16c6feade5abd3082ceb18..9c3a5673c0bd9c84d231582ae9e4ad5dfe137993 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -645,7 +645,7 @@ dealloc(struct ofproto *ofproto_)
 }
 
 static void
-close_dpif_backer(struct dpif_backer *backer)
+close_dpif_backer(struct dpif_backer *backer, bool del)
 {
     ovs_assert(backer->refcount > 0);
 
@@ -661,6 +661,9 @@ close_dpif_backer(struct dpif_backer *backer)
     shash_find_and_delete(&all_dpif_backers, backer->type);
     free(backer->type);
     free(backer->dp_version_string);
+    if (del) {
+        dpif_delete(backer->dpif);
+    }
     dpif_close(backer->dpif);
     id_pool_destroy(backer->meter_ids);
     free(backer);
@@ -773,7 +776,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     if (error) {
         VLOG_ERR("failed to listen on datapath of type %s: %s",
                  type, ovs_strerror(error));
-        close_dpif_backer(backer);
+        close_dpif_backer(backer, false);
         return error;
     }
 
@@ -1525,7 +1528,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
 }
 
 static void
-destruct(struct ofproto *ofproto_)
+destruct(struct ofproto *ofproto_, bool del)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct ofproto_async_msg *am;
@@ -1578,7 +1581,7 @@ destruct(struct ofproto *ofproto_)
 
     seq_destroy(ofproto->ams_seq);
 
-    close_dpif_backer(ofproto->backer);
+    close_dpif_backer(ofproto->backer, del);
 }
 
 static int

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 688a9e5d32ebdd83cf38251057576bc77c6f8885..9dc73c4821190e08be9d5a7be18c5b4929c6c118 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -829,7 +829,7 @@ struct ofproto_class {
      */
     struct ofproto *(*alloc)(void);
     int (*construct)(struct ofproto *ofproto);
-    void (*destruct)(struct ofproto *ofproto);
+    void (*destruct)(struct ofproto *ofproto, bool del);
     void (*dealloc)(struct ofproto *ofproto);
 
     /* Performs any periodic activity required by 'ofproto'.  It should:

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 32b5b303b9d1f9007bb0d8633451c6d0a20e897c..d5410fd1b20fe413c91a43d79250d26180a337ae 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1649,7 +1649,7 @@ ofproto_destroy(struct ofproto *p, bool del)
         free(usage);
     }
 
-    p->ofproto_class->destruct(p);
+    p->ofproto_class->destruct(p, del);
 
     /* We should not postpone this because it involves deleting a listening
      * socket which we may want to reopen soon. 'connmgr' may be used by other

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index ebb6249416fa21a681dbe041aa6daf4937433e1b..31203d1ec232a30d11fbfb72cf5d507462a67dd5 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -496,14 +496,14 @@ bridge_init(const char *remote)
 }
 
 void
-bridge_exit(void)
+bridge_exit(bool delete_datapath)
 {
     struct bridge *br, *next_br;
 
     if_notifier_destroy(ifnotifier);
     seq_destroy(ifaces_changed);
     HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
-        bridge_destroy(br, false);
+        bridge_destroy(br, delete_datapath);
     }
     ovsdb_idl_destroy(idl);
 }

diff --git a/vswitchd/bridge.h b/vswitchd/bridge.h
index 3783a21e3b4cb82b05b89ad33b466b0382734e4e..8b2fce451928540f8c583c52e02d2e38a11fb2af 100644 (file)
--- a/vswitchd/bridge.h
+++ b/vswitchd/bridge.h
@@ -16,10 +16,12 @@
 #ifndef VSWITCHD_BRIDGE_H
 #define VSWITCHD_BRIDGE_H 1
 
+#include <stdbool.h>
+
 struct simap;
 
 void bridge_init(const char *remote);
-void bridge_exit(void);
+void bridge_exit(bool delete_datapath);
 
 void bridge_run(void);
 void bridge_wait(void);

diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index 8496aa68af973f9bbb937a9363c376e8d8337f08..be7bd66c7b2f86da3265b43232187bfc62e17697 100644 (file)
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -109,8 +109,11 @@ configuration.
 described below.  The command descriptions assume an understanding of
 how to configure Open vSwitch.
 .SS "GENERAL COMMANDS"
-.IP "\fBexit\fR"
-Causes \fBovs\-vswitchd\fR to gracefully terminate.
+.IP "\fBexit\fR \fI--cleanup\fR"
+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.
+.
 .IP "\fBqos/show-types\fR \fIinterface\fR"
 Queries the interface for a list of Quality of Service types that are
 configurable via Open vSwitch for the given \fIinterface\fR.

diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index bed3fb5c374df454a84e2d5fa549967c51622ffb..7de6d89abbeab6ef4ee7e52a29553da25356de14 100644 (file)
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -60,13 +60,19 @@ static unixctl_cb_func ovs_vswitchd_exit;
 static char *parse_options(int argc, char *argv[], char **unixctl_path);
 OVS_NO_RETURN static void usage(void);
 
+struct ovs_vswitchd_exit_args {
+    bool *exiting;
+    bool *cleanup;
+};
+
 int
 main(int argc, char *argv[])
 {
     char *unixctl_path = NULL;
     struct unixctl_server *unixctl;
     char *remote;
-    bool exiting;
+    bool exiting, cleanup;
+    struct ovs_vswitchd_exit_args exit_args = {&exiting, &cleanup};
     int retval;
 
     set_program_name(argv[0]);
@@ -92,12 +98,14 @@ main(int argc, char *argv[])
     if (retval) {
         exit(EXIT_FAILURE);
     }
-    unixctl_command_register("exit", "", 0, 0, ovs_vswitchd_exit, &exiting);
+    unixctl_command_register("exit", "[--cleanup]", 0, 1,
+                             ovs_vswitchd_exit, &exit_args);
 
     bridge_init(remote);
     free(remote);
 
     exiting = false;
+    cleanup = false;
     while (!exiting) {
         memory_run();
         if (memory_should_report()) {
@@ -124,7 +132,7 @@ main(int argc, char *argv[])
             exiting = true;
         }
     }
-    bridge_exit();
+    bridge_exit(cleanup);
     unixctl_server_destroy(unixctl);
     service_stop();
 
@@ -265,10 +273,11 @@ usage(void)
 }
 
 static void
-ovs_vswitchd_exit(struct unixctl_conn *conn, int argc OVS_UNUSED,
-                  const char *argv[] OVS_UNUSED, void *exiting_)
+ovs_vswitchd_exit(struct unixctl_conn *conn, int argc,
+                  const char *argv[], void *exit_args_)
 {
-    bool *exiting = exiting_;
-    *exiting = true;
+    struct ovs_vswitchd_exit_args *exit_args = exit_args_;
+    *exit_args->exiting = true;
+    *exit_args->cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
     unixctl_command_reply(conn, NULL);
 }