From d0bc543fa58e322dfabc9ceb95f4b4adb3743453 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Wed, 29 Apr 2009 15:42:16 -0700
Subject: [PATCH] secchan: Factor common code into rule_remove().

Several pieces of code were calling rule_uninstall(), classifier_remove(),
then rule_destroy() in sequence.  Factor this out into a helper function.
---
 secchan/ofproto.c | 76 +++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index 28e19ea0..5b7e169f 100644
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -165,6 +165,7 @@ static void rule_destroy(struct ofproto *, struct rule *);
 static struct rule *rule_from_cls_rule(const struct cls_rule *);
 static void rule_insert(struct ofproto *, struct rule *,
                         struct ofpbuf *packet);
+static void rule_remove(struct ofproto *, struct rule *);
 static bool rule_make_actions(struct ofproto *, struct rule *,
                               const struct ofpbuf *packet);
 static void rule_install(struct ofproto *, struct rule *,
@@ -975,11 +976,7 @@ ofproto_delete_flow(struct ofproto *ofproto, const flow_t *flow,
                                                            flow, wildcards,
                                                            priority));
     if (rule) {
-        if (rule->cr.wc.wildcards) {
-            ofproto->need_revalidate = true;
-        }
-        classifier_remove(&ofproto->cls, &rule->cr);
-        rule_destroy(ofproto, rule);
+        rule_remove(ofproto, rule);
     }
 }
 
@@ -989,8 +986,13 @@ destroy_rule(struct cls_rule *rule_, void *ofproto_)
     struct rule *rule = rule_from_cls_rule(rule_);
     struct ofproto *ofproto = ofproto_;
 
-    classifier_remove(&ofproto->cls, &rule->cr);
-    rule_free(rule);
+    /* Mark the flow as not installed, even though it might really be
+     * installed, so that rule_remove() doesn't bother trying to uninstall it.
+     * There is no point in uninstalling it individually since we are about to
+     * blow away all the flows with dpif_flow_flush(). */
+    rule->installed = false;
+
+    rule_remove(ofproto, rule);
 }
 
 void
@@ -1428,6 +1430,18 @@ rule_insert(struct ofproto *p, struct rule *rule, struct ofpbuf *packet)
     }
 }
 
+static void
+rule_remove(struct ofproto *ofproto, struct rule *rule)
+{
+    if (rule->cr.wc.wildcards) {
+        ofproto->need_revalidate = true;
+    } else {
+        rule_uninstall(ofproto, rule);
+    }
+    classifier_remove(&ofproto->cls, &rule->cr);
+    rule_destroy(ofproto, rule);
+}
+
 /* Returns true if the actions changed, false otherwise. */
 static bool
 rule_make_actions(struct ofproto *p, struct rule *rule,
@@ -2480,21 +2494,17 @@ modify_flow(struct ofproto *p, const struct ofp_flow_mod *ofm,
         return 0;
     }
 
-    if (rule->cr.wc.wildcards) {
-        p->need_revalidate = true;
-    }
     if (command == OFPFC_DELETE) {
-        if (!rule->cr.wc.wildcards) {
-            rule_uninstall(p, rule);
-        }
-        classifier_remove(&p->cls, &rule->cr);
-        rule_destroy(p, rule);
+        rule_remove(p, rule);
     } else {
         free(rule->actions);
         rule->actions = xmemdup(ofm->actions,
                                 n_actions * sizeof *rule->actions);
         rule->n_actions = n_actions;
-        if (!rule->cr.wc.wildcards) {
+
+        if (rule->cr.wc.wildcards) {
+            p->need_revalidate = true;
+        } else {
             if (rule_make_actions(p, rule, NULL)) {
                 rule_update(p, rule);
             } else if (rule->installed && !rule->may_install) {
@@ -2891,9 +2901,7 @@ revalidate_rule(struct ofproto *p, struct rule *rule)
         struct rule *super;
         super = rule_from_cls_rule(classifier_lookup_wild(&p->cls, flow));
         if (!super) {
-            rule_uninstall(p, rule);
-            classifier_remove(&p->cls, &rule->cr);
-            rule_destroy(p, rule);
+            rule_remove(p, rule);
             return false;
         } else if (super != rule->super) {
             list_remove(&rule->list);
@@ -2961,10 +2969,10 @@ uninstall_idle_flow(struct ofproto *ofproto, struct rule *rule)
     assert(rule->installed);
     assert(!rule->cr.wc.wildcards);
 
-    rule_uninstall(ofproto, rule);
     if (rule->super) {
-        classifier_remove(&ofproto->cls, &rule->cr);
-        rule_destroy(ofproto, rule);
+        rule_remove(ofproto, rule);
+    } else {
+        rule_uninstall(ofproto, rule);
     }
 }
 
@@ -2998,28 +3006,20 @@ expire_rule(struct cls_rule *cls_rule, void *p_)
         return;
     }
 
-    if (!rule->super) {
-        if (rule->cr.wc.wildcards) {
-            /* Update stats.  (This code will be a no-op if the rule expired
-             * due to an idle timeout, because in that case the rule has no
-             * subrules left.) */
-            struct rule *subrule, *next_subrule;
-            LIST_FOR_EACH_SAFE (subrule, next_subrule,
-                                struct rule, list, &rule->list) {
-                rule_uninstall(p, subrule);
-                classifier_remove(&p->cls, &subrule->cr);
-                rule_destroy(p, subrule);
-            }
-        } else {
-            rule_uninstall(p, rule);
+    if (rule->cr.wc.wildcards) {
+        /* Update stats.  (This code will be a no-op if the rule expired
+         * due to an idle timeout, because in that case the rule has no
+         * subrules left.) */
+        struct rule *subrule, *next;
+        LIST_FOR_EACH_SAFE (subrule, next, struct rule, list, &rule->list) {
+            rule_remove(p, subrule);
         }
     }
 
     send_flow_exp(p, rule, now,
                   (now >= hard_expire
                    ? OFPER_HARD_TIMEOUT : OFPER_IDLE_TIMEOUT));
-    classifier_remove(&p->cls, &rule->cr);
-    rule_destroy(p, rule);
+    rule_remove(p, rule);
 }
 
 static void
-- 
2.30.2