The series looks good to me. Ethan
On Wed, Aug 3, 2011 at 15:01, Ben Pfaff <[email protected]> wrote: > 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 <[email protected]> > --- > 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 865fa29..38052ac 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 ae1f1b1..764b252 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 e2d8e80..f7f2ec6 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 *, > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
