From 6da1e8091eb2e19de7ba5e0c73ac3e7dd437743d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 3 Aug 2011 15:01:11 -0700 Subject: [PATCH] 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 --- ofproto/connmgr.c | 15 +++++++++------ ofproto/in-band.c | 11 +++++++++-- ofproto/in-band.h | 2 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 865fa295..38052ac5 100644 --- 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 ae1f1b13..764b2529 100644 --- 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 e2d8e80b..f7f2ec65 100644 --- 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 *, -- 2.30.2