ofp-util: For OF1.0, don't wildcard PCP field when 802.1Q header absent.

OpenFlow 1.0.1 says:

    The dl_vlan_pcp field must be ignored when the OFPFW_DL_VLAN wildcard
    bit is set or when the dl_vlan value is set to OFP_VLAN_NONE.  Fields
    that are ignored don’t need to be wildcarded and should be set to 0.

Previously, OVS wildcarded the PCP field when dl_vlan was OFP_VLAN_NONE,
but this commit changes the behavior to that suggested above: the PCP
field should not be wildcarded (and should be set to 0, but the code
already did that).

This commit only changes the translation from OVS's internal flow format
to the OpenFlow 1.0 wire format.  Translation in the other direction and
to other formats is unaffected.

Found by OFTest.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

diff --git a/DESIGN.md b/DESIGN.md
index 58826d3..6865d47 100644 (file)
--- a/DESIGN.md
+++ b/DESIGN.md
@@ -436,11 +436,13 @@ Each column is interpreted as follows.
     NXM_OF_VLAN_TCI(_W), a mask of ffff is equivalent to
     NXM_OF_VLAN_TCI.
 
-  - OF1.0 and OF1.1: wwww/x,yy/z means dl_vlan wwww, OFPFW_DL_VLAN
-    x, dl_vlan_pcp yy, and OFPFW_DL_VLAN_PCP z.  ? means that the
-    given nibble is ignored (and conventionally 0 for wwww or yy,
-    conventionally 1 for x or z).  <none> means that the given match
-    is not supported.
+  - OF1.0 and OF1.1: wwww/x,yy/z means dl_vlan wwww, OFPFW_DL_VLAN x,
+    dl_vlan_pcp yy, and OFPFW_DL_VLAN_PCP z.  If OFPFW_DL_VLAN or
+    OFPFW_DL_VLAN_PCP is 1, the corresponding field value is
+    wildcarded, otherwise it is matched.  ? means that the given bits
+    are ignored (their conventional values are 0000/x,00/0 in OF1.0,
+    0000/x,00/1 in OF1.1; x is never ignored).  <none> means that the
+    given match is not supported.
 
   - OF1.2: xxxx/yyyy,zz means OXM_OF_VLAN_VID_W with value xxxx and
     mask yyyy, and OXM_OF_VLAN_PCP (which is not maskable) with

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index eb22fb7..0a5232d 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -344,7 +344,6 @@ ofputil_match_to_ofp10_match(const struct match *match,
     } else if (match->wc.masks.vlan_tci & htons(VLAN_CFI)
                && !(match->flow.vlan_tci & htons(VLAN_CFI))) {
         ofmatch->dl_vlan = htons(OFP10_VLAN_NONE);
-        ofpfw |= OFPFW10_DL_VLAN_PCP;
     } else {
         if (!(match->wc.masks.vlan_tci & htons(VLAN_VID_MASK))) {
             ofpfw |= OFPFW10_DL_VLAN;

diff --git a/tests/flowgen.pl b/tests/flowgen.pl
index cdc275e..a0fc345 100755 (executable)
--- a/tests/flowgen.pl
+++ b/tests/flowgen.pl
@@ -1,6 +1,6 @@
 #! /usr/bin/perl
 
-# Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (c) 2009, 2010, 2011, 2012, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -115,8 +115,6 @@ sub output {
     $packet .= pack_ethaddr($flow{DL_SRC});
     if ($flow{DL_VLAN} != 0xffff) {
         $packet .= pack('nn', 0x8100, $flow{DL_VLAN});
-    } else {
-        $wildcards |= 1 << 20;   # OFPFW10_DL_VLAN_PCP
     }
     my $len_ofs = length($packet);
     $packet .= pack('n', 0) if $attrs{DL_HEADER} =~ /^802.2/;

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index c1e9ec4..ad08b31 100644 (file)
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -1140,14 +1140,16 @@ xxxxxxxx xxxxxxxx xxxx xxxx
 002820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx 0123 04 xx xxxx xx xx xxxx dnl
 xxxxxxxx xxxxxxxx xxxx xxxx
 
+dnl dl_vlan_pcp doesn't make sense when 802.1Q is not present, so
+dnl OVS ignores it and drops it on output.
 # vlan_tci=0x0000
+#  1: 38 -> 28
 003820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx ffff xx xx xxxx xx xx xxxx dnl
 xxxxxxxx xxxxxxxx xxxx xxxx
 
-dnl dl_vlan_pcp doesn't make sense when dl_vlan is "none", so
+dnl dl_vlan_pcp doesn't make sense when 802.1Q is not present, so
 dnl OVS ignores it and drops it on output.
 # vlan_tci=0x0000
-#  1: 28 -> 38
 # 20: 05 -> 00
 002820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx ffff 05 xx xxxx xx xx xxxx dnl
 xxxxxxxx xxxxxxxx xxxx xxxx
@@ -2374,7 +2376,7 @@ AT_CHECK([ovs-ofctl check-vlan 0000 ffff], [0], [dnl
 vlan_tci=0x0000 -> 0000/ffff
 NXM: NXM_OF_VLAN_TCI(0000) -> 0000/ffff
 OXM: OXM_OF_VLAN_VID(0000) -> 0000/1fff,--
-OF1.0: ffff/0,00/1 -> 0000/ffff
+OF1.0: ffff/0,00/0 -> 0000/ffff
 OF1.1: ffff/0,00/1 -> 0000/ffff
 ])
 
@@ -2419,7 +2421,7 @@ AT_CHECK([ovs-ofctl check-vlan 0000 f000], [0], [dnl
 vlan_tci=0x0000/0xf000 -> 0000/f000
 NXM: NXM_OF_VLAN_TCI_W(0000/f000) -> 0000/f000
 OXM: OXM_OF_VLAN_VID_W(0000/1000) -> 0000/1000,--
-OF1.0: ffff/0,00/1 -> 0000/ffff
+OF1.0: ffff/0,00/0 -> 0000/ffff
 OF1.1: ffff/0,00/1 -> 0000/ffff
 ])