Signed-off-by: Jarno Rajahalme <jrajaha...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev