diff options
author | Ben Pfaff <blp@nicira.com> | 2012-01-06 15:03:07 -0800 |
---|---|---|
committer | Ben Pfaff <blp@nicira.com> | 2012-01-06 17:02:13 -0800 |
commit | b87dd2dcf4a2af434d1b92f6efcb32f54fee03b2 (patch) | |
tree | a4a3cd23208761e64738a6a35de72faead2f9e78 /ofproto/ofproto-dpif.c | |
parent | 2716b6379dc86331330d78b496985e908320ddfa (diff) |
ofproto-dpif: Keep subfacets longer to avoid assert-fail in facet_account().
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 <blp@nicira.com>
Reported-by: Mike Kruze <mkruze@nicira.com>
Bug #9074.
Diffstat (limited to 'ofproto/ofproto-dpif.c')
-rw-r--r-- | ofproto/ofproto-dpif.c | 23 |
1 files changed, 19 insertions, 4 deletions
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index cca23cac..09f4724c 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3166,12 +3166,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); @@ -3646,9 +3659,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); } } |