From: Ben Pfaff Date: Wed, 11 Sep 2013 22:35:44 +0000 (-0700) Subject: ofproto: Reduce number of "collect" functions taking lots of parameters. X-Git-Tag: v2.0~53 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=505ed3fbc1ff20720d2419a62626b2c3247a036f ofproto: Reduce number of "collect" functions taking lots of parameters. The long lists of parameters for all these "collect" functions was starting to get to me. This reduces the number of such functions to one. Signed-off-by: Ben Pfaff Acked-by: Ethan Jackson --- diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 6921e0710..852a18d2d 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -190,6 +190,36 @@ 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. */ + ofp_port_t out_port; +}; + +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); +static void rule_criteria_destroy(struct rule_criteria *); + /* ofport. */ static void ofport_destroy__(struct ofport *); static void ofport_destroy(struct ofport *); @@ -2920,77 +2950,92 @@ next_matching_table(const struct ofproto *ofproto, (TABLE) != NULL; \ (TABLE) = next_matching_table(OFPROTO, TABLE, TABLE_ID)) +/* Initializes 'criteria' in a straightforward way based on the other + * parameters. + * + * 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) +{ + 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; +} + +static void +rule_criteria_destroy(struct rule_criteria *criteria) +{ + cls_rule_destroy(&criteria->cr); +} + static enum ofperr -collect_rule(struct rule *rule, uint8_t table_id, - ovs_be64 cookie, ovs_be64 cookie_mask, - ofp_port_t out_port, struct list *rules) +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 ((table_id == rule->table_id || table_id == 0xff) - && ofproto_rule_has_out_port(rule, out_port) - && !((rule->flow_cookie ^ cookie) & cookie_mask)) { + if ((c->table_id == rule->table_id || c->table_id == 0xff) + && ofproto_rule_has_out_port(rule, c->out_port) + && !((rule->flow_cookie ^ c->cookie) & c->cookie_mask)) { list_push_back(rules, &rule->ofproto_node); } return 0; } } -/* 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 +/* 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'. * - * If 'out_port' is anything other than OFPP_ANY, then only rules that output - * to 'out_port' are included. - * * 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, 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 (cls_rule_is_loose_match(&rule->cr, &cr.match)) { - error = collect_rule(rule, table_id, cookie, cookie_mask, - out_port, rules); + if (cls_rule_is_loose_match(&rule->cr, &criteria->cr.match)) { + error = collect_rule(rule, criteria, rules); if (error) { break; } } } } else { - FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) { + 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_init(&cursor, &table->cls, &criteria->cr); CLS_CURSOR_FOR_EACH (rule, cr, &cursor) { - error = collect_rule(rule, table_id, cookie, cookie_mask, - out_port, rules); + error = collect_rule(rule, criteria, rules); if (error) { break; } @@ -2999,63 +3044,54 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t table_id, } } - 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, 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 (cls_rule_equal(&rule->cr, &cr)) { - error = collect_rule(rule, table_id, cookie, cookie_mask, - out_port, rules); + if (cls_rule_equal(&rule->cr, &criteria->cr)) { + error = collect_rule(rule, criteria, rules); if (error) { break; } } } } else { - FOR_EACH_MATCHING_TABLE (table, table_id, ofproto) { + 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)); + 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, table_id, cookie, cookie_mask, - out_port, rules); + error = collect_rule(rule, criteria, rules); if (error) { break; } @@ -3063,7 +3099,6 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t table_id, } } - cls_rule_destroy(&cr); return error; } @@ -3083,6 +3118,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; @@ -3093,9 +3129,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, &rules); + rule_criteria_init(&criteria, fsr.table_id, &fsr.match, 0, fsr.cookie, + fsr.cookie_mask, fsr.out_port); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); if (error) { return error; } @@ -3210,6 +3247,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; @@ -3220,9 +3258,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, &rules); + rule_criteria_init(&criteria, request.table_id, &request.match, 0, + request.cookie, request.cookie_mask, + request.out_port); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); if (error) { return error; } @@ -3685,12 +3725,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, &rules); + rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, + fm->cookie, fm->cookie_mask, OFPP_ANY); + error = collect_rules_loose(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + if (error) { return error; } else if (list_is_empty(&rules)) { @@ -3710,12 +3753,14 @@ 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, &rules); + rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, + fm->cookie, fm->cookie_mask, OFPP_ANY); + error = collect_rules_strict(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); if (error) { return error; @@ -3770,12 +3815,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, &rules); + rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, + fm->cookie, fm->cookie_mask, + fm->out_port); + 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) @@ -3788,12 +3837,15 @@ 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, &rules); + rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, + fm->cookie, fm->cookie_mask, fm->out_port); + error = collect_rules_strict(ofproto, &criteria, &rules); + rule_criteria_destroy(&criteria); + return (error ? error : list_is_singleton(&rules) ? delete_flows__(ofproto, ofconn, request, &rules,