From: Ben Pfaff <blp@nicira.com>
Date: Sat, 7 Mar 2009 00:16:19 +0000 (-0800)
Subject: secchan: Remove an invalid optimization.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=117e116d39bba48bda994cf038b44b4cf69c7282;p=openvswitch

secchan: Remove an invalid optimization.
---

diff --git a/secchan/ofproto.c b/secchan/ofproto.c
index a95a2ef0..60d3c95d 100644
--- a/secchan/ofproto.c
+++ b/secchan/ofproto.c
@@ -2145,34 +2145,22 @@ add_flow(struct ofproto *p, struct ofconn *ofconn,
     if (rule->cr.wc.wildcards) {
         if (displaced_rule) {
             /* The displaced rule matches exactly the same packets as the new
-             * rule, and it has exactly the same priority, so we can transfer
-             * all displaced_rule's subrules to the new rule.  The subrule
-             * actions might have changed, so we have to update the datapath
-             * flows, which also has the convenient side effect of zeroing the
-             * counters for those flows. */
-            struct rule *subrule;
-
-            list_splice(&rule->list, displaced_rule->list.next,
-                        &displaced_rule->list);
-            LIST_FOR_EACH (subrule, struct rule, list, &rule->list) {
-                struct odp_actions actions;
-                struct odp_flow odp_flow;
-
-                subrule->super = rule;
-                xlate_actions(rule->actions, rule->n_actions,
-                              &subrule->cr.flow, p, &actions);
-                odp_flow.key = subrule->cr.flow;
-                odp_flow.actions = actions.actions;
-                odp_flow.n_actions = actions.n_actions;
-                dpif_flow_add(&p->dpif, &odp_flow);
-                free_actions(&actions);
-            }
+             * rule, and it has the same priority, so we could usually transfer
+             * all displaced_rule's subrules to the new rule, then update the
+             * subrules' flow in the datapath.  We used to do this.  But there
+             * is one case where this optimization doesn't work: when
+             * 'displaced_rule' is a rule selected by NXAST_RESUBMIT, because
+             * in such a case 'displaced_rule' will not be the super-rule of
+             * the subrule that got actions from 'displaced_rule'.  We could
+             * add more data to track such situations, but we have no evidence
+             * that the optimization would be worthwhile performance-wise
+             * anyhow.
+             */
             rule_destroy(displaced_rule);
-        } else {
-            /* No rule was exactly displaced, so we might need to change the
-             * rule that arbitrary subrules correspond to. */
-            p->need_revalidate = true;
         }
+
+        /* We might need to change the rules for arbitrary subrules. */
+        p->need_revalidate = true;
     } else {
         struct odp_flow odp_flow;
         struct odp_actions actions;