ofproto: Reduce number of "collect" functions taking lots of parameters.
[sliver-openvswitch.git] / ofproto / ofproto.c
index 9605baa..26f8069 100644 (file)
@@ -190,6 +190,38 @@ static uint32_t rule_eviction_priority(struct rule *);
 static void eviction_group_add_rule(struct rule *);
 static void eviction_group_remove_rule(struct rule *);
 
+/* Criteria that flow_mod and other operations use for selecting rules on
+ * which to operate. */
+struct rule_criteria {
+    /* An OpenFlow table or 255 for all tables. */
+    uint8_t table_id;
+
+    /* OpenFlow matching criteria.  Interpreted different in "loose" way by
+     * collect_rules_loose() and "strict" way by collect_rules_strict(), as
+     * defined in the OpenFlow spec. */
+    struct cls_rule cr;
+
+    /* Matching criteria for the OpenFlow cookie.  Consider a bit B in a rule's
+     * cookie and the corresponding bits C in 'cookie' and M in 'cookie_mask'.
+     * The rule will not be selected if M is 1 and B != C.  */
+    ovs_be64 cookie;
+    ovs_be64 cookie_mask;
+
+    /* Selection based on actions within a rule:
+     *
+     * If out_port != OFPP_ANY, selects only rules that output to out_port.
+     * If out_group != OFPG_ALL, select only rules that output to out_group. */
+    ofp_port_t out_port;
+    uint32_t out_group;
+};
+
+static void rule_criteria_init(struct rule_criteria *, uint8_t table_id,
+                               const struct match *match,
+                               unsigned int priority,
+                               ovs_be64 cookie, ovs_be64 cookie_mask,
+                               ofp_port_t out_port, uint32_t out_group);
+static void rule_criteria_destroy(struct rule_criteria *);
+
 /* ofport. */
 static void ofport_destroy__(struct ofport *);
 static void ofport_destroy(struct ofport *);
@@ -1140,11 +1172,6 @@ ofproto_destroy__(struct ofproto *ofproto)
     ovs_assert(list_is_empty(&ofproto->pending));
     ovs_assert(!ofproto->n_pending);
 
-    if (ofproto->meters) {
-        meter_delete(ofproto, 1, ofproto->meter_features.max_meters);
-        free(ofproto->meters);
-    }
-
     delete_group(ofproto, OFPG_ALL);
     ovs_rwlock_destroy(&ofproto->groups_rwlock);
     hmap_destroy(&ofproto->groups);
@@ -1186,6 +1213,13 @@ ofproto_destroy(struct ofproto *p)
         return;
     }
 
