[ovs-dev] [nxm 33/42] classifier: Add more cls_rule_set_*() functions.

[Justin Pettit]

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

> +bool
> +cls_rule_set_dl_tci_masked(struct cls_rule *rule, ovs_be16 tci, ovs_be16 mask)
> +{
> +    switch (ntohs(mask)) {
> +    case 0xffff:
> +        if (tci == htons(0)) {
> +            rule->wc.wildcards &= ~(OFPFW_DL_VLAN | OFPFW_DL_VLAN_PCP);
> +            rule->flow.dl_vlan = htons(OFP_VLAN_NONE);
> +            rule->flow.dl_vlan_pcp = 0;
> +            return true;
> +        } else if (tci & htons(VLAN_CFI)) {
> +            rule->wc.wildcards &= ~(OFPFW_DL_VLAN | OFPFW_DL_VLAN_PCP);
> +            rule->flow.dl_vlan = htons(vlan_tci_to_vid(tci));
> +            rule->flow.dl_vlan_pcp = vlan_tci_to_pcp(tci);
> +            return true;
> +        } else {
> +            return false;
> +        }
> +
> +    case 0x1fff:
> +        if (!(tci & htons(VLAN_CFI))) {
> +            return false;
> +        } else {
> +            cls_rule_set_dl_vlan(rule, tci & htons(VLAN_VID_MASK));
> +            rule->wc.wildcards |= OFPFW_DL_VLAN_PCP;
> +            rule->flow.dl_vlan_pcp = 0;
> +            return true;
> +        }
> +
> +    case 0xf000:
> +        if (!(tci & htons(VLAN_CFI))) {
> +            return false;
> +        } else {
> +            cls_rule_set_dl_vlan_pcp(rule, vlan_tci_to_pcp(tci));
> +            rule->wc.wildcards |= OFPFW_DL_VLAN;
> +            rule->flow.dl_vlan = 0;
> +            return true;
> +        }
> +
> +    default:
> +        return false;
> +    }
> +

I think it would be helpful to label what's happening in each of these "case" statements, and the two "if" statements in 0xffff; it's not immediately obvious from looking at them.  Looking ahead at the extensible match patch, it looks like "0xe000" is a supported mask, but it's not covered above.

Otherwise, it looks good.

--Justin