-1 was used to indicate both nested and variable length attributes.
This patch introduces OVS_KEY_LEN_NESTED and OVS_KEY_LEN_VARIABLE to
tell them apart.

Refactor nlattr_set() to more more generally all ovs netlink key
attributes.

Signed-off-by: Andy Zhou <az...@nicira.com>
---
 datapath/flow_netlink.c | 53 +++++++++++++++++++++++++------------------------
 datapath/flow_netlink.h |  6 ++++++
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index e525c9d..e258302 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -248,7 +248,7 @@ static bool match_validate(const struct sw_flow_match 
*match,
 
 /* The size of the argument for each %OVS_KEY_ATTR_* Netlink attribute.  */
 static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
-       [OVS_KEY_ATTR_ENCAP] = -1,
+       [OVS_KEY_ATTR_ENCAP] = OVS_KEY_LEN_NESTED,
        [OVS_KEY_ATTR_PRIORITY] = sizeof(u32),
        [OVS_KEY_ATTR_IN_PORT] = sizeof(u32),
        [OVS_KEY_ATTR_SKB_MARK] = sizeof(u32),
@@ -267,7 +267,7 @@ static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
        [OVS_KEY_ATTR_ND] = sizeof(struct ovs_key_nd),
        [OVS_KEY_ATTR_DP_HASH] = sizeof(u32),
        [OVS_KEY_ATTR_RECIRC_ID] = sizeof(u32),
-       [OVS_KEY_ATTR_TUNNEL] = -1,
+       [OVS_KEY_ATTR_TUNNEL] = OVS_KEY_LEN_NESTED,
        [OVS_KEY_ATTR_MPLS] = sizeof(struct ovs_key_mpls),
 };
 
@@ -310,7 +310,8 @@ static int __parse_flow_nlattrs(const struct nlattr *attr,
                }
 
                expected_len = ovs_key_lens[type];
-               if (nla_len(nla) != expected_len && expected_len != -1) {
+               if (nla_len(nla) != expected_len &&
+                       expected_len != OVS_KEY_LEN_NESTED) {
                        OVS_NLERR("Key attribute has unexpected length (type=%d"
                                  ", length=%d, expected=%d).\n", type,
                                  nla_len(nla), expected_len);
@@ -362,7 +363,8 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
                        [OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT] = 0,
                        [OVS_TUNNEL_KEY_ATTR_CSUM] = 0,
                        [OVS_TUNNEL_KEY_ATTR_OAM] = 0,
-                       [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = -1,
+                       [OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] =
+                               OVS_KEY_LEN_VARIABLE,
                };
 
                if (type > OVS_TUNNEL_KEY_ATTR_MAX) {
@@ -372,7 +374,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
                }
 
                if (ovs_tunnel_key_lens[type] != nla_len(a) &&
-                   ovs_tunnel_key_lens[type] != -1) {
+                   ovs_tunnel_key_lens[type] != OVS_KEY_LEN_VARIABLE) {
                        OVS_NLERR("IPv4 tunnel attribute type has unexpected "
                                  " length (type=%d, length=%d, 
expected=%d).\n",
                                  type, nla_len(a), ovs_tunnel_key_lens[type]);
@@ -819,26 +821,22 @@ static int ovs_key_from_nlattrs(struct sw_flow_match 
*match, u64 attrs,
        return 0;
 }
 
-static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
+static void nlattr_set(struct nlattr *attr, u8 val)
 {
        struct nlattr *nla;
        int rem;
 
        /* The nlattr stream should already have been validated */
        nla_for_each_nested(nla, attr, rem) {
-               /* We assume that ovs_key_lens[type] == -1 means that type is a
-                * nested attribute
-                */
-               if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
-                       nlattr_set(nla, val, false);
-               else
+               int len = ovs_key_lens[nla_type(nla)];
+
+               if (len > 0 || len == OVS_KEY_LEN_NESTED)
                        memset(nla_data(nla), val, nla_len(nla));
-       }
-}
+               else if (len == OVS_KEY_LEN_NESTED)
+                       nlattr_set(nla, val);
 
-static void mask_set_nlattr(struct nlattr *attr, u8 val)
-{
-       nlattr_set(attr, val, true);
+               BUG_ON(len != 0);
+       }
 }
 
 /**
@@ -925,7 +923,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
                        if (!newmask)
                                return -ENOMEM;
 
-                       mask_set_nlattr(newmask, 0xff);
+                       nlattr_set(newmask, 0xff);
 
                        /* The userspace does not send tunnel attributes that
                         * are 0, but we should not wildcard them nonetheless.
@@ -1529,7 +1527,7 @@ static int validate_set(const struct nlattr *a,
 
        if (key_type > OVS_KEY_ATTR_MAX ||
            (ovs_key_lens[key_type] != nla_len(ovs_key) &&
-            ovs_key_lens[key_type] != -1))
+            ovs_key_lens[key_type] != OVS_KEY_LEN_NESTED))
                return -EINVAL;
 
        switch (key_type) {
@@ -1661,17 +1659,20 @@ static int __ovs_nla_copy_actions(const struct nlattr 
*attr,
                return -EOVERFLOW;
 
        nla_for_each_nested(a, attr, rem) {
-               /* Expected argument lengths, (u32)-1 for variable length. */
+               /* Expected argument lengths, or OVS_KEY_LEN_NETSTED for
+                * nested actions attributes. */
                static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
                        [OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
                        [OVS_ACTION_ATTR_RECIRC] = sizeof(u32),
-                       [OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
-                       [OVS_ACTION_ATTR_PUSH_MPLS] = sizeof(struct 
ovs_action_push_mpls),
+                       [OVS_ACTION_ATTR_USERSPACE] = (u32)OVS_KEY_LEN_NESTED,
+                       [OVS_ACTION_ATTR_PUSH_MPLS] =
+                               sizeof(struct ovs_action_push_mpls),
                        [OVS_ACTION_ATTR_POP_MPLS] = sizeof(__be16),
-                       [OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct 
ovs_action_push_vlan),
+                       [OVS_ACTION_ATTR_PUSH_VLAN] =
+                               sizeof(struct ovs_action_push_vlan),
                        [OVS_ACTION_ATTR_POP_VLAN] = 0,
-                       [OVS_ACTION_ATTR_SET] = (u32)-1,
-                       [OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
+                       [OVS_ACTION_ATTR_SET] = (u32)OVS_KEY_LEN_NESTED,
+                       [OVS_ACTION_ATTR_SAMPLE] = (u32)OVS_KEY_LEN_NESTED,
                        [OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
                };
                const struct ovs_action_push_vlan *vlan;
@@ -1680,7 +1681,7 @@ static int __ovs_nla_copy_actions(const struct nlattr 
*attr,
 
                if (type > OVS_ACTION_ATTR_MAX ||
                    (action_lens[type] != nla_len(a) &&
-                    action_lens[type] != (u32)-1))
+                    action_lens[type] != (u32)OVS_KEY_LEN_NESTED))
                        return -EINVAL;
 
                skip_copy = false;
diff --git a/datapath/flow_netlink.h b/datapath/flow_netlink.h
index 8ac40b5..d8db30b 100644
--- a/datapath/flow_netlink.h
+++ b/datapath/flow_netlink.h
@@ -56,4 +56,10 @@ int ovs_nla_put_actions(const struct nlattr *attr,
 
 void ovs_nla_free_flow_actions(struct sw_flow_actions *);
 
+enum {
+       /* values >= 0 are fixed key len */
+       OVS_KEY_LEN_NESTED = -1,
+       OVS_KEY_LEN_VARIABLE = -2,
+};
+
 #endif /* flow_netlink.h */
-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to