ofproto: Remove soon-to-be-invalid optimizations.
[sliver-openvswitch.git] / ofproto / ofproto.c
index 3ae6cce..f84510f 100644 (file)
@@ -1115,10 +1115,20 @@ ofproto_get_snoops(const struct ofproto *ofproto, struct sset *snoops)
 
 /* Deletes 'rule' from 'cls' within 'ofproto'.
  *
+ * Within an ofproto implementation, this function allows an ofproto
+ * implementation to destroy any rules that remain when its ->destruct()
+ * function is called.  This function is not suitable for use elsewhere in an
+ * ofproto implementation.
+ *
+ * This function is also used internally in ofproto.c.
+ *
+ * This function implements steps 4.4 and 4.5 in the section titled "Rule Life
+ * Cycle" in ofproto-provider.h.
+
  * The 'cls' argument is redundant (it is &ofproto->tables[rule->table_id].cls)
  * but it allows Clang to do better checking. */
-static void
-ofproto_delete_rule(struct ofproto *ofproto, struct classifier *cls,
+void
+ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls,
                     struct rule *rule)
     OVS_REQ_WRLOCK(cls->rwlock)
 {
@@ -1156,7 +1166,7 @@ ofproto_flush__(struct ofproto *ofproto)
         cls_cursor_init(&cursor, &table->cls, NULL);
         CLS_CURSOR_FOR_EACH_SAFE (rule, next_rule, cr, &cursor) {
             if (!rule->pending) {
-                ofproto_delete_rule(ofproto, &table->cls, rule);
+                ofproto_rule_delete(ofproto, &table->cls, rule);
             }
         }
         ovs_rwlock_unlock(&table->cls.rwlock);
@@ -1722,6 +1732,33 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port)
     return error;
 }
 
+static int
+simple_flow_mod(struct ofproto *ofproto,
+                const struct match *match, unsigned int priority,
+                const struct ofpact *ofpacts, size_t ofpacts_len,
+                enum ofp_flow_mod_command command)
+{
+    struct ofputil_flow_mod fm;
+
+    memset(&fm, 0, sizeof fm);
+    fm.match = *match;
+    fm.priority = priority;
+    fm.cookie = 0;
+    fm.new_cookie = 0;
+    fm.modify_cookie = false;
+    fm.table_id = 0;
+    fm.command = command;
+    fm.idle_timeout = 0;
+    fm.hard_timeout = 0;
+    fm.buffer_id = UINT32_MAX;
+    fm.out_port = OFPP_ANY;
+    fm.out_group = OFPG_ANY;
+    fm.flags = 0;
+    fm.ofpacts = CONST_CAST(struct ofpact *, ofpacts);
+    fm.ofpacts_len = ofpacts_len;
+    return handle_flow_mod__(ofproto, NULL, &fm, NULL);
+}
+
 /* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
  * performs the 'n_actions' actions in 'actions'.  The new flow will not
  * timeout.
@@ -1740,21 +1777,21 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
 {
     const struct rule *rule;
 
+    /* First do a cheap check whether the rule we're looking for already exists
+     * with the actions that we want.  If it does, then we're done. */
     ovs_rwlock_rdlock(&ofproto->tables[0].cls.rwlock);
     rule = rule_from_cls_rule(classifier_find_match_exactly(
                                   &ofproto->tables[0].cls, match, priority));
     ovs_rwlock_unlock(&ofproto->tables[0].cls.rwlock);
+
+    /* If there's no such rule or the rule doesn't have the actions we want,
+     * fall back to a executing a full flow mod.  We can't optimize this at
+     * all because we didn't take enough locks above to ensure that the flow
+     * table didn't already change beneath us.  */
     if (!rule || !ofpacts_equal(rule->ofpacts, rule->ofpacts_len,
                                 ofpacts, ofpacts_len)) {
-        struct ofputil_flow_mod fm;
-
-        memset(&fm, 0, sizeof fm);
-        fm.match = *match;
-        fm.priority = priority;
-        fm.buffer_id = UINT32_MAX;
-        fm.ofpacts = ofpacts;
-        fm.ofpacts_len = ofpacts_len;
-        add_flow(ofproto, NULL, &fm, NULL);
+        simple_flow_mod(ofproto, match, priority, ofpacts, ofpacts_len,
+                        OFPFC_MODIFY_STRICT);
     }
 }
 