+    if (p->meters) {
+        meter_delete(p, 1, p->meter_features.max_meters);
+        p->meter_features.max_meters = 0;
+        free(p->meters);
+        p->meters = NULL;
+    }
+
     ofproto_flush__(p);
     HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) {
         ofport_destroy(ofport);
@@ -2935,169 +2969,158 @@ next_matching_table(const struct ofproto *ofproto,
          (TABLE) != NULL;                                         \
          (TABLE) = next_matching_table(OFPROTO, TABLE, TABLE_ID))
 
-/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
- * 'table_id' is 0xff) that match 'match' in the "loose" way required for
- * OpenFlow OFPFC_MODIFY and OFPFC_DELETE requests and puts them on list
- * 'rules'.
+/* Initializes 'criteria' in a straightforward way based on the other
+ * parameters.
  *
- * If 'out_port' is anything other than OFPP_ANY, then only rules that output
- * to 'out_port' are included.
+ * For "loose" matching, the 'priority' parameter is unimportant and may be
+ * supplied as 0. */
+static void
+rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id,
+                   const struct match *match, unsigned int priority,
+                   ovs_be64 cookie, ovs_be64 cookie_mask,
+                   ofp_port_t out_port, uint32_t out_group)
+{
+    criteria->table_id = table_id;
+    cls_rule_init(&criteria->cr, match, priority);
+    criteria->cookie = cookie;
+    criteria->cookie_mask = cookie_mask;
+    criteria->out_port = out_port;
+    criteria->out_group = out_group;
+}
+
+static void
+rule_criteria_destroy(struct rule_criteria *criteria)
+{
+    cls_rule_destroy(&criteria->cr);
+}
+
+static enum ofperr
+collect_rule(struct rule *rule, const struct rule_criteria *c,
+             struct list *rules)
+{
+    if (ofproto_rule_is_hidden(rule)) {
+        return 0;
+    } else if (rule->pending) {
+        return OFPROTO_POSTPONE;
+    } else {
+        if ((c->table_id == rule->table_id || c->table_id == 0xff)
+            && ofproto_rule_has_out_port(rule, c->out_port)
+            && ofproto_rule_has_out_group(rule, c->out_group)
+            && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask)) {
+            list_push_back(rules, &rule->ofproto_node);
+        }
+        return 0;
+    }
+}
+
+/* Searches 'ofproto' for rules that match the criteria in 'criteria'.  Matches
+ * on classifiers rules are done in the "loose" way required for OpenFlow
+ * OFPFC_MODIFY and OFPFC_DELETE requests.  Puts the selected rules on list
+ * 'rules'.
  *
  * Hidden rules are always omitted.
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static enum ofperr
-collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
-                    const struct match *match,
-                    ovs_be64 cookie, ovs_be64 cookie_mask,
-                    ofp_port_t out_port, uint32_t out_group,
-                    struct list *rules)
+collect_rules_loose(struct ofproto *ofproto,
+                    const struct rule_criteria *criteria, struct list *rules)
 {
     struct oftable *table;
-    struct cls_rule cr;
     enum ofperr error;
 
-    error = check_table_id(ofproto, table_id);
+    error = check_table_id(ofproto, criteria->table_id);
     if (error) {
         return error;
     }
 
     list_init(rules);
-    cls_rule_init(&cr, match, 0);
 
-    if (cookie_mask == htonll(UINT64_MAX)) {
+    if (criteria->cookie_mask == htonll(UINT64_MAX)) {
         struct rule *rule;
 
-        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
+        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node,
+                                   hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
-            if (table_id != rule->table_id && table_id != 0xff) {
-                continue;
-            }
-            if (ofproto_rule_is_hidden(rule)) {
-                continue;
-            }
-            if (cls_rule_is_loose_match(&rule->cr, &cr.match)) {
-                if (rule->pending) {
-                    error = OFPROTO_POSTPONE;
-                    goto exit;
-                }
-                if (rule->flow_cookie == cookie /* Hash collisions possible. */
-                    && ofproto_rule_has_out_port(rule, out_port)
-                    && ofproto_rule_has_out_group(rule, out_group)) {
-                    list_push_back(rules, &rule->ofproto_node);
+            if (cls_rule_is_loose_match(&rule->cr, &criteria->cr.match)) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
                 }
             }
         }
-        goto exit;
-    }
-
-    FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
-        struct cls_cursor cursor;
-        struct rule *rule;
+    } else {
+        FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) {
+            struct cls_cursor cursor;
+            struct rule *rule;
 
-        ovs_rwlock_rdlock(&table->cls.rwlock);
-        cls_cursor_init(&cursor, &table->cls, &cr);
-        CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-            if (rule->pending) {
-                ovs_rwlock_unlock(&table->cls.rwlock);
-                error = OFPROTO_POSTPONE;
-                goto exit;
-            }
-            if (!ofproto_rule_is_hidden(rule)
-                && ofproto_rule_has_out_port(rule, out_port)
-                && ofproto_rule_has_out_group(rule, out_group)
-                    && !((rule->flow_cookie ^ cookie) & cookie_mask)) {
-                list_push_back(rules, &rule->ofproto_node);
+            ovs_rwlock_rdlock(&table->cls.rwlock);
+            cls_cursor_init(&cursor, &table->cls, &criteria->cr);
+            CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
+                }
             }
+            ovs_rwlock_unlock(&table->cls.rwlock);
         }
-        ovs_rwlock_unlock(&table->cls.rwlock);
     }
 
-exit:
-    cls_rule_destroy(&cr);
     return error;
 }
 
