Looks good. Thanks! --Justin
On Jan 6, 2012, at 3:04 PM, Ben Pfaff wrote: > If a subfacet expired when its facet still had statistics that had not > yet been pushed into the rule, and the facet either used the "normal" > action or the bridge contained a bond port, then facet_account() would > be called after the last subfacet was removed from its facet's list of > subfacets, triggering an assertion failure in list_front(). > > This fixes the problem by always running facet_flush_stats() (which calls > facet_account()) before deleting the last subfacet from a facet. > > This problem took a while to surface because subfacets usually expire only > long after their statistics have been pushed into the rule. > > Signed-off-by: Ben Pfaff <[email protected]> > Reported-by: Mike Kruze <[email protected]> > Bug #9074. > --- > AUTHORS | 1 + > ofproto/ofproto-dpif.c | 23 +++++++++++++++++++---- > 2 files changed, 20 insertions(+), 4 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index 821f780..631dfec 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -100,6 +100,7 @@ Michael A. Collins [email protected] > Michael Hu [email protected] > Michael Mao [email protected] > Mike Bursell [email protected] > +Mike Kruze [email protected] > Murphy McCauley [email protected] > Mikael Doverhag [email protected] > Niklas Andersson [email protected] > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index baa191e..fe99d70 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -3231,12 +3231,25 @@ facet_remove(struct ofproto_dpif *ofproto, struct > facet *facet) > { > struct subfacet *subfacet, *next_subfacet; > > + assert(!list_is_empty(&facet->subfacets)); > + > + /* First uninstall all of the subfacets to get final statistics. */ > + LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) { > + subfacet_uninstall(ofproto, subfacet); > + } > + > + /* Flush the final stats to the rule. > + * > + * This might require us to have at least one subfacet around so that we > + * can use its actions for accounting in facet_account(), which is why we > + * have uninstalled but not yet destroyed the subfacets. */ > + facet_flush_stats(ofproto, facet); > + > + /* Now we're really all done so destroy everything. */ > LIST_FOR_EACH_SAFE (subfacet, next_subfacet, list_node, > &facet->subfacets) { > subfacet_destroy__(ofproto, subfacet); > } > - > - facet_flush_stats(ofproto, facet); > hmap_remove(&ofproto->facets, &facet->hmap_node); > list_remove(&facet->list_node); > facet_free(facet); > @@ -3711,9 +3724,11 @@ subfacet_destroy(struct ofproto_dpif *ofproto, struct > subfacet *subfacet) > { > struct facet *facet = subfacet->facet; > > - subfacet_destroy__(ofproto, subfacet); > - if (list_is_empty(&facet->subfacets)) { > + if (list_is_singleton(&facet->subfacets)) { > + /* facet_remove() needs at least one subfacet (it will remove it). */ > facet_remove(ofproto, facet); > + } else { > + subfacet_destroy__(ofproto, subfacet); > } > } > > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
