On Wed, Jun 03, 2015 at 11:21:50PM -0700, Alex Wang wrote:
> OVS datapath has check which prevents the installation of flow
> that matches VLAN TCI but does not have exact match for VLAN_CFI
> bit. However, the ovs userspace does not enforce it, so OpenFlow
> flow like "vlan_tci=0x000a/0x0fff,action=output:local" can be added
> to ovs. Subsequently, the generated megaflow will have match
> field for vlan like "vlan(vid=5/0xfff,pcp=0/0x0,cfi=1/0)".
>
> With the OVS datapath check, the installation of such megaflow
> will be rejected with:
> "|WARN|system@ovs-system: failed to put[create][modify] (Invalid argument)"
>
> This commit adds a check in userspace that mark the vlan mask
> invalid if it does not exact match for VLAN_CFI. So users will
> be asked to provide correct mask.
This is not the right fix, because it is legitimate and useful not to
match on the "CFI" (actually "vlan present") bit in OpenFlow. See the
comment in meta-flow.h:
/* "vlan_tci".
*
* 802.1Q TCI.
*
* For a packet with an 802.1Q header, this is the Tag Control Information
* (TCI) field, with the CFI bit forced to 1. For a packet with no 802.1Q
* header, this has value 0.
*
* This field can be used in various ways:
*
* - If it is not constrained at all, the nx_match matches packets
* without an 802.1Q header or with an 802.1Q header that has any TCI
* value.
*
* - Testing for an exact match with 0 matches only packets without an
* 802.1Q header.
*
* - Testing for an exact match with a TCI value with CFI=1 matches
* packets that have an 802.1Q header with a specified VID and PCP.
*
* - Testing for an exact match with a nonzero TCI value with CFI=0 does
* not make sense. The switch may reject this combination.
*
* - Testing with a specific VID and CFI=1, with nxm_mask=0x1fff, matches
* packets that have an 802.1Q header with that VID (and any PCP).
*
* - Testing with a specific PCP and CFI=1, with nxm_mask=0xf000, matches
* packets that have an 802.1Q header with that PCP (and any VID).
*
* - Testing with nxm_value=0, nxm_mask=0x0fff matches packets with no
* 802.1Q header or with an 802.1Q header with a VID of 0.
*
* - Testing with nxm_value=0, nxm_mask=0xe000 matches packets with no
* 802.1Q header or with an 802.1Q header with a PCP of 0.
*
* - Testing with nxm_value=0, nxm_mask=0xefff matches packets with no
* 802.1Q header or with an 802.1Q header with both VID and PCP of 0.
*
* Type: be16.
* Maskable: bitwise.
* Formatting: hexadecimal.
* Prerequisites: none.
* Access: read/write.
* NXM: NXM_OF_VLAN_TCI(4) since v1.1.
* OXM: none.
* OF1.0: exact match.
* OF1.1: exact match.
*/
MFF_VLAN_TCI,
I think that we should fix this in flow translation, by "unwildcarding"
the CFI bit if any other bits in vlan_tci are unwildcarded.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
