On Oct 15, 2013, at 10:06 AM, Jarno Rajahalme <[email protected]> wrote:
> > On Oct 15, 2013, at 9:43 AM, Ben Pfaff <[email protected]> wrote: > >> On Tue, Oct 15, 2013 at 09:19:51AM -0700, Jarno Rajahalme wrote: >>> Sets mask bits for the given field and its prerequisite fields. >>> Needed for unwildcarding the proper bits from datapath masks. >>> >>> Signed-off-by: Jarno Rajahalme <[email protected]> >> >> I still think that MFP_VLAN_VID doesn't have any mask prereqs, >> e.g. instead of this: >> case MFP_ARP: >> case MFP_IPV4: >> case MFP_IPV6: >> case MFP_MPLS: >> case MFP_VLAN_VID: >> case MFP_IP_ANY: >> mask->dl_type = OVS_BE16_MAX; >> /* Fall through. */ >> case MFP_NONE: >> break; >> this: >> case MFP_ARP: >> case MFP_IPV4: >> case MFP_IPV6: >> case MFP_MPLS: >> case MFP_IP_ANY: >> mask->dl_type = OVS_BE16_MAX; >> /* Fall through. */ >> case MFP_VLAN_VID: >> case MFP_NONE: >> break; >> >> Otherwise: >> Acked-by: Ben Pfaff <[email protected]> > > Sorry that I missed that, I was thinking about the ether type dependency on > the wire, and did not remember that it is not visible in struct flow. > > But now I wonder if the mask's VLAN_CFI bit should be set in this case, as > mf_are_prereqs_ok() checks the CFI bit in this case. Thoughts? > Update: MFP_VLAN_VID is only ever used for MFF_VLAN_PCP. The flow_set_vlan_pcp() always sets the VLAN_CFI bit as well, so setting the bit again in case MFP_VLAN_VID is not necessary, but it may be a bit confusing not to set it. I guess it would be prudent to set it anyway for clarity. Jarno _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
