[ovs-dev] [PATCH v2 11/19] ofproto: Refactor modify_flows__().
Jarno Rajahalme
jrajahalme at nicira.com
Mon May 18 23:10:20 UTC 2015
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
ofproto/ofproto.c | 317 +++++++++++++++++++++++++++++++----------------------
1 file changed, 187 insertions(+), 130 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 6a8239e..5c8b1a5 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -256,9 +256,14 @@ struct flow_mod_requester {
static enum ofperr add_flow(struct ofproto *, struct ofputil_flow_mod *,
const struct flow_mod_requester *);
-static enum ofperr modify_flows__(struct ofproto *, struct ofputil_flow_mod *,
- const struct rule_collection *,
- const struct flow_mod_requester *);
+static enum ofperr modify_flow_check__(struct ofproto *,
+ struct ofputil_flow_mod *,
+ const struct rule *)
+ OVS_REQUIRES(ofproto_mutex);
+static void modify_flow__(struct ofproto *, struct ofputil_flow_mod *,
+ struct rule *, const struct flow_mod_requester *,
+ struct ovs_list *dead_cookies)
+ OVS_REQUIRES(ofproto_mutex);
static void delete_flows__(const struct rule_collection *,
enum ofp_flow_removed_reason,
const struct flow_mod_requester *)
@@ -4397,16 +4402,18 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
/* Transform "add" into "modify" if there's an existing identical flow. */
rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, &cr));
if (rule) {
- struct rule_collection rules;
-
cls_rule_destroy(&cr);
- rule_collection_init(&rules);
- rule_collection_add(&rules, rule);
fm->modify_cookie = true;
- error = modify_flows__(ofproto, fm, &rules, req);
- rule_collection_destroy(&rules);
+ error = modify_flow_check__(ofproto, fm, rule);
+ if (!error) {
+ struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
+ modify_flow__(ofproto, fm, rule, req, &dead_cookies);
+ learned_cookies_flush(ofproto, &dead_cookies);
+
+ goto send_packet;
+ }
return error;
}
@@ -4467,160 +4474,192 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
ofmonitor_report(ofproto->connmgr, rule, NXFME_ADDED, 0,
req ? req->ofconn : NULL, req ? req->xid : 0, NULL);
-
+send_packet:
return req ? send_buffered_packet(req->ofconn, fm->buffer_id, rule) : 0;
}
/* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
-/* Modifies the rules listed in 'rules', changing their actions to match those
- * in 'fm'.
+/* Checks if the 'rule' can be modified to match 'fm'.
*
- * 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id,
- * if any.
+ * Returns 0 on success, otherwise an OpenFlow error code. */
+static enum ofperr
+modify_flow_check__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct rule *rule)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ if (ofproto->ofproto_class->rule_premodify_actions) {
+ enum ofperr error;
+
+ error = ofproto->ofproto_class->rule_premodify_actions(
+ rule, fm->ofpacts, fm->ofpacts_len);
+ if (error) {
+ return error;
+ }
+ }
+
+ return 0;
+}
+
+/* Checks if the rules listed in 'rules' can have their actions changed to
+ * match those in 'fm'.
*
* Returns 0 on success, otherwise an OpenFlow error code. */
static enum ofperr
-modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct rule_collection *rules,
- const struct flow_mod_requester *req)
+modify_flows_check__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct rule_collection *rules)
OVS_REQUIRES(ofproto_mutex)
{
- struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
- enum nx_flow_update_event event;
+ enum ofperr error;
size_t i;
if (ofproto->ofproto_class->rule_premodify_actions) {
for (i = 0; i < rules->n; i++) {
- struct rule *rule = rules->rules[i];
- enum ofperr error;
-
- error = ofproto->ofproto_class->rule_premodify_actions(
- rule, fm->ofpacts, fm->ofpacts_len);
+ error = modify_flow_check__(ofproto, fm, rules->rules[i]);
if (error) {
return error;
}
}
}
- event = fm->command == OFPFC_ADD ? NXFME_ADDED : NXFME_MODIFIED;
- for (i = 0; i < rules->n; i++) {
- struct rule *rule = rules->rules[i];
+ return 0;
+}
- /* 'fm' says that */
- bool change_cookie = (fm->modify_cookie
- && fm->new_cookie != OVS_BE64_MAX
- && fm->new_cookie != rule->flow_cookie);
+/* Modifies the 'rule', changing them to match 'fm'. */
+static void
+modify_flow__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ struct rule *rule, const struct flow_mod_requester *req,
+ struct ovs_list *dead_cookies)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ enum nx_flow_update_event event = fm->command == OFPFC_ADD
+ ? NXFME_ADDED : NXFME_MODIFIED;
- const struct rule_actions *actions = rule_get_actions(rule);
- bool change_actions = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
- actions->ofpacts,
- actions->ofpacts_len);
+ /* 'fm' says that */
+ bool change_cookie = (fm->modify_cookie
+ && fm->new_cookie != OVS_BE64_MAX
+ && fm->new_cookie != rule->flow_cookie);
- bool reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
+ const struct rule_actions *actions = rule_get_actions(rule);
+ bool change_actions = !ofpacts_equal(fm->ofpacts, fm->ofpacts_len,
+ actions->ofpacts,
+ actions->ofpacts_len);
- long long int now = time_msec();
+ bool reset_counters = (fm->flags & OFPUTIL_FF_RESET_COUNTS) != 0;
- if (change_cookie) {
- cookies_remove(ofproto, rule);
- }
+ long long int now = time_msec();
- ovs_mutex_lock(&rule->mutex);
- if (fm->command == OFPFC_ADD) {
- rule->idle_timeout = fm->idle_timeout;
- rule->hard_timeout = fm->hard_timeout;
- rule->importance = fm->importance;
- rule->flags = fm->flags & OFPUTIL_FF_STATE;
- rule->created = now;
- }
- if (change_cookie) {
- rule->flow_cookie = fm->new_cookie;
- }
- rule->modified = now;
- ovs_mutex_unlock(&rule->mutex);
+ if (change_cookie) {
+ cookies_remove(ofproto, rule);
+ }
- if (change_cookie) {
- cookies_insert(ofproto, rule);
- }
- if (fm->command == OFPFC_ADD) {
- if (fm->idle_timeout || fm->hard_timeout || fm->importance) {
- if (!rule->eviction_group) {
- eviction_group_add_rule(rule);
- }
- } else {
- eviction_group_remove_rule(rule);
+ ovs_mutex_lock(&rule->mutex);
+ if (fm->command == OFPFC_ADD) {
+ rule->idle_timeout = fm->idle_timeout;
+ rule->hard_timeout = fm->hard_timeout;
+ rule->importance = fm->importance;
+ rule->flags = fm->flags & OFPUTIL_FF_STATE;
+ rule->created = now;
+ }
+ if (change_cookie) {
+ rule->flow_cookie = fm->new_cookie;
+ }
+ rule->modified = now;
+ ovs_mutex_unlock(&rule->mutex);
+
+ if (change_cookie) {
+ cookies_insert(ofproto, rule);
+ }
+ if (fm->command == OFPFC_ADD) {
+ if (fm->idle_timeout || fm->hard_timeout || fm->importance) {
+ if (!rule->eviction_group) {
+ eviction_group_add_rule(rule);
}
+ } else {
+ eviction_group_remove_rule(rule);
}
+ }
- if (change_actions) {
- /* We have to change the actions. The rule's conjunctive match set
- * is a function of its actions, so we need to update that too. The
- * conjunctive match set is used in the lookup process to figure
- * which (if any) collection of conjunctive sets the packet matches
- * with. However, a rule with conjunction actions is never to be
- * returned as a classifier lookup result. To make sure a rule with
- * conjunction actions is not returned as a lookup result, we update
- * them in a carefully chosen order:
- *
- * - If we're adding a conjunctive match set where there wasn't one
- * before, we have to make the conjunctive match set available to
- * lookups before the rule's actions are changed, as otherwise
- * rule with a conjunction action could be returned as a lookup
- * result.
- *
- * - To clear some nonempty conjunctive set, we set the rule's
- * actions first, so that a lookup can't return a rule with
- * conjunction actions.
- *
- * - Otherwise, order doesn't matter for changing one nonempty
- * conjunctive match set to some other nonempty set, since the
- * rule's actions are not seen by the classifier, and hence don't
- * matter either before or after the change. */
- struct cls_conjunction *conjs;
- size_t n_conjs;
- get_conjunctions(fm, &conjs, &n_conjs);
-
- if (n_conjs) {
- set_conjunctions(rule, conjs, n_conjs);
- }
- ovsrcu_set(&rule->actions, rule_actions_create(fm->ofpacts,
+ if (change_actions) {
+ /* We have to change the actions. The rule's conjunctive match set
+ * is a function of its actions, so we need to update that too. The
+ * conjunctive match set is used in the lookup process to figure
+ * which (if any) collection of conjunctive sets the packet matches
+ * with. However, a rule with conjunction actions is never to be
+ * returned as a classifier lookup result. To make sure a rule with
+ * conjunction actions is not returned as a lookup result, we update
+ * them in a carefully chosen order:
+ *
+ * - If we're adding a conjunctive match set where there wasn't one
+ * before, we have to make the conjunctive match set available to
+ * lookups before the rule's actions are changed, as otherwise
+ * rule with a conjunction action could be returned as a lookup
+ * result.
+ *
+ * - To clear some nonempty conjunctive set, we set the rule's
+ * actions first, so that a lookup can't return a rule with
+ * conjunction actions.
+ *
+ * - Otherwise, order doesn't matter for changing one nonempty
+ * conjunctive match set to some other nonempty set, since the
+ * rule's actions are not seen by the classifier, and hence don't
+ * matter either before or after the change. */
+ struct cls_conjunction *conjs;
+ size_t n_conjs;
+ get_conjunctions(fm, &conjs, &n_conjs);
+
+ if (n_conjs) {
+ set_conjunctions(rule, conjs, n_conjs);
+ }
+ ovsrcu_set(&rule->actions, rule_actions_create(fm->ofpacts,
fm->ofpacts_len));
- if (!conjs) {
- set_conjunctions(rule, conjs, n_conjs);
- }
-
- free(conjs);
+ if (!conjs) {
+ set_conjunctions(rule, conjs, n_conjs);
}
- if (change_actions || reset_counters) {
- ofproto->ofproto_class->rule_modify_actions(rule, reset_counters);
- }
+ free(conjs);
+ }
- if (event != NXFME_MODIFIED || change_actions || change_cookie) {
- ofmonitor_report(ofproto->connmgr, rule, event, 0,
- req ? req->ofconn : NULL, req ? req->xid : 0,
- change_actions ? actions : NULL);
- }
+ if (change_actions || reset_counters) {
+ ofproto->ofproto_class->rule_modify_actions(rule, reset_counters);
+ }
- if (change_actions) {
- learned_cookies_inc(ofproto, rule_get_actions(rule));
- learned_cookies_dec(ofproto, actions, &dead_cookies);
- rule_actions_destroy(actions);
- }
+ if (event != NXFME_MODIFIED || change_actions || change_cookie) {
+ ofmonitor_report(ofproto->connmgr, rule, event, 0,
+ req ? req->ofconn : NULL, req ? req->xid : 0,
+ change_actions ? actions : NULL);
}
- learned_cookies_flush(ofproto, &dead_cookies);
- if (fm->buffer_id != UINT32_MAX && req) {
- return send_buffered_packet(req->ofconn, fm->buffer_id,
- rules->rules[0]);
+ if (change_actions) {
+ learned_cookies_inc(ofproto, rule_get_actions(rule));
+ learned_cookies_dec(ofproto, actions, dead_cookies);
+ rule_actions_destroy(actions);
}
+}
- return 0;
+/* Modifies the rules listed in 'rules', changing their actions to match those
+ * in 'fm'.
+ *
+ * 'req' is used to retrieve the packet buffer specified in fm->buffer_id,
+ * if any. */
+static void
+modify_flows__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct rule_collection *rules,
+ const struct flow_mod_requester *req)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
+ size_t i;
+
+ for (i = 0; i < rules->n; i++) {
+ modify_flow__(ofproto, fm, rules->rules[i], req, &dead_cookies);
+ }
+ learned_cookies_flush(ofproto, &dead_cookies);
}
static enum ofperr
-modify_flows_add(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req)
+modify_flows_add__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct flow_mod_requester *req)
OVS_REQUIRES(ofproto_mutex)
{
if (fm->cookie_mask != htonll(0) || fm->new_cookie == OVS_BE64_MAX) {
@@ -4629,6 +4668,31 @@ modify_flows_add(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
return add_flow(ofproto, fm, req);
}
+static enum ofperr
+modify_flows(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct rule_collection *rules,
+ const struct flow_mod_requester *req)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ enum ofperr error;
+
+ if (rules->n > 0) {
+ error = modify_flows_check__(ofproto, fm, rules);
+ if (!error) {
+ modify_flows__(ofproto, fm, rules, req);
+
+ if (fm->buffer_id != UINT32_MAX && req) {
+ error = send_buffered_packet(req->ofconn, fm->buffer_id,
+ rules->rules[0]);
+ }
+ }
+ } else {
+ error = modify_flows_add__(ofproto, fm, req);
+ }
+
+ return error;
+}
+
/* Implements OFPFC_MODIFY. Returns 0 on success or an OpenFlow error code on
* failure.
*
@@ -4651,11 +4715,8 @@ modify_flows_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
rule_criteria_destroy(&criteria);
if (!error) {
- error = (rules.n > 0
- ? modify_flows__(ofproto, fm, &rules, req)
- : modify_flows_add(ofproto, fm, req));
+ error = modify_flows(ofproto, fm, &rules, req);
}
-
rule_collection_destroy(&rules);
return error;
@@ -4680,13 +4741,9 @@ modify_flow_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
rule_criteria_destroy(&criteria);
if (!error) {
- if (rules.n == 0) {
- error = modify_flows_add(ofproto, fm, req);
- } else if (rules.n == 1) {
- error = modify_flows__(ofproto, fm, &rules, req);
- }
+ /* collect_rules_strict() can return max 1 rule. */
+ error = modify_flows(ofproto, fm, &rules, req);
}
-
rule_collection_destroy(&rules);
return error;
--
1.7.10.4
More information about the dev
mailing list