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

Ben Pfaff blp at nicira.com
Mon Nov 8 18:33:42 UTC 2010


On Sun, Nov 07, 2010 at 11:16:21PM -0800, Justin Pettit wrote:
> 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.)

For now I added a comment, thanks.

> > 
> > @@ -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"?

I guess you must be talking about cls_rule_zero_wildcards().  I changed
it to cls_rule_zero_wildcarded_fields().  I guess that makes it clear
what it does.




More information about the dev mailing list