vswitchd: Wait for a bridge exit before replying to exit unixctl.

Before the cleanup option, the bridge_exit() call was fairly fast,
because it didn't include any particularly long operations.  However,
with the cleanup flag, this function destroys a lot of datapath
resources freeing a lot of memory, waiting on RCU and talking to
the kernel.  That may take a noticeable amount of time, especially
on a busy system or under profilers/sanitizers.  However, the unixctl
'exit' command replies instantly without waiting for any work to
actually be done.  This may cause system test failures or other
issues where scripts expect ovs-vswitchd to exit or destroy all the
datapath resources shortly after appctl call.

Fix that by waiting for the bridge_exit() before replying to the user.
At least, all the datapath resources will actually be destroyed by
the time ovs-appctl exits.

Also moving a structure from stack to global.  Seems cleaner this way.

Since we're not replying right away and it's technically possible
to have multiple clients requesting exit at the same time, storing
connections in an array.

Fixes: fe13ccd ("vswitchd: Add --cleanup option to the 'appctl exit' command")
Acked-by: Eelco Chaudron <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>

vswitchd/ovs-vswitchd.c

@@ -65,19 +65,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;
-};
+static struct ovs_vswitchd_exit_args {
+    struct unixctl_conn **conns;
+    size_t n_conns;
+    bool exiting;
+    bool cleanup;
+} exit_args;
 
 int
 main(int argc, char *argv[])
 {
-    char *unixctl_path = NULL;
     struct unixctl_server *unixctl;
+    char *unixctl_path = NULL;
     char *remote;
-    bool exiting, cleanup;
-    struct ovs_vswitchd_exit_args exit_args = {&exiting, &cleanup};
     int retval;
 
     set_program_name(argv[0]);
@@ -108,14 +108,12 @@ main(int argc, char *argv[])
         exit(EXIT_FAILURE);
     }
     unixctl_command_register("exit", "[--cleanup]", 0, 1,
-                             ovs_vswitchd_exit, &exit_args);
+                             ovs_vswitchd_exit, NULL);
 
     bridge_init(remote);
     free(remote);
 
-    exiting = false;
-    cleanup = false;
-    while (!exiting) {
+    while (!exit_args.exiting) {
         OVS_USDT_PROBE(main, run_start);
         memory_run();
         if (memory_should_report()) {
@@ -134,16 +132,22 @@ main(int argc, char *argv[])
         bridge_wait();
         unixctl_server_wait(unixctl);
         netdev_wait();
-        if (exiting) {
+        if (exit_args.exiting) {
             poll_immediate_wake();
         }
         OVS_USDT_PROBE(main, poll_block);
         poll_block();
         if (should_service_stop()) {
-            exiting = true;
+            exit_args.exiting = true;
         }
     }
-    bridge_exit(cleanup);
+    bridge_exit(exit_args.cleanup);
+
+    for (size_t i = 0; i < exit_args.n_conns; i++) {
+        unixctl_command_reply(exit_args.conns[i], NULL);
+    }
+    free(exit_args.conns);
+
     unixctl_server_destroy(unixctl);
     service_stop();
     vlog_disable_async();
@@ -295,10 +299,14 @@ usage(void)
 
 static void
 ovs_vswitchd_exit(struct unixctl_conn *conn, int argc,
-                  const char *argv[], void *exit_args_)
+                  const char *argv[], void *args OVS_UNUSED)
 {
-    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);
+    exit_args.n_conns++;
+    exit_args.conns = xrealloc(exit_args.conns,
+                               exit_args.n_conns * sizeof *exit_args.conns);
+    exit_args.conns[exit_args.n_conns - 1] = conn;
+    exit_args.exiting = true;
+    if (!exit_args.cleanup) {
+        exit_args.cleanup = argc == 2 && !strcmp(argv[1], "--cleanup");
+    }
 }