bridge: Move logic for flushing flows and standalone mode into connmgr.

This improves the abstraction behind ofproto and connmgr.

Some of this could even go into fail_open, but I'm not sure that it would
make anything easier to understand.

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 83b3159..b27fe94 100644 (file)
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -390,6 +390,7 @@ connmgr_set_controllers(struct connmgr *mgr,
                         const struct ofproto_controller *controllers,
                         size_t n_controllers)
 {
+    bool had_controllers = connmgr_has_controllers(mgr);
     struct shash new_controllers;
     struct ofconn *ofconn, *next_ofconn;
     struct ofservice *ofservice, *next_ofservice;
@@ -451,6 +452,9 @@ connmgr_set_controllers(struct connmgr *mgr,
 
     update_in_band_remotes(mgr);
     update_fail_open(mgr);
+    if (had_controllers != connmgr_has_controllers(mgr)) {
+        ofproto_flush_flows(mgr->ofproto);
+    }
 }
 
 /* Drops the connections between 'mgr' and all of its primary and secondary
@@ -1081,8 +1085,13 @@ connmgr_get_fail_mode(const struct connmgr *mgr)
 void
 connmgr_set_fail_mode(struct connmgr *mgr, enum ofproto_fail_mode fail_mode)
 {
-    mgr->fail_mode = fail_mode;
-    update_fail_open(mgr);
+    if (mgr->fail_mode != fail_mode) {
+        mgr->fail_mode = fail_mode;
+        update_fail_open(mgr);
+        if (!connmgr_has_controllers(mgr)) {
+            ofproto_flush_flows(mgr->ofproto);
+        }
+    }
 }
 \f
 /* Fail-open implementation. */
@@ -1265,6 +1274,23 @@ connmgr_flushed(struct connmgr *mgr)
     if (mgr->fail_open) {
         fail_open_flushed(mgr->fail_open);
     }
+
+    /* If there are no controllers and we're in standalone mode, set up a flow
+     * that matches every packet and directs them to OFPP_NORMAL (which goes to
+     * us).  Otherwise, the switch is in secure mode and we won't pass any
+     * traffic until a controller has been defined and it tells us to do so. */
+    if (!connmgr_has_controllers(mgr)
+        && mgr->fail_mode == OFPROTO_FAIL_STANDALONE) {
+        union ofp_action action;
+        struct cls_rule rule;
+
+        memset(&action, 0, sizeof action);
+        action.type = htons(OFPAT_OUTPUT);
+        action.output.len = htons(sizeof action);
+        action.output.port = htons(OFPP_NORMAL);
+        cls_rule_init_catchall(&rule, 0);
+        ofproto_add_flow(mgr->ofproto, &rule, &action, 1);
+    }
 }
 \f
 /* Creates a new ofservice for 'target' in 'mgr'.  Returns 0 if successful,

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index fb63606..6745a49 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -657,12 +657,6 @@ ofproto_get_datapath_id(const struct ofproto *ofproto)
     return ofproto->datapath_id;
 }
 
-bool
-ofproto_has_primary_controller(const struct ofproto *ofproto)
-{
-    return connmgr_has_controllers(ofproto->connmgr);
-}
-
 enum ofproto_fail_mode
 ofproto_get_fail_mode(const struct ofproto *p)
 {

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 9a56bee..d999b8d 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -134,7 +134,6 @@ const struct cfm *ofproto_iface_get_cfm(struct ofproto *, uint32_t port_no);
 
 /* Configuration querying. */
 uint64_t ofproto_get_datapath_id(const struct ofproto *);
-bool ofproto_has_primary_controller(const struct ofproto *);
 enum ofproto_fail_mode ofproto_get_fail_mode(const struct ofproto *);
 void ofproto_get_listeners(const struct ofproto *, struct sset *);
 bool ofproto_has_snoops(const struct ofproto *);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 20ecca3..eb10cf0 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1923,16 +1923,8 @@ bridge_reconfigure_one(struct bridge *br)
                 || !strcmp(br->cfg->fail_mode, "standalone")
                     ? OFPROTO_FAIL_STANDALONE
                     : OFPROTO_FAIL_SECURE;
-    if (ofproto_get_fail_mode(br->ofproto) != fail_mode
-        && !ofproto_has_primary_controller(br->ofproto)) {
-        ofproto_flush_flows(br->ofproto);
-    }
     ofproto_set_fail_mode(br->ofproto, fail_mode);
 
-    /* Delete all flows if we're switching from connected to standalone or vice
-     * versa.  (XXX Should we delete all flows if we are switching from one
-     * controller to another?) */
-
     /* Configure OpenFlow controller connection snooping. */
     if (!ofproto_has_snoops(br->ofproto)) {
         struct sset snoops;
@@ -2033,7 +2025,6 @@ bridge_reconfigure_remotes(struct bridge *br,
 
     struct ovsrec_controller **controllers;
     size_t n_controllers;
-    bool had_primary;
 
     struct ofproto_controller *ocs;
     size_t n_ocs;
@@ -2055,7 +2046,6 @@ bridge_reconfigure_remotes(struct bridge *br,
     } else {
         ofproto_set_extra_in_band_remotes(br->ofproto, managers, n_managers);
     }
-    had_primary = ofproto_has_primary_controller(br->ofproto);
 
     n_controllers = bridge_get_controllers(br, &controllers);
 
@@ -2089,28 +2079,6 @@ bridge_reconfigure_remotes(struct bridge *br,
     ofproto_set_controllers(br->ofproto, ocs, n_ocs);
     free(ocs[0].target); /* From bridge_ofproto_controller_for_mgmt(). */
     free(ocs);
-
-    if (had_primary != ofproto_has_primary_controller(br->ofproto)) {
-        ofproto_flush_flows(br->ofproto);
-    }
-
-    /* If there are no controllers and the bridge is in standalone
-     * mode, set up a flow that matches every packet and directs
-     * them to OFPP_NORMAL (which goes to us).  Otherwise, the
-     * switch is in secure mode and we won't pass any traffic until
-     * a controller has been defined and it tells us to do so. */
-    if (!n_controllers
-        && ofproto_get_fail_mode(br->ofproto) == OFPROTO_FAIL_STANDALONE) {
-        union ofp_action action;
-        struct cls_rule rule;
-
-        memset(&action, 0, sizeof action);
-        action.type = htons(OFPAT_OUTPUT);
-        action.output.len = htons(sizeof action);
-        action.output.port = htons(OFPP_NORMAL);
-        cls_rule_init_catchall(&rule, 0);
-        ofproto_add_flow(br->ofproto, &rule, &action, 1);
-    }
 }
 
 static void