From 63ff04e82623e76521add85287b2ac0e1829bcfe Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 10 Apr 2013 10:33:39 -0700 Subject: 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 Acked-by: Ansis Atteka Bug #15983. --- vswitchd/bridge.c | 53 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 9 deletions(-) (limited to 'vswitchd/bridge.c') 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(); -- cgit v1.2.1