[ovs-dev] [dev master 3/3] bridge: Re-try iface status update if the previous transaction fails.

[Alex Wang]

This commit adds logic that marks the 'struct iface's when their
status need to be updated but the current status update transaction
fails.  So, the main thread can re-try the update in the next run.

Bug #1256577

Signed-off-by: Alex Wang <alexw at nicira.com>
---
 vswitchd/bridge.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 835de0f..1682992 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -77,11 +77,18 @@ struct iface {
     char *name;                 /* Host network device name. */
     struct netdev *netdev;      /* Network device. */
     ofp_port_t ofp_port;        /* OpenFlow port number. */
-    uint64_t change_seq;
+    uint64_t netdev_change_seq;
 
     /* These members are valid only within bridge_reconfigure(). */
     const char *type;           /* Usually same as cfg->type. */
     const struct ovsrec_interface *cfg;
+
+    /* 'status_update_try_again' is set when iface's status needs to be
+     * be updated to database but the update transaction fails.  It is
+     * marked so that the next run will try updating it again. */
+    struct list try_again_node; /* Element in "may_try_again_ifaces" list. */
+    bool try_again_inserted;    /* Guarantee iface is inserted only once. */
+    bool status_update_try_again;
 };
 
 struct mirror {
@@ -190,6 +197,12 @@ static bool status_txn_try_again;
 static int stats_timer_interval;
 static long long int stats_timer = LLONG_MIN;
 
+/* List of 'struct iface's that may be checked again for status
+ * update, if the update transaction fails.  The list should be
+ * cleared at the end of bridge_run(). */
+static struct list may_try_again_ifaces =
+    LIST_INITIALIZER(&may_try_again_ifaces);
+
 /* In some datapaths, creating and destroying OpenFlow ports can be extremely
  * expensive.  This can cause bridge_reconfigure() to take a long time during
  * which no other work can be done.  To deal with this problem, we limit port
@@ -290,6 +303,7 @@ static bool iface_is_synthetic(const struct iface *);
 static ofp_port_t iface_get_requested_ofp_port(
     const struct ovsrec_interface *);
 static ofp_port_t iface_pick_ofport(const struct ovsrec_interface *);
+static void iface_register_status_update_try_again(struct iface *);
 
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
  *
@@ -1547,6 +1561,8 @@ iface_create(struct bridge *br, const struct ovsrec_interface *iface_cfg,
     iface->netdev = netdev;
     iface->type = iface_get_type(iface_cfg, br->cfg);
     iface->cfg = iface_cfg;
+    /* Sets the try-again flag to force re-check of initial status. */
+    iface->status_update_try_again = true;
     hmap_insert(&br->ifaces, &iface->ofp_port_node,
                 hash_ofp_port(ofp_port));
 
@@ -1825,11 +1841,13 @@ iface_refresh_netdev_status(struct iface *iface)
         return;
     }
 
-    if (iface->change_seq == netdev_get_change_seq(iface->netdev)) {
+    if (iface->netdev_change_seq == netdev_get_change_seq(iface->netdev)
+        && !iface->status_update_try_again) {
         return;
     }
 
-    iface->change_seq = netdev_get_change_seq(iface->netdev);
+    iface->netdev_change_seq = netdev_get_change_seq(iface->netdev);
+    iface_register_status_update_try_again(iface);
 
     smap_init(&smap);
 
@@ -1914,14 +1932,20 @@ iface_refresh_ofproto_status(struct iface *iface)
     }
 
     if (ofproto_port_cfm_status_changed(iface->port->bridge->ofproto,
-                                        iface->ofp_port)) {
+                                        iface->ofp_port)
+        || iface->status_update_try_again) {
+
+        iface_register_status_update_try_again(iface);
         iface_refresh_cfm_stats(iface);
     }
 
     if (ofproto_port_bfd_status_changed(iface->port->bridge->ofproto,
-                                        iface->ofp_port)) {
+                                        iface->ofp_port)
+        || iface->status_update_try_again) {
         struct smap smap;
 
+        iface_register_status_update_try_again(iface);
+
         smap_init(&smap);
         ofproto_port_get_bfd_status(iface->port->bridge->ofproto,
                                     iface->ofp_port, &smap);
@@ -2354,6 +2378,9 @@ bridge_run(void)
         if (initial_config_done) {
             ovsdb_idl_txn_commit(txn);
             ovsdb_idl_txn_destroy(txn);
+            /* Always sets the 'status_txn_try_again' to check again,
+             * in case that this transaction fails. */
+            status_txn_try_again = true;
         } else {
             initial_config_done = true;
             daemonize_txn = txn;
@@ -2450,6 +2477,7 @@ bridge_run(void)
 
     if (status_txn) {
         enum ovsdb_idl_txn_status status;
+        struct iface *iface, *next;
 
         status = ovsdb_idl_txn_commit(status_txn);
         /* Do not destroy "status_txn" if the transaction is
@@ -2466,6 +2494,16 @@ bridge_run(void)
         } else {
             status_txn_try_again = true;
         }
+
+        /* Unregisters all 'iface's from 'may_try_again_faces' list,
+         * and sets the 'iface->status_update_try_again' to
+         * 'status_txn_try_again'. */
+        LIST_FOR_EACH_SAFE (iface, next, try_again_node,
+                            &may_try_again_ifaces) {
+            iface->try_again_inserted = false;
+            iface->status_update_try_again = status_txn_try_again;
+            list_remove(&iface->try_again_node);
+        }
     }
 
     run_system_stats();
@@ -3800,6 +3838,15 @@ iface_pick_ofport(const struct ovsrec_interface *cfg)
             ? requested_ofport
             : iface_validate_ofport__(cfg->n_ofport, cfg->ofport));
 }
+
+static void
+iface_register_status_update_try_again(struct iface *iface)
+{
+    if (!iface->try_again_inserted) {
+        iface->try_again_inserted = true;
+        list_insert(&may_try_again_ifaces, &iface->try_again_node);
+    }
+}
 
 /* Port mirroring. */
 
-- 
1.7.9.5