On Aug 29, 2013, at 4:23 PM, Ben Pfaff <b...@nicira.com> wrote:
> Since the meter code was introduced, the oftable_remove_rule() function
> (recently renamed oftable_remove_rule__()) has had two blocks of code to
> remove a rule from its meter's list of rules that reference that meter.
> This commit reduces that to one.
>
> Also modifies oftable_remove_rule__() to maintain the invariant that
> 'rule' is in a meter's list if and only if
> !list_is_empty(&rule->meter_list_node). I don't believe that that fixes
> a real bug, but it seems like a good idea.
>
> (This seemed like a serious bug at first, but in fact list_remove() is
> idempotent: calling it two times in a row has the same effect as calling
> it once.)
I think that this duplication crept in once the rule's 'meter_id' member was
added.
Thanks for fixing it!
Jarno
Acked-by: Jarno Rajahalme <jrajaha...@nicira.com
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> CC: Jarno Rajahalme <jrajaha...@nicira.com>
> ---
> ofproto/ofproto.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 1173936..0b054e7 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -5573,9 +5573,6 @@ oftable_remove_rule__(struct ofproto *ofproto, struct
> classifier *cls,
> OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->evict)
> {
> classifier_remove(cls, &rule->cr);
> - if (rule->meter_id) {
> - list_remove(&rule->meter_list_node);
> - }
> cookies_remove(ofproto, rule);
> eviction_group_remove_rule(rule);
> ovs_mutex_lock(&ofproto->expirable_mutex);
> @@ -5585,6 +5582,7 @@ oftable_remove_rule__(struct ofproto *ofproto, struct
> classifier *cls,
> ovs_mutex_unlock(&ofproto->expirable_mutex);
> if (!list_is_empty(&rule->meter_list_node)) {
> list_remove(&rule->meter_list_node);
> + list_init(&rule->meter_list_node);
> }
> ovs_rwlock_unlock(&rule->evict);
> }
> --
> 1.7.10.4
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
