On Fri, Jan 25, 2013 at 05:22:49PM -0800, Justin Pettit wrote: > On Jan 25, 2013, at 3:21 PM, Ben Pfaff <[email protected]> wrote: > > > @@ -4858,6 +4858,7 @@ oftable_replace_rule(struct rule *rule) > > > > victim = rule_from_cls_rule(classifier_replace(&table->cls, &rule->cr)); > > if (victim) { > > + list_remove(&victim->expirable); > > In oftable_remove_rule(), there's a check whether the list is empty > before removing it. While it doesn't really matter, it might be > nice to keep them looking consistent.
Good point, I fixed that and applied this as follows. --8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> Date: Fri, 25 Jan 2013 17:26:42 -0800 Subject: [PATCH] ofproto: Maintain "expirable" list for "add" flow_mod with replaced rule. Commit e503cc199 (ofproto: Optimise OpenFlow flow expiry) optimized OpenFlow flow expiration by putting expirable flows on a list, but it failed to remove flows from the list when they were replaced by a new flow with an OpenFlow flow_mod "add" operation. This commit fixes the problem. Found by valgrind. CC: Simon Horman <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> --- ofproto/ofproto.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 249f2d5..c0d94f7 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4858,6 +4858,9 @@ oftable_replace_rule(struct rule *rule) victim = rule_from_cls_rule(classifier_replace(&table->cls, &rule->cr)); if (victim) { + if (!list_is_empty(&victim->expirable)) { + list_remove(&victim->expirable); + } eviction_group_remove_rule(victim); } eviction_group_add_rule(rule); -- 1.7.2.5 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
