[ovs-dev] [nxm 35/42] flow: Better abstract flow_wildcards and use it more widely.

[Justin Pettit]

On Oct 28, 2010, at 10:28 AM, Ben Pfaff wrote:

> -/* Converts the flow in 'flow' into a cls_rule in 'rule', with the given
> - * 'wildcards' and 'priority'.*/
> +/* Converts the flow in 'flow' into an exact-match cls_rule in 'rule', with the
> + * given 'priority'. */
> void
> -cls_rule_from_flow(const struct flow *flow, uint32_t wildcards,
> -                   unsigned int priority, struct cls_rule *rule)
> +cls_rule_init_exact(const struct flow *flow,
> +                    unsigned int priority, struct cls_rule *rule)
> {
> -    cls_rule_init__(rule, flow, wildcards);
> +    rule->flow = *flow;
> +    flow_wildcards_init_exact(&rule->wc);
>     rule->priority = priority;
> }

With OpenFlow <= 1.0, exact match entries are always the highest priority (65,535).  I can see that you're allowing the priority to be set to allow "hidden" flows with a higher priority than OpenFlow allows.  It might be good to reference that in the function's comment, and maybe even enforce that the priority is not less than 65,535 with an assertion or warning.  (I realize that when we add support for multiple tables, that this requirement will likely go away.)

> 
> @@ -131,12 +135,9 @@ cls_rule_from_match(const struct ofp_match *match, unsigned int priority,
>                     int flow_format, uint64_t cookie,
>                     struct cls_rule *rule)
> {
> -    uint32_t wildcards;
> -    struct flow flow;
> -
> -    flow_from_match(match, flow_format, cookie, &flow, &wildcards);
> -    cls_rule_init__(rule, &flow, wildcards);
> -    rule->priority = rule->wc.wildcards ? priority : UINT16_MAX;
> +    flow_from_match(match, flow_format, cookie, &rule->flow, &rule->wc);
> +    rule->priority = !rule->wc.wildcards ? UINT16_MAX : priority;
> +    cls_rule_zero_wildcards(rule);

I realize this is a relatively recent function, but the name keeps throwing me.  It sounds like it's going to zero out the wildcards field of 'rule'.  It's not great either, but what about something along the lines of "cls_rule_sanitize_wildcards"?

Otherwise, looks good.  Thanks,

--Justin