in-band: Delete remaining rules when disabling in-band control.

in_band_destroy() doesn't remove all of the rules that in-band control
adds (and it cannot, because that might require waiting for an existing
asynchronous flow modification or addition to complete), so turning on
other-config:disable-in-band or deleting all of the OpenFlow controllers
did not delete all of the in-band rules.

This commit fixes the problem by making the in-band control object hang
around until all of the flows that it set up have actually been deleted.

This problem was introduced as part of commit 7ee20df "ofproto: Implement
asynchronous OFPT_FLOW_MOD commands."

Reported-by: Brad Hall <brad@nicira.com>

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 865fa29..38052ac 100644 (file)
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -240,7 +240,10 @@ connmgr_run(struct connmgr *mgr,
     size_t i;
 
     if (handle_openflow && mgr->in_band) {
-        in_band_run(mgr->in_band);
+        if (!in_band_run(mgr->in_band)) {
+            in_band_destroy(mgr->in_band);
+            mgr->in_band = NULL;
+        }
     }
 
     LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) {
@@ -604,13 +607,13 @@ update_in_band_remotes(struct connmgr *mgr)
         if (!mgr->in_band) {
             in_band_create(mgr->ofproto, mgr->local_port_name, &mgr->in_band);
         }
-        if (mgr->in_band) {
-            in_band_set_remotes(mgr->in_band, addrs, n_addrs);
-        }
         in_band_set_queue(mgr->in_band, mgr->in_band_queue);
     } else {
-        in_band_destroy(mgr->in_band);
-        mgr->in_band = NULL;
+        /* in_band_run() needs a chance to delete any existing in-band flows.
+         * We will destroy mgr->in_band after it's done with that. */
+    }
+    if (mgr->in_band) {
+        in_band_set_remotes(mgr->in_band, addrs, n_addrs);
     }
 
     /* Clean up. */

diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index ae1f1b1..764b252 100644 (file)
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -310,7 +310,7 @@ update_rules(struct in_band *ib)
         ib_rule->op = DELETE;
     }
 
-    if (!eth_addr_is_zero(ib->local_mac)) {
+    if (ib->n_remotes && !eth_addr_is_zero(ib->local_mac)) {
         /* (a) Allow DHCP requests sent from the local port. */
         cls_rule_init_catchall(&rule, IBR_FROM_LOCAL_DHCP);
         cls_rule_set_in_port(&rule, ODPP_LOCAL);
@@ -395,7 +395,12 @@ update_rules(struct in_band *ib)
     }
 }
 
-void
+/* Updates the OpenFlow flow table for the current state of in-band control.
+ * Returns true ordinarily.  Returns false if no remotes are configured on 'ib'
+ * and 'ib' doesn't have any rules left to remove from the OpenFlow flow
+ * table.  Thus, a false return value means that the caller can destroy 'ib'
+ * without leaving extra flows hanging around in the flow table. */
+bool
 in_band_run(struct in_band *ib)
 {
     struct {
@@ -446,6 +451,8 @@ in_band_run(struct in_band *ib)
             break;
         }
     }
+
+    return ib->n_remotes || !hmap_is_empty(&ib->rules);
 }
 
 void

diff --git a/ofproto/in-band.h b/ofproto/in-band.h
index e2d8e80..f7f2ec6 100644 (file)
--- a/ofproto/in-band.h
+++ b/ofproto/in-band.h
@@ -35,7 +35,7 @@ void in_band_set_queue(struct in_band *, int queue_id);
 void in_band_set_remotes(struct in_band *,
                          const struct sockaddr_in *, size_t n);
 
-void in_band_run(struct in_band *);
+bool in_band_run(struct in_band *);
 void in_band_wait(struct in_band *);
 
 bool in_band_msg_in_hook(struct in_band *, const struct flow *,