On Tue, Oct 15, 2013 at 10:49:41AM -0700, Jarno Rajahalme wrote: > > 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.
I think I'd be OK either way. I guess it's harmless, at worst. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
