On 06/05/14 at 10:02pm, Ben Pfaff wrote:
> 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>
[...]
> -
> -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);
I noticed the change in order of exposing the rule to the
eviction group and inserting the meter rule plus inserting the
classifier. Is that a problem?
> +/* 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)
> +{
Would it make sense to pass the rule_actions into the function
directly?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