@@ -1780,26 +1817,21 @@ ofproto_delete_flow(struct ofproto *ofproto,
     struct classifier *cls = &ofproto->tables[0].cls;
     struct rule *rule;
 
+    /* First do a cheap check whether the rule we're looking for has already
+     * been deleted.  If so, then we're done. */
     ovs_rwlock_rdlock(&cls->rwlock);
     rule = rule_from_cls_rule(classifier_find_match_exactly(cls, target,
                                                             priority));
     ovs_rwlock_unlock(&cls->rwlock);
     if (!rule) {
-        /* No such rule -> success. */
-        return true;
-    } else if (rule->pending) {
-        /* An operation on the rule is already pending -> failure.
-         * Caller must retry later if it's important. */
-        return false;
-    } else {
-        /* Initiate deletion -> success. */
-        ovs_rwlock_wrlock(&cls->rwlock);
-        ofproto_delete_rule(ofproto, cls, rule);
-        ovs_rwlock_unlock(&cls->rwlock);
-
         return true;
     }
 
+    /* Fall back to a executing a full flow mod.  We can't optimize this at all
+     * because we didn't take enough locks above to ensure that the flow table
+     * didn't already change beneath us.  */
+    return simple_flow_mod(ofproto, target, priority, NULL, 0,
+                           OFPFC_DELETE_STRICT) != OFPROTO_POSTPONE;
 }
 
 /* Starts the process of deleting all of the flows from all of ofproto's flow
@@ -2309,21 +2341,6 @@ ofproto_rule_destroy__(struct rule *rule)
     rule->ofproto->ofproto_class->rule_dealloc(rule);
 }
 
-/* This function allows an ofproto implementation to destroy any rules that
- * remain when its ->destruct() function is called.  This function implements
- * steps 4.4 and 4.5 in the section titled "Rule Life Cycle" in
- * ofproto-provider.h.
- *
- * This function should only be called from an ofproto implementation's
- * ->destruct() function.  It is not suitable elsewhere. */
-void
-ofproto_rule_delete(struct ofproto *ofproto, struct classifier *cls,
-                    struct rule *rule)
-    OVS_REQ_WRLOCK(cls->rwlock)
-{
-    ofproto_delete_rule(ofproto, cls, rule);
-}
-
 /* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
  * that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
 bool
@@ -2529,30 +2546,6 @@ reject_slave_controller(struct ofconn *ofconn)
     }
 }
 
-/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
- * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
- *
- * This function relies on the order of 'ofpacts' being correct (as checked by
- * ofpacts_verify()). */
-static uint32_t
-find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
-{
-    const struct ofpact *a;
-
-    OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
-        enum ovs_instruction_type inst;
-
-        inst = ovs_instruction_type_from_ofpact_type(a->type);
-        if (a->type == OFPACT_METER) {
-            return ofpact_get_METER(a)->meter_id;
-        } else if (inst > OVSINST_OFPIT13_METER) {
-            break;
-        }
-    }
-
-    return 0;
-}
-
 /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are appropriate
  * for a packet with the prerequisites satisfied by 'flow' in table 'table_id'.
  * 'flow' may be temporarily modified, but is restored at return.
@@ -2571,7 +2564,7 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
         return error;
     }
 
-    mid = find_meter(ofpacts, ofpacts_len);
+    mid = ofpacts_get_meter(ofpacts, ofpacts_len);
     if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
         return OFPERR_OFPMMFC_INVALID_METER;
     }
@@ -3659,7 +3652,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
 
     rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
     rule->ofpacts_len = fm->ofpacts_len;
-    rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+    rule->meter_id = ofpacts_get_meter(rule->ofpacts, rule->ofpacts_len);
     list_init(&rule->meter_list_node);
     rule->eviction_group = NULL;
     list_init(&rule->expirable);
@@ -3765,7 +3758,8 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
             rule->ofpacts_len = fm->ofpacts_len;
             ovs_rwlock_unlock(&rule->rwlock);
 
-            rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+            rule->meter_id = ofpacts_get_meter(rule->ofpacts,
+                                               rule->ofpacts_len);
             rule->ofproto->ofproto_class->rule_modify_actions(rule,
                                                               reset_counters);
         } else {
@@ -3982,7 +3976,7 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
     ofproto_rule_send_removed(rule, reason);
 
     ovs_rwlock_wrlock(&cls->rwlock);
-    ofproto_delete_rule(ofproto, cls, rule);
+    ofproto_rule_delete(ofproto, cls, rule);
     ovs_rwlock_unlock(&cls->rwlock);
 }