ovn-controller: honor ovs_idl_txn when calculating and installing flows.

ovs_idl_txn is checked before various routines (like patch_run) execute. However, flow calculation and installation does not also check this variable, which can lead to oscillations as described in [1]. [1] http://openvswitch.org/pipermail/dev/2016-October/080247.html Signed-off-by: Ryan Moats <[email protected]> Signed-off-by: Ben Pfaff <[email protected]>
water0615 · Oct 6, 2016 · 13c0048 · 13c0048
1 parent fd3b31e
commit 13c0048
Showing 1 changed file with 21 additions and 24 deletions.
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
@@ -312,14 +312,9 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths,
 }
 
 static void
-commit_ct_zones(struct controller_ctx *ctx,
-                const struct ovsrec_bridge *br_int,
+commit_ct_zones(const struct ovsrec_bridge *br_int,
                 struct shash *pending_ct_zones)
 {
-    if (!ctx->ovs_idl_txn) {
-        return;
-    }
-
     struct smap new_ids;
     smap_clone(&new_ids, &br_int->external_ids);
 
@@ -543,24 +538,26 @@ main(int argc, char *argv[])
             pinctrl_run(&ctx, &lports, br_int, chassis_id, &local_datapaths);
             update_ct_zones(&all_lports, &patched_datapaths, &ct_zones,
                             ct_zone_bitmap, &pending_ct_zones);
-            commit_ct_zones(&ctx, br_int, &pending_ct_zones);
-
-            struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
-            lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
-                      &patched_datapaths, &group_table, &ct_zones,
-                      &flow_table);
-
-            physical_run(&ctx, mff_ovn_geneve,
-                         br_int, chassis_id, &ct_zones, &flow_table,
-                         &local_datapaths, &patched_datapaths);
-
-            ofctrl_put(&flow_table, &pending_ct_zones,
-                       get_nb_cfg(ctx.ovnsb_idl));
-            hmap_destroy(&flow_table);
-            if (ctx.ovnsb_idl_txn) {
-                int64_t cur_cfg = ofctrl_get_cur_cfg();
-                if (cur_cfg && cur_cfg != chassis->nb_cfg) {
-                    sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+            if (ctx.ovs_idl_txn) {
+                commit_ct_zones(br_int, &pending_ct_zones);
+
+                struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
+                lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
+                          &patched_datapaths, &group_table, &ct_zones,
+                          &flow_table);
+
+                physical_run(&ctx, mff_ovn_geneve,
+                             br_int, chassis_id, &ct_zones, &flow_table,
+                             &local_datapaths, &patched_datapaths);
+
+                ofctrl_put(&flow_table, &pending_ct_zones,
+                           get_nb_cfg(ctx.ovnsb_idl));
+                hmap_destroy(&flow_table);
+                if (ctx.ovnsb_idl_txn) {
+                    int64_t cur_cfg = ofctrl_get_cur_cfg();
+                    if (cur_cfg && cur_cfg != chassis->nb_cfg) {
+                        sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+                    }
                 }
             }
             mcgroup_index_destroy(&mcgroups);