[ovs-dev] [PATCH] connmgr: fix flow-restore-wait not work with controller connects

[wenxu at ucloud.cn]

From: wenxu <wenxu at ucloud.cn>

When restart the vswitchd with flow-restore-wait. The Vswitch doesn't
connect to the controller util the flow-restore-wait finished.

Because when bridge_configure_remotes() calls bridge_get_controllers(),
it first checks if flow-restore-wait has been set, and if so,
it ignores any controllers in the controller database and
sets n_controllers to 0.

So after the flows restore in ovs and remove the flow-restore-wait.
The vswitchd will connect the controller. But it will flush all
the flows we restored.

In the connmgr_set_controllers if (had_controllers != connmgr_has_controllers(mgr))
it will flush all the flows through ofproto_flush_flows(mgr->ofproto);
This make flow-restore-wait feature not work at all with controller connects.

This patch record the flow-restore-wait event and it will avoid the flow
flush if it experience a flow-restore-wait event..

Signed-off-by: wenxu <wenxu at ucloud.cn>
---
 ofproto/connmgr.c | 16 ++++++++++++++--
 ofproto/connmgr.h |  3 ++-
 ofproto/ofproto.c |  5 +++--
 ofproto/ofproto.h |  3 ++-
 vswitchd/bridge.c |  9 ++++++---
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 51d656c..4debc9f 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -231,6 +231,8 @@ struct connmgr {
     size_t n_extra_remotes;
     int in_band_queue;
 
+    bool flow_restore;
+
     ATOMIC(int) want_packet_in_on_miss;   /* Sum of ofconns' values. */
 };
 
@@ -571,7 +573,8 @@ connmgr_free_controller_info(struct shash *info)
 /* Changes 'mgr''s set of controllers to the 'n_controllers' controllers in
  * 'controllers'. */
 void
-connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers)
+connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers,
+                        bool flow_restore)
     OVS_EXCLUDED(ofproto_mutex)
 {
     bool had_controllers = connmgr_has_controllers(mgr);
@@ -611,8 +614,17 @@ connmgr_set_controllers(struct connmgr *mgr, struct shash *controllers)
     update_in_band_remotes(mgr);
     update_fail_open(mgr);
     if (had_controllers != connmgr_has_controllers(mgr)) {
-        ofproto_flush_flows(mgr->ofproto);
+        if (had_controllers == false &&
+            mgr->flow_restore == true &&
+            flow_restore == false) {
+            goto out;
+        } else {
+            ofproto_flush_flows(mgr->ofproto);
+        }
     }
+
+out:
+    mgr->flow_restore = flow_restore;
 }
 
 /* Drops the connections between 'mgr' and all of its primary and secondary
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 079c843..e164ea1 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -74,7 +74,8 @@ void connmgr_retry(struct connmgr *);
 bool connmgr_has_controllers(const struct connmgr *);
 void connmgr_get_controller_info(struct connmgr *, struct shash *);
 void connmgr_free_controller_info(struct shash *);
-void connmgr_set_controllers(struct connmgr *, struct shash *);
+void connmgr_set_controllers(struct connmgr *, struct shash *,
+                             bool flow_restore);
 void connmgr_reconnect(const struct connmgr *);
 
 int connmgr_set_snoops(struct connmgr *, const struct sset *snoops);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0fbd6c3..f20d86a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -643,9 +643,10 @@ ofproto_set_datapath_id(struct ofproto *p, uint64_t datapath_id)
 }
 
 void
-ofproto_set_controllers(struct ofproto *p, struct shash *controllers)
+ofproto_set_controllers(struct ofproto *p, struct shash *controllers,
+                        bool flow_restore)
 {
-    connmgr_set_controllers(p->connmgr, controllers);
+    connmgr_set_controllers(p->connmgr, controllers, flow_restore);
 }
 
 void
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 2dd2531..be7097a 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -329,7 +329,8 @@ int ofproto_port_query_by_name(const struct ofproto *, const char *devname,
 /* Top-level configuration. */
 uint64_t ofproto_get_datapath_id(const struct ofproto *);
 void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
-void ofproto_set_controllers(struct ofproto *, struct shash *controllers);
+void ofproto_set_controllers(struct ofproto *, struct shash *controllers,
+                             bool flow_restore);
 void ofproto_set_fail_mode(struct ofproto *, enum ofproto_fail_mode fail_mode);
 void ofproto_reconnect_controllers(struct ofproto *);
 void ofproto_set_extra_in_band_remotes(struct ofproto *,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index fe73c38..d87de45 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3867,6 +3867,8 @@ bridge_configure_remotes(struct bridge *br,
 
     enum ofproto_fail_mode fail_mode;
 
+    bool flow_restore;
+
     /* Check if we should disable in-band control on this bridge. */
     disable_in_band = smap_get_bool(&br->cfg->other_config, "disable-in-band",
                                     false);
@@ -3882,8 +3884,9 @@ bridge_configure_remotes(struct bridge *br,
         ofproto_set_extra_in_band_remotes(br->ofproto, managers, n_managers);
     }
 
-    n_controllers = (ofproto_get_flow_restore_wait() ? 0
-                     : bridge_get_controllers(br, &controllers));
+    flow_restore = ofproto_get_flow_restore_wait();
+    n_controllers = flow_restore ? 0
+                    : bridge_get_controllers(br, &controllers);
 
     /* The set of controllers to pass down to ofproto. */
     struct shash ocs = SHASH_INITIALIZER(&ocs);
@@ -3981,7 +3984,7 @@ bridge_configure_remotes(struct bridge *br,
         };
         shash_add(&ocs, c->target, oc);
     }
-    ofproto_set_controllers(br->ofproto, &ocs);
+    ofproto_set_controllers(br->ofproto, &ocs, flow_restore);
     shash_destroy_free_data(&ocs);
 
     /* Set the fail-mode. */
-- 
1.8.3.1