-/* Searches 'ofproto' for rules in table 'table_id' (or in all tables, if
- * 'table_id' is 0xff) that match 'match' in the "strict" way required for
- * OpenFlow OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests and puts them
- * on list 'rules'.
- *
- * If 'out_port' is anything other than OFPP_ANY, then only rules that output
- * to 'out_port' are included.
+/* Searches 'ofproto' for rules that match the criteria in 'criteria'.  Matches
+ * on classifiers rules are done in the "strict" way required for OpenFlow
+ * OFPFC_MODIFY_STRICT and OFPFC_DELETE_STRICT requests.  Puts the selected
+ * rules on list 'rules'.
  *
  * Hidden rules are always omitted.
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static enum ofperr
-collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
-                     const struct match *match, unsigned int priority,
-                     ovs_be64 cookie, ovs_be64 cookie_mask,
-                     ofp_port_t out_port, uint32_t out_group,
-                     struct list *rules)
+collect_rules_strict(struct ofproto *ofproto,
+                     const struct rule_criteria *criteria, struct list *rules)
 {
     struct oftable *table;
-    struct cls_rule cr;
     int error;
 
-    error = check_table_id(ofproto, table_id);
+    error = check_table_id(ofproto, criteria->table_id);
     if (error) {
         return error;
     }
 
     list_init(rules);
-    cls_rule_init(&cr, match, priority);
 
-    if (cookie_mask == htonll(UINT64_MAX)) {
+    if (criteria->cookie_mask == htonll(UINT64_MAX)) {
         struct rule *rule;
 
-        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node, hash_cookie(cookie),
+        HINDEX_FOR_EACH_WITH_HASH (rule, cookie_node,
+                                   hash_cookie(criteria->cookie),
                                    &ofproto->cookies) {
-            if (table_id != rule->table_id && table_id != 0xff) {
-                continue;
-            }
-            if (ofproto_rule_is_hidden(rule)) {
-                continue;
-            }
-            if (cls_rule_equal(&rule->cr, &cr)) {
-                if (rule->pending) {
-                    error = OFPROTO_POSTPONE;
-                    goto exit;
-                }
-                if (rule->flow_cookie == cookie /* Hash collisions possible. */
-                    && ofproto_rule_has_out_port(rule, out_port)
-                    && ofproto_rule_has_out_group(rule, out_group)) {
-                    list_push_back(rules, &rule->ofproto_node);
+            if (cls_rule_equal(&rule->cr, &criteria->cr)) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
                 }
             }
         }
-        goto exit;
-    }
-
-    FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) {
-        struct rule *rule;
+    } else {
+        FOR_EACH_MATCHING_TABLE (table, criteria->table_id, ofproto) {
+            struct rule *rule;
 
-        ovs_rwlock_rdlock(&table->cls.rwlock);
-        rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls,
-                                                               &cr));
-        ovs_rwlock_unlock(&table->cls.rwlock);
-        if (rule) {
-            if (rule->pending) {
-                error = OFPROTO_POSTPONE;
-                goto exit;
-            }
-            if (!ofproto_rule_is_hidden(rule)
-                && ofproto_rule_has_out_port(rule, out_port)
-                && ofproto_rule_has_out_group(rule, out_group)
-                    && !((rule->flow_cookie ^ cookie) & cookie_mask)) {
-                list_push_back(rules, &rule->ofproto_node);
+            ovs_rwlock_rdlock(&table->cls.rwlock);
+            rule = rule_from_cls_rule(classifier_find_rule_exactly(
+                                          &table->cls, &criteria->cr));
+            ovs_rwlock_unlock(&table->cls.rwlock);
+            if (rule) {
+                error = collect_rule(rule, criteria, rules);
+                if (error) {
+                    break;
+                }
             }
         }
     }
 
