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
