[ovs-dev] [PATCH v4] bridge: Resend status changes to database if previous transaction was not successful.

[Ryan Wilson]

Bridge, port and interface status changes are not sent to the
database if the connectivity and netdev sequence numbers have not
changed. However, if the previous database transaction fails, then
status changes will not be updated in the database until the
connectivity and netdev sequence numbers change again. This could
leave the database in an incorrect state for a long period of time.

This patch always sends status changes to the database if the last
transaction was not successful.

Signed-off-by: Ryan Wilson <wryan at nicira.com>

---
v2: Addressed Alex's comments, edited commit message to be more
accurate
v3: Remove iface_refresh_netdev_status() from iface_create() upon
further discussion with Alex
v4: Changed force_status_update to force_status_commit since it now
is set to true when status == 'TXN_INCOMPLETE'. Status txn also
needs to be run every STATUS_CHECK_AGAIN_MSEC if last transaction
was not successful.
---
 vswitchd/bridge.c |   35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9764c1f..a266c85 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -172,9 +172,15 @@ static uint64_t connectivity_seqno = LLONG_MIN;
  * we check the return status of each update transaction and do not start new
  * update if the previous transaction status is 'TXN_INCOMPLETE'.
  *
- * 'statux_txn' is NULL if there is no ongoing status update.
+ * 'status_txn' is NULL if there is no ongoing status update.
+ *
+ * If the previous database transaction was incomplete or failed (is not
+ * 'TXN_SUCCESS' or 'TXN_UNCHANGED'), 'force_status_commit' is set to true.
+ * This means that 'status_txn' must be committed next iteration of bridge_run()
+ * even if the connectivity or netdev sequence numbers do not change.
  */
 static struct ovsdb_idl_txn *status_txn;
+bool force_status_commit = true;
 
 /* When the status update transaction returns 'TXN_INCOMPLETE', should register a
  * timeout in 'STATUS_CHECK_AGAIN_MSEC' to check again. */
@@ -1547,7 +1553,6 @@ iface_create(struct bridge *br, const struct ovsrec_interface *iface_cfg,
 
     /* Populate initial status in database. */
     iface_refresh_stats(iface);
-    iface_refresh_netdev_status(iface);
 
     /* Add bond fake iface if necessary. */
     if (port_is_bond_fake_iface(port)) {
@@ -1820,7 +1825,8 @@ iface_refresh_netdev_status(struct iface *iface)
         return;
     }
 
-    if (iface->change_seq == netdev_get_change_seq(iface->netdev)) {
+    if (iface->change_seq == netdev_get_change_seq(iface->netdev)
+        && !force_status_commit) {
         return;
     }
 
@@ -2420,7 +2426,7 @@ bridge_run(void)
 
         /* Check the need to update status. */
         seq = seq_read(connectivity_seq_get());
-        if (seq != connectivity_seqno) {
+        if (seq != connectivity_seqno || force_status_commit) {
             connectivity_seqno = seq;
             status_txn = ovsdb_idl_txn_create(idl);
             HMAP_FOR_EACH (br, node, &all_bridges) {
@@ -2444,6 +2450,17 @@ bridge_run(void)
         enum ovsdb_idl_txn_status status;
 
         status = ovsdb_idl_txn_commit(status_txn);
+
+        /* If the transaction is incomplete or fails, 'status_txn'
+         * needs to be committed next iteration of bridge_run() even if
+         * connectivity or netdev sequence numbers do not change. */
+        if (status == TXN_SUCCESS || status == TXN_UNCHANGED)
+        {
+            force_status_commit = false;
+        } else {
+            force_status_commit = true;
+        }
+
         /* Do not destroy "status_txn" if the transaction is
          * "TXN_INCOMPLETE". */
         if (status != TXN_INCOMPLETE) {
@@ -2483,11 +2500,11 @@ bridge_wait(void)
         poll_timer_wait_until(stats_timer);
     }
 
-    /* If the status database transaction is 'TXN_INCOMPLETE' in this run,
-     * register a timeout in 'STATUS_CHECK_AGAIN_MSEC'.  Else, wait on the
-     * global connectivity sequence number.  Note, this also helps batch
-     * multiple status changes into one transaction. */
-    if (status_txn) {
+    /* If the status database transaction is 'TXN_INCOMPLETE' or is
+     * unsuccessful, register a timeout in 'STATUS_CHECK_AGAIN_MSEC'.  Else,
+     * wait on the global connectivity sequence number.  Note, this also helps
+     * batch multiple status changes into one transaction. */
+    if (force_status_commit) {
         poll_timer_wait_until(time_msec() + STATUS_CHECK_AGAIN_MSEC);
     } else {
         seq_wait(connectivity_seq_get(), connectivity_seqno);
-- 
1.7.9.5