-exit:
-    cls_rule_destroy(&cr);
-    return 0;
+    return error;
 }
 
 /* Returns 'age_ms' (a duration in milliseconds), converted to seconds and
@@ -3116,6 +3139,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_stats_request fsr;
+    struct rule_criteria criteria;
     struct list replies;
     struct list rules;
     struct rule *rule;
@@ -3126,9 +3150,10 @@ handle_flow_stats_request(struct ofconn *ofconn,
         return error;
     }
 
-    error = collect_rules_loose(ofproto, fsr.table_id, &fsr.match,
-                                fsr.cookie, fsr.cookie_mask,
-                                fsr.out_port, fsr.out_group, &rules);
+    rule_criteria_init(&criteria, fsr.table_id, &fsr.match, 0, fsr.cookie,
+                       fsr.cookie_mask, fsr.out_port, fsr.out_group);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
     if (error) {
         return error;
     }
@@ -3239,6 +3264,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn,
     struct ofputil_flow_stats_request request;
     struct ofputil_aggregate_stats stats;
     bool unknown_packets, unknown_bytes;
+    struct rule_criteria criteria;
     struct ofpbuf *reply;
     struct list rules;
     struct rule *rule;
@@ -3249,9 +3275,11 @@ handle_aggregate_stats_request(struct ofconn *ofconn,
         return error;
     }
 
-    error = collect_rules_loose(ofproto, request.table_id, &request.match,
-                                request.cookie, request.cookie_mask,
-                                request.out_port, request.out_group, &rules);
+    rule_criteria_init(&criteria, request.table_id, &request.match, 0,
+                       request.cookie, request.cookie_mask,
+                       request.out_port, request.out_group);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
     if (error) {
         return error;
     }
@@ -3713,12 +3741,15 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
                    struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
+    struct rule_criteria criteria;
     struct list rules;
     int error;
 
-    error = collect_rules_loose(ofproto, fm->table_id, &fm->match,
-                                fm->cookie, fm->cookie_mask,
-                                OFPP_ANY, OFPG11_ANY, &rules);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, 0,
+                       fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
@@ -3738,12 +3769,15 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
                    struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
+    struct rule_criteria criteria;
     struct list rules;
     int error;
 
-    error = collect_rules_strict(ofproto, fm->table_id, &fm->match,
-                                 fm->priority, fm->cookie, fm->cookie_mask,
-                                 OFPP_ANY, OFPG11_ANY, &rules);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority,
+                       fm->cookie, fm->cookie_mask, OFPP_ANY, OFPG11_ANY);
+    error = collect_rules_strict(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
@@ -3797,12 +3831,16 @@ delete_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
+    struct rule_criteria criteria;
     struct list rules;
     enum ofperr error;
 
-    error = collect_rules_loose(ofproto, fm->table_id, &fm->match,
-                                fm->cookie, fm->cookie_mask,
-                                fm->out_port, fm->out_group, &rules);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, 0,
+                       fm->cookie, fm->cookie_mask,
+                       fm->out_port, fm->out_group);
+    error = collect_rules_loose(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
     return (error ? error
             : !list_is_empty(&rules) ? delete_flows__(ofproto, ofconn, request,
                                                       &rules, OFPRR_DELETE)
@@ -3815,12 +3853,16 @@ delete_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
+    struct rule_criteria criteria;
     struct list rules;
     enum ofperr error;
 
-    error = collect_rules_strict(ofproto, fm->table_id, &fm->match,
-                                 fm->priority, fm->cookie, fm->cookie_mask,
-                                 fm->out_port, fm->out_group, &rules);
+    rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority,
+                       fm->cookie, fm->cookie_mask,
+                       fm->out_port, fm->out_group);
+    error = collect_rules_strict(ofproto, &criteria, &rules);
+    rule_criteria_destroy(&criteria);
+
     return (error ? error
             : list_is_singleton(&rules) ? delete_flows__(ofproto, ofconn,
                                                          request, &rules,
@@ -4630,9 +4672,12 @@ handle_meter_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 
     if (mm.command != OFPMC13_DELETE) {
         /* Fails also when meters are not implemented by the provider. */
-        if (!meter_id || meter_id > ofproto->meter_features.max_meters) {
+        if (meter_id == 0 || meter_id > OFPM13_MAX) {
             error = OFPERR_OFPMMFC_INVALID_METER;
             goto exit_free_bands;
+        } else if (meter_id > ofproto->meter_features.max_meters) {
+            error = OFPERR_OFPMMFC_OUT_OF_METERS;
+            goto exit_free_bands;
         }
         if (mm.meter.n_bands > ofproto->meter_features.max_bands) {
             error = OFPERR_OFPMMFC_OUT_OF_BANDS;