bridge: Only complete daemonization after db commits initial config.

An earlier commit changed the Open vSwitch startup scripts so that they
connect to remote managers only after ovs-vswitchd does its initial
configuration, as signaled by ovs-vswitchd detaching from its parent
process.  However, a race window remains, because ovs-vswitchd detaching
does not mean that the database server has received and committed the
transaction, only that ovs-vswitchd has sent it.  This commit fixes that
race window, by changing ovs-vswitchd to complete detaching only after
the database server acknowledges the transaction.

It is still possible for unusual events to cause ovs-vswitchd to detach
before ephemeral columns are filled in.  There is always a slim possibility
that the transaction will fail or that some other client has added new
bridges, ports, etc. while ovs-vswitchd was configuring using an old
configuration.  The latter race is inherent to the design of the system
and cannot be avoided without radical changes.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ansis Atteka <aatteka@nicira.com>
Bug #15983.

1 files changed, 44 insertions, 9 deletions

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 6dc3db228..1fcf547b5 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -146,6 +146,23 @@ static struct hmap all_bridges = HMAP_INITIALIZER(&all_bridges);
 /* OVSDB IDL used to obtain configuration. */
 static struct ovsdb_idl *idl;
 
+/* We want to complete daemonization, fully detaching from our parent process,
+ * only after we have completed our initial configuration, committed our state
+ * to the database, and received confirmation back from the database server
+ * that it applied the commit.  This allows our parent process to know that,
+ * post-detach, ephemeral fields such as datapath-id and ofport are very likely
+ * to have already been filled in.  (It is only "very likely" rather than
+ * certain because there is always a slim possibility that the transaction will
+ * fail or that some other client has added new bridges, ports, etc. while
+ * ovs-vswitchd was configuring using an old configuration.)
+ *
+ * We only need to do this once for our initial configuration at startup, so
+ * 'initial_config_done' tracks whether we've already done it.  While we are
+ * waiting for a response to our commit, 'daemonize_txn' tracks the transaction
+ * itself and is otherwise NULL. */
+static bool initial_config_done;
+static struct ovsdb_idl_txn *daemonize_txn;
+
 /* Most recently processed IDL sequence number. */
 static unsigned int idl_seqno;
 
@@ -601,15 +618,6 @@ bridge_reconfigure_continue(const struct ovsrec_open_vswitch *ovs_cfg)
     }
     free(managers);
 
-    if (done) {
-        /* ovs-vswitchd has completed initialization, so allow the process that
-         * forked us to exit successfully. */
-        daemonize_complete();
-        reconfiguring = false;
-
-        VLOG_INFO_ONCE("%s (Open vSwitch) %s", program_name, VERSION);
-    }
-
     return done;
 }
 
@@ -2267,6 +2275,16 @@ bridge_run(void)
             }
             if (bridge_reconfigure_continue(cfg)) {
                 ovsrec_open_vswitch_set_cur_cfg(cfg, cfg->next_cfg);
+                reconfiguring = false;
+
+                /* If we are completing our initial configuration for this run
+                 * of ovs-vswitchd, then keep the transaction around to monitor
+                 * it for completion. */
+                if (!initial_config_done) {
+                    initial_config_done = true;
+                    daemonize_txn = reconf_txn;
+                    reconf_txn = NULL;
+                }
             }
         } else {
             bridge_reconfigure_continue(&null_cfg);
@@ -2279,6 +2297,20 @@ bridge_run(void)
         reconf_txn = NULL;
     }
 
+    if (daemonize_txn) {
+        enum ovsdb_idl_txn_status status = ovsdb_idl_txn_commit(daemonize_txn);
+        if (status != TXN_INCOMPLETE) {
+            ovsdb_idl_txn_destroy(daemonize_txn);
+            daemonize_txn = NULL;
+
+            /* ovs-vswitchd has completed initialization, so allow the
+             * process that forked us to exit successfully. */
+            daemonize_complete();
+
+            VLOG_INFO_ONCE("%s (Open vSwitch) %s", program_name, VERSION);
+        }
+    }
+
     /* Refresh interface and mirror stats if necessary. */
     if (time_msec() >= iface_stats_timer) {
         if (cfg) {
@@ -2322,6 +2354,9 @@ bridge_wait(void)
     const char *type;
 
     ovsdb_idl_wait(idl);
+    if (daemonize_txn) {
+        ovsdb_idl_txn_wait(daemonize_txn);
+    }
 
     if (reconfiguring) {
         poll_immediate_wake();