diff mbox

[ovs-dev] ovn-controller: honor ovs_idl_txn when calculating and installing flows.

Message ID 1475718903-23155-1-git-send-email-rmoats@us.ibm.com
State Accepted
Headers show

Commit Message

Ryan Moats Oct. 6, 2016, 1:55 a.m. UTC 
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 <rmoats@us.ibm.com>
---
Note: This patch also applies to the 2.6 branch

 ovn/controller/ovn-controller.c | 45 +++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

Comments

Ben Pfaff Oct. 6, 2016, 8:36 p.m. UTC | #1 
On Wed, Oct 05, 2016 at 08:55:03PM -0500, Ryan Moats wrote:
> 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 <rmoats@us.ibm.com>
> ---
> Note: This patch also applies to the 2.6 branch

Applied to master and branch-2.6, thanks!
diff mbox

Patch

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 00392ca..4ac1425 100644
--- 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);