Thank you, I pushed this to master.
On Wed, Aug 03, 2011 at 04:03:15PM -0700, Ethan Jackson wrote: > 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
