odp-util: Fix VLAN parsing behavior in parse_8021q_onward().

Anytime there is a VLAN the flow needs to properly reflect that.  Keeping
the TPID in dl_type never makes sense and will probably cause problems.
The existing code did the right thing in the common case but not in corner
cases where it returned ODP_FIT_TOO_MUCH or ODP_FIT_TOO_LITTLE (the cases
where it returned an error don't matter since nothing looks at the flow
in that case).

Found by inspection.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5cc2757..7c6aad4 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3146,7 +3146,6 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
            ? attrs[OVS_KEY_ATTR_ENCAP] : NULL);
     enum odp_key_fitness encap_fitness;
     enum odp_key_fitness fitness;
-    ovs_be16 tci;
 
     /* Calculate fitness of outer attributes. */
     if (!is_mask) {
@@ -3163,35 +3162,32 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
     fitness = check_expectations(present_attrs, out_of_range_attr,
                                  expected_attrs, key, key_len);
 
-    /* Get the VLAN TCI value. */
-    if (!is_mask && !(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN))) {
-        return ODP_FIT_TOO_LITTLE;
+    /* Set vlan_tci.
+     * Remove the TPID from dl_type since it's not the real Ethertype.  */
+    flow->dl_type = htons(0);
+    flow->vlan_tci = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)
+                      ? nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN])
+                      : htons(0));
+    if (!is_mask) {
+        if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN))) {
+            return ODP_FIT_TOO_LITTLE;
+        } else if (flow->vlan_tci == htons(0)) {
+            /* Corner case for a truncated 802.1Q header. */
+            if (fitness == ODP_FIT_PERFECT && nl_attr_get_size(encap)) {
+                return ODP_FIT_TOO_MUCH;
+            }
+            return fitness;
+        } else if (!(flow->vlan_tci & htons(VLAN_CFI))) {
+            VLOG_ERR_RL(&rl, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is nonzero "
+                        "but CFI bit is not set", ntohs(flow->vlan_tci));
+            return ODP_FIT_ERROR;
+        }
     } else {
-        tci = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)
-               ? nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN])
-               : htons(0));
-        if (!is_mask) {
-            if (tci == htons(0)) {
-                /* Corner case for a truncated 802.1Q header. */
-                if (fitness == ODP_FIT_PERFECT && nl_attr_get_size(encap)) {
-                    return ODP_FIT_TOO_MUCH;
-                }
-                return fitness;
-            } else if (!(tci & htons(VLAN_CFI))) {
-                VLOG_ERR_RL(&rl, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is nonzero "
-                            "but CFI bit is not set", ntohs(tci));
-                return ODP_FIT_ERROR;
-            }
+        if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ENCAP))) {
+            return fitness;
         }
-        /* Set vlan_tci.
-         * Remove the TPID from dl_type since it's not the real Ethertype.  */
-        flow->dl_type = htons(0);
-        flow->vlan_tci = tci;
     }
 
-    if (is_mask && !(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ENCAP))) {
-        return fitness;
-    }
     /* Now parse the encapsulated attributes. */
     if (!parse_flow_nlattrs(nl_attr_get(encap), nl_attr_get_size(encap),
                             attrs, &present_attrs, &out_of_range_attr)) {