Re: [ovs-dev] [encap v2 3/3] datapath: Don't drop packets with partial vlan tags.

[Ben Pfaff]

On Mon, Nov 14, 2011 at 04:29:18PM -0800, Ben Pfaff wrote:
> OK, here's an incremental for that part.  There's a lot of irrelevant
> churn not shown here due to the addition of ovs_action_push_vlan in
> revising patch 2, so I'll send a new copy of the patch separately.

Oops, forgot the incremental.


diff --git a/datapath/flow.c b/datapath/flow.c
index fe3aec6..7a13624 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -1048,13 +1048,18 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int 
*key_lenp,
        memcpy(swkey->eth.src, eth_key->eth_src, ETH_ALEN);
        memcpy(swkey->eth.dst, eth_key->eth_dst, ETH_ALEN);
 
-       if (attrs == ((1 << OVS_KEY_ATTR_VLAN) |
-                     (1 << OVS_KEY_ATTR_ETHERTYPE) |
-                     (1 << OVS_KEY_ATTR_ENCAP)) &&
+       if (attrs & (1u << OVS_KEY_ATTR_ETHERTYPE) &&
            nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) == htons(ETH_P_8021Q)) {
-               const struct nlattr *encap = a[OVS_KEY_ATTR_ENCAP];
-               __be16 tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
+               const struct nlattr *encap;
+               __be16 tci;
+
+               if (attrs != ((1 << OVS_KEY_ATTR_VLAN) |
+                             (1 << OVS_KEY_ATTR_ETHERTYPE) |
+                             (1 << OVS_KEY_ATTR_ENCAP)))
+                       return -EINVAL;
 
+               encap = a[OVS_KEY_ATTR_ENCAP];
+               tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
                if (tci & htons(VLAN_TAG_PRESENT)) {
                        swkey->eth.tci = tci;
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 1a4da92..2edba2d 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1207,15 +1211,26 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t 
key_len,
     }
     expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ETHERNET;
 
-    if ((present_attrs & ~expected_attrs)
-        == ((UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE) |
-            (UINT64_C(1) << OVS_KEY_ATTR_VLAN) |
-            (UINT64_C(1) << OVS_KEY_ATTR_ENCAP))
+    if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE)
         && (nl_attr_get_be16(attrs[OVS_KEY_ATTR_ETHERTYPE])
             == htons(ETH_TYPE_VLAN))) {
-        const struct nlattr *encap = attrs[OVS_KEY_ATTR_ENCAP];
-        ovs_be16 tci = nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]);
+        /* The Ethernet type is 0x8100 so there must be a VLAN tag
+         * and encapsulated protocol information. */
+        const struct nlattr *encap;
+        __be16 tci;
+        int error;
+
+        expected_attrs |= ((UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE) |
+                           (UINT64_C(1) << OVS_KEY_ATTR_VLAN) |
+                           (UINT64_C(1) << OVS_KEY_ATTR_ENCAP));
+        error = check_expectations(present_attrs, expected_attrs,
+                                   key, key_len);
+        if (error) {
+            return error;
+        }
 
+        encap = attrs[OVS_KEY_ATTR_ENCAP];
+        tci = nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]);
         if (tci & htons(VLAN_CFI)) {
             flow->vlan_tci = tci;
 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev