tests: Test that ofp10_match bytes that should be ignored really are.

Rob Sherwood reported a bug in OVS treatment of ofp10_match bytes that
should be ignored some time ago:

> In any case, the pktact.SingleWildcardMatch and
> pktact.AllExceptOneWildcardMatch tests were failing because it looks
> like OVS (v1.4 release) was  not matching vlan tagged packets when the
> match wildcarded vlan but the dl_vlan value (which should be ignored,
> because it is wildcarded) was non-zero.  We've worked around this in
> OFTest by making sure that the dl_vlan value is zero when vlan is
> wildcarded and now the test passes.
>
> In other words:
>
> if (ofp_match->wildcards&OFPFW_DL_VLAN) is true, then the match should
> match both tagged and untagged packets, independent of the value of
> ofp_match->dl_vlan.  OVS (seemingly) only matches tagged packets if
> ofp_match->dl_vlan == 0.

I wasn't able to spot the problem at the time, and I still don't see a
problem (perhaps it has been fixed since then), but this commit should
prevent any regression for this specific problem and for anything like it.

It would be natural to modify the parse-ofp11-match test in the same way,
but this commit doesn't do it.

Rob's original bug report is at:
https://mailman.stanford.edu/pipermail/openflow-discuss/2012-March/003107.html

Reported-by: Rob Sherwood <rob.sherwood@bigswitch.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index b1b4c6c17c2211ff5fe0c0ee7b9c6b77230de00a..f35ac41ecdc54c3e87f56908f2e661b88826ec52 100644 (file)
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -730,55 +730,55 @@ AT_SETUP([ovs-ofctl parse-ofp10-match])
 AT_KEYWORDS([OF1.0])
 AT_DATA([test-data], [dnl
 # in_port=65534
-003820fe fffe 000000000000 000000000000 0000 00 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+003820fe fffe xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_src=00:01:02:03:04:05
-003820fb 0000 000102030405 000000000000 0000 00 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+003820fb xxxx 000102030405 xxxxxxxxxxxx xxxx xx xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_dst=10:20:30:40:50:60
-003820f7 0000 000000000000 102030405060 0000 00 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+003820f7 xxxx xxxxxxxxxxxx 102030405060 xxxx xx xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_vlan=291
-003820fd 0000 000000000000 000000000000 0123 00 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+003820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx 0123 xx xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_vlan_pcp=5
-002820ff 0000 000000000000 000000000000 0000 05 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+002820ff xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx 05 xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_vlan=291,dl_vlan_pcp=4
-002820fd 0000 000000000000 000000000000 0123 04 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+002820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx 0123 04 xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # vlan_tci=0x0000
-003820fd 0000 000000000000 000000000000 ffff 00 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+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 OVS ignores it and drops it on output.
 # vlan_tci=0x0000
 #  1: 28 -> 38
 # 20: 05 -> 00
-002820fd 0000 000000000000 000000000000 ffff 05 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+002820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx ffff 05 xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 dnl Invalid VID and PCP discards out-of-range bits:
 # dl_vlan=256,dl_vlan_pcp=7
 # 18: f1 -> 01
 # 20: ff -> 07
-002820fd 0000 000000000000 000000000000 f100 ff 00 0000 00 00 0000 dnl
-00000000 00000000 0000 0000
+002820fd xxxx xxxxxxxxxxxx xxxxxxxxxxxx f100 ff xx xxxx xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # dl_type=0x1234
-003820ef 0000 000000000000 000000000000 0000 00 00 1234 00 00 0000 dnl
-00000000 00000000 0000 0000
+003820ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 1234 xx xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # ip,nw_proto=5
-003820cf 0000 000000000000 000000000000 0000 00 00 0800 00 05 0000 dnl
-00000000 00000000 0000 0000
+003820cf xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 05 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 dnl Ignore nw_proto if not IP or ARP:
 # dl_type=0x1234,nw_proto=5
@@ -787,12 +787,12 @@ dnl Ignore nw_proto if not IP or ARP:
 & ofp_util|INFO|normalization changed ofp_match, details:
 & ofp_util|INFO| pre: dl_type=0x1234,nw_proto=5
 & ofp_util|INFO|post: dl_type=0x1234
-003820cf 0000 000000000000 000000000000 0000 00 00 1234 00 05 0000 dnl
-00000000 00000000 0000 0000
+003820cf xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 1234 xx 05 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # ip,nw_tos=252
-001820ef 0000 000000000000 000000000000 0000 00 00 0800 fc 00 0000 dnl
-00000000 00000000 0000 0000
+001820ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 fc xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 dnl Ignore nw_tos if not IP:
 # arp,nw_tos=4
@@ -802,54 +802,54 @@ dnl Ignore nw_tos if not IP:
 & ofp_util|INFO|normalization changed ofp_match, details:
 & ofp_util|INFO| pre: arp,nw_tos=4
 & ofp_util|INFO|post: arp
-001820ef 0000 000000000000 000000000000 0000 00 00 0806 05 00 0000 dnl
-00000000 00000000 0000 0000
+001820ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 05 xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 dnl Low 2 bits of invalid TOS are forced to 0:
 # ip,nw_tos=48
 # 24: 31 -> 30
-001820ef 0000 000000000000 000000000000 0000 00 00 0800 31 00 0000 dnl
-00000000 00000000 0000 0000
+001820ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 31 xx xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # arp,arp_op=2
-003820cf 0000 000000000000 000000000000 0000 00 00 0806 00 02 0000 dnl
-00000000 00000000 0000 0000
+003820cf xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 xx 02 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx xxxx
 
 # ip,nw_src=192.168.128.85
-003800ef 0000 000000000000 000000000000 0000 00 00 0800 00 00 0000 dnl
-c0a88055 00000000 0000 0000
+003800ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx xx xxxx dnl
+c0a88055 xxxxxxxx xxxx xxxx
 
 # ip,nw_src=192.168.128.0/24
 # 31: 55 -> 00
-003808ef 0000 000000000000 000000000000 0000 00 00 0800 00 00 0000 dnl
-c0a88055 00000000 0000 0000
+003808ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx xx xxxx dnl
+c0a88055 xxxxxxxx xxxx xxxx
 
 # ip,nw_dst=192.168.128.85
-003020ef 0000 000000000000 000000000000 0000 00 00 0800 00 00 0000 dnl
-00000000 c0a88055 0000 0000
+003020ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx xx xxxx dnl
+xxxxxxxx c0a88055 xxxx xxxx
 
 # ip,nw_dst=192.168.128.0/24
 # 35: 55 -> 00
-003220ef 0000 000000000000 000000000000 0000 00 00 0800 00 00 0000 dnl
-00000000 c0a88055 0000 0000
+003220ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx xx xxxx dnl
+xxxxxxxx c0a88055 xxxx xxxx
 
 # arp,nw_src=192.168.128.85
-003800ef 0000 000000000000 000000000000 0000 00 00 0806 00 00 0000 dnl
-c0a88055 00000000 0000 0000
+003800ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 xx xx xxxx dnl
+c0a88055 xxxxxxxx xxxx xxxx
 
 # arp,nw_src=192.168.128.0/24
 # 31: 55 -> 00
-003808ef 0000 000000000000 000000000000 0000 00 00 0806 00 00 0000 dnl
-c0a88055 00000000 0000 0000
+003808ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 xx xx xxxx dnl
+c0a88055 xxxxxxxx xxxx xxxx
 
 # arp,nw_dst=192.168.128.85
-003020ef 0000 000000000000 000000000000 0000 00 00 0806 00 00 0000 dnl
-00000000 c0a88055 0000 0000
+003020ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 xx xx xxxx dnl
+xxxxxxxx c0a88055 xxxx xxxx
 
 # arp,nw_dst=192.168.128.0/24
 # 35: 55 -> 00
-003220ef 0000 000000000000 000000000000 0000 00 00 0806 00 00 0000 dnl
-00000000 c0a88055 0000 0000
+003220ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0806 xx xx xxxx dnl
+xxxxxxxx c0a88055 xxxx xxxx
 
 dnl Ignore nw_src if not IP or ARP:
 # dl_type=0x1234,nw_src=192.168.128.0/24
@@ -861,8 +861,8 @@ dnl Ignore nw_src if not IP or ARP:
 & ofp_util|INFO|normalization changed ofp_match, details:
 & ofp_util|INFO| pre: dl_type=0x1234,nw_src=192.168.128.0/24
 & ofp_util|INFO|post: dl_type=0x1234
-003808ef 0000 000000000000 000000000000 0000 00 00 1234 00 00 0000 dnl
-c0a88055 00000000 0000 0000
+003808ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 1234 xx xx xxxx dnl
+c0a88055 xxxxxxxx xxxx xxxx
 
 dnl Ignore nw_dst if not IP or ARP:
 # dl_type=0x1234,nw_dst=192.168.128.0/24
@@ -874,32 +874,32 @@ dnl Ignore nw_dst if not IP or ARP:
 & ofp_util|INFO|normalization changed ofp_match, details:
 & ofp_util|INFO| pre: dl_type=0x1234,nw_dst=192.168.128.0/24
 & ofp_util|INFO|post: dl_type=0x1234
-003220ef 0000 000000000000 000000000000 0000 00 00 1234 00 00 0000 dnl
-00000000 c0a88055 0000 0000
+003220ef xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 1234 xx xx xxxx dnl
+xxxxxxxx c0a88055 xxxx xxxx
 
 # tcp,tp_src=443
-0038208f 0000 000000000000 000000000000 0000 00 00 0800 00 06 0000 dnl
-00000000 00000000 01bb 0000
+0038208f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 06 xxxx dnl
+xxxxxxxx xxxxxxxx 01bb xxxx
 
 # tcp,tp_dst=443
-0038204f 0000 000000000000 000000000000 0000 00 00 0800 00 06 0000 dnl
-00000000 00000000 0000 01bb
+0038204f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 06 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx 01bb
 
 # udp,tp_src=443
-0038208f 0000 000000000000 000000000000 0000 00 00 0800 00 11 0000 dnl
-00000000 00000000 01bb 0000
+0038208f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 11 xxxx dnl
+xxxxxxxx xxxxxxxx 01bb xxxx
 
 # udp,tp_dst=443
-0038204f 0000 000000000000 000000000000 0000 00 00 0800 00 11 0000 dnl
-00000000 00000000 0000 01bb
+0038204f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 11 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx 01bb
 
 # icmp,icmp_type=5
-0038208f 0000 000000000000 000000000000 0000 00 00 0800 00 01 0000 dnl
-00000000 00000000 0005 0000
+0038208f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 01 xxxx dnl
+xxxxxxxx xxxxxxxx 0005 xxxx
 
 # icmp,icmp_code=8
-0038204f 0000 000000000000 000000000000 0000 00 00 0800 00 01 0000 dnl
-00000000 00000000 0000 0008
+0038204f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 01 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx 0008
 
 dnl Ignore tp_src if not TCP or UDP:
 # ip,nw_proto=21,tp_src=443
@@ -909,8 +909,8 @@ dnl Ignore tp_src if not TCP or UDP:
 & ofp_util|INFO|normalization changed ofp_match, details:
 & ofp_util|INFO| pre: ip,nw_proto=21,tp_src=443
 & ofp_util|INFO|post: ip,nw_proto=21
-0038208f 0000 000000000000 000000000000 0000 00 00 0800 00 15 0000 dnl
-00000000 00000000 01bb 0000
+0038208f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 15 xxxx dnl
+xxxxxxxx xxxxxxxx 01bb xxxx
 
 dnl Ignore tp_dst if not TCP or UDP:
 # ip,nw_proto=21,tp_dst=443
@@ -918,8 +918,8 @@ dnl Ignore tp_dst if not TCP or UDP:
 # normal: 38: 01 -> 00
 # normal: 39: bb -> 00
 dnl The normalization details are suppressed here due to rate-limiting.
-0038204f 0000 000000000000 000000000000 0000 00 00 0800 00 15 0000 dnl
-00000000 00000000 0000 01bb
+0038204f xxxx xxxxxxxxxxxx xxxxxxxxxxxx xxxx xx xx 0800 xx 15 xxxx dnl
+xxxxxxxx xxxxxxxx xxxx 01bb
 
 ])
 sed '/^[[#&]]/d' < test-data > input.txt

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 2343240bb87095a0681ac445ee75a3a8f6685e30..cd42b969c43ca21a55f21bcda88d73fc5707f8b0 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2334,20 +2334,50 @@ ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 /* "parse-ofp10-match": reads a series of ofp10_match specifications as hex
  * bytes from stdin, converts them to cls_rules, prints them as strings on
  * stdout, and then converts them back to hex bytes and prints any differences
- * from the input. */
+ * from the input.
+ *
+ * The input hex bytes may contain "x"s to represent "don't-cares", bytes whose
+ * values are ignored in the input and will be set to zero when OVS converts
+ * them back to hex bytes.  ovs-ofctl actually sets "x"s to random bits when
+ * it does the conversion to hex, to ensure that in fact they are ignored. */
 static void
 ofctl_parse_ofp10_match(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 {
+    struct ds expout;
     struct ds in;
 
     ds_init(&in);
+    ds_init(&expout);
     while (!ds_get_preprocessed_line(&in, stdin)) {
-        struct ofpbuf match_in;
+        struct ofpbuf match_in, match_expout;
         struct ofp10_match match_out;
         struct ofp10_match match_normal;
         struct cls_rule rule;
+        char *p;
+
+        /* Parse hex bytes to use for expected output. */
+        ds_clear(&expout);
+        ds_put_cstr(&expout, ds_cstr(&in));
+        for (p = ds_cstr(&expout); *p; p++) {
+            if (*p == 'x') {
+                *p = '0';
+            }
+        }
+        ofpbuf_init(&match_expout, 0);
+        if (ofpbuf_put_hex(&match_expout, ds_cstr(&expout), NULL)[0] != '\0') {
+            ovs_fatal(0, "Trailing garbage in hex data");
+        }
+        if (match_expout.size != sizeof(struct ofp10_match)) {
+            ovs_fatal(0, "Input is %zu bytes, expected %zu",
+                      match_expout.size, sizeof(struct ofp10_match));
+        }
 
-        /* Parse hex bytes. */
+        /* Parse hex bytes for input. */
+        for (p = ds_cstr(&in); *p; p++) {
+            if (*p == 'x') {
+                *p = "0123456789abcdef"[random_uint32() & 0xf];
+            }
+        }
         ofpbuf_init(&match_in, 0);
         if (ofpbuf_put_hex(&match_in, ds_cstr(&in), NULL)[0] != '\0') {
             ovs_fatal(0, "Trailing garbage in hex data");
@@ -2364,7 +2394,7 @@ ofctl_parse_ofp10_match(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 
         /* Convert back to ofp10_match and print differences from input. */
         ofputil_cls_rule_to_ofp10_match(&rule, &match_out);
-        print_differences("", match_in.data, match_in.size,
+        print_differences("", match_expout.data, match_expout.size,
                           &match_out, sizeof match_out);
 
         /* Normalize, then convert and compare again. */
@@ -2375,8 +2405,10 @@ ofctl_parse_ofp10_match(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
         putchar('\n');
 
         ofpbuf_uninit(&match_in);
+        ofpbuf_uninit(&match_expout);
     }
     ds_destroy(&in);
+    ds_destroy(&expout);
 }
 
 /* "parse-ofp11-match": reads a series of ofp11_match specifications as hex