Each of these functions had only a single caller, and breaking them out as separate functions didn't seem to many the code clearer.
I added a new function meter_insert_rule(): oftable_insert_rule() had used members of struct meter directly, which worked because it was after the definition of struct meter. So there was a choice to either move the definition of struct meter earlier or add a helper function; I chose the latter. Signed-off-by: Ben Pfaff <b...@nicira.com> --- ofproto/ofproto.c | 86 ++++++++++++++++++++----------------------------------- 1 file changed, 31 insertions(+), 55 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 26f1ed4..80c7126 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -163,7 +163,6 @@ static void oftable_enable_eviction(struct oftable *, static void oftable_remove_rule(struct rule *rule) OVS_REQUIRES(ofproto_mutex); static void oftable_remove_rule__(struct ofproto *, struct rule *) OVS_REQUIRES(ofproto_mutex); -static void oftable_insert_rule(struct rule *); /* A set of rules within a single OpenFlow table (oftable) that have the same * values for the oftable's eviction_fields. A rule to be evicted, when one is @@ -273,9 +272,6 @@ static bool rule_is_modifiable(const struct rule *rule, static enum ofperr add_flow(struct ofproto *, struct ofconn *, struct ofputil_flow_mod *, const struct ofp_header *); -static void do_add_flow(struct ofproto *, struct ofconn *, - const struct ofp_header *request, uint32_t buffer_id, - struct rule *); static enum ofperr modify_flows__(struct ofproto *, struct ofconn *, struct ofputil_flow_mod *, const struct ofp_header *, @@ -304,6 +300,7 @@ static uint64_t pick_fallback_dpid(void); static void ofproto_destroy__(struct ofproto *); static void update_mtu(struct ofproto *, struct ofport *); static void meter_delete(struct ofproto *, uint32_t first, uint32_t last); +static void meter_insert_rule(struct rule *); /* unixctl. */ static void ofproto_unixctl_init(void); @@ -3962,6 +3959,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct ofputil_flow_mod *fm, const struct ofp_header *request) OVS_REQUIRES(ofproto_mutex) { + const struct rule_actions *actions; + struct ofopgroup *group; struct oftable *table; struct cls_rule cr; struct rule *rule; @@ -4083,8 +4082,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, *CONST_CAST(uint8_t *, &rule->table_id) = table - ofproto->tables; rule->flags = fm->flags & OFPUTIL_FF_STATE; - ovsrcu_set(&rule->actions, - rule_actions_create(ofproto, fm->ofpacts, fm->ofpacts_len)); + actions = rule_actions_create(ofproto, fm->ofpacts, fm->ofpacts_len); + ovsrcu_set(&rule->actions, actions); list_init(&rule->meter_list_node); rule->eviction_group = NULL; list_init(&rule->expirable); @@ -4099,26 +4098,25 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, return error; } - /* Insert rule. */ - do_add_flow(ofproto, ofconn, request, fm->buffer_id, rule); - - return error; -} - -static void -do_add_flow(struct ofproto *ofproto, struct ofconn *ofconn, - const struct ofp_header *request, uint32_t buffer_id, - struct rule *rule) - OVS_REQUIRES(ofproto_mutex) -{ - struct ofopgroup *group; + if (fm->hard_timeout || fm->idle_timeout) { + list_insert(&ofproto->expirable, &rule->expirable); + } + cookies_insert(ofproto, rule); + eviction_group_add_rule(rule); + if (actions->provider_meter_id != UINT32_MAX) { + meter_insert_rule(rule); + } - oftable_insert_rule(rule); + fat_rwlock_wrlock(&table->cls.rwlock); + classifier_insert(&table->cls, CONST_CAST(struct cls_rule *, &rule->cr)); + fat_rwlock_unlock(&table->cls.rwlock); - group = ofopgroup_create(ofproto, ofconn, request, buffer_id); + group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id); ofoperation_create(group, rule, OFOPERATION_ADD, 0); ofproto->ofproto_class->rule_insert(rule); ofopgroup_submit(group); + + return error; } /* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */ @@ -5025,6 +5023,18 @@ get_provider_meter_id(const struct ofproto *ofproto, uint32_t of_meter_id) return UINT32_MAX; } +/* Finds the meter invoked by 'rule''s actions and adds 'rule' to the meter's + * list of rules. */ +static void +meter_insert_rule(struct rule *rule) +{ + const struct rule_actions *a = rule_get_actions(rule); + uint32_t meter_id = ofpacts_get_meter(a->ofpacts, a->ofpacts_len); + struct meter *meter = rule->ofproto->meters[meter_id]; + + list_insert(&meter->rules, &rule->meter_list_node); +} + static void meter_update(struct meter *meter, const struct ofputil_meter_config *config) { @@ -6880,40 +6890,6 @@ oftable_remove_rule(struct rule *rule) { oftable_remove_rule__(rule->ofproto, rule); } - -/* Inserts 'rule' into its oftable, which must not already contain any rule for - * the same cls_rule. */ -static void -oftable_insert_rule(struct rule *rule) - OVS_REQUIRES(ofproto_mutex) -{ - struct ofproto *ofproto = rule->ofproto; - struct oftable *table = &ofproto->tables[rule->table_id]; - const struct rule_actions *actions; - bool may_expire; - - ovs_mutex_lock(&rule->mutex); - may_expire = rule->hard_timeout || rule->idle_timeout; - ovs_mutex_unlock(&rule->mutex); - - if (may_expire) { - list_insert(&ofproto->expirable, &rule->expirable); - } - - cookies_insert(ofproto, rule); - - actions = rule_get_actions(rule); - if (actions->provider_meter_id != UINT32_MAX) { - uint32_t meter_id = ofpacts_get_meter(actions->ofpacts, - actions->ofpacts_len); - struct meter *meter = ofproto->meters[meter_id]; - list_insert(&meter->rules, &rule->meter_list_node); - } - fat_rwlock_wrlock(&table->cls.rwlock); - classifier_insert(&table->cls, CONST_CAST(struct cls_rule *, &rule->cr)); - fat_rwlock_unlock(&table->cls.rwlock); - eviction_group_add_rule(rule); -} /* unixctl commands. */ -- 1.9.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev