[ovs-dev] [v3] datapath: Distinguish between nested and variable length attributes

Andy Zhou azhou at nicira.com
Wed Aug 13 05:18:39 UTC 2014


-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 support ovs netlink key attributes in a more
general way.

Signed-off-by: Andy Zhou <azhou at nicira.com>
Acked-by: Daniele Di Proietto <ddiproietto at vmware.com>

---
V3: address comments form Daniele
---
 datapath/flow_netlink.c | 88 ++++++++++++++++++++++++++++---------------------
 datapath/flow_netlink.h |  6 ++++
 2 files changed, 57 insertions(+), 37 deletions(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index e525c9d..522303d 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,10 +267,24 @@ 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),
 };
 
+/* The size of the argument for each %OVS_TUNNEL_KEY_ATTR_* Netlink
+ * attribute.  */
+static const int ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] = {
+	[OVS_TUNNEL_KEY_ATTR_ID] = sizeof(u64),
+	[OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = sizeof(u32),
+	[OVS_TUNNEL_KEY_ATTR_IPV4_DST] = sizeof(u32),
+	[OVS_TUNNEL_KEY_ATTR_TOS] = 1,
+	[OVS_TUNNEL_KEY_ATTR_TTL] = 1,
+	[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] = OVS_KEY_LEN_VARIABLE,
+};
+
 static bool is_all_zero(const u8 *fp, size_t size)
 {
 	int i;
@@ -310,7 +324,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);
@@ -353,17 +368,6 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 
 	nla_for_each_nested(a, attr, rem) {
 		int type = nla_type(a);
-		static const u32 ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] = {
-			[OVS_TUNNEL_KEY_ATTR_ID] = sizeof(u64),
-			[OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = sizeof(u32),
-			[OVS_TUNNEL_KEY_ATTR_IPV4_DST] = sizeof(u32),
-			[OVS_TUNNEL_KEY_ATTR_TOS] = 1,
-			[OVS_TUNNEL_KEY_ATTR_TTL] = 1,
-			[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,
-		};
 
 		if (type > OVS_TUNNEL_KEY_ATTR_MAX) {
 			OVS_NLERR("Unknown IPv4 tunnel attribute (type=%d, max=%d).\n",
@@ -372,7 +376,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 +823,33 @@ 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, const int key_lens[])
 {
 	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 = key_lens[nla_type(nla)];
+
+		if (len > 0 || len == OVS_KEY_LEN_VARIABLE)
 			memset(nla_data(nla), val, nla_len(nla));
-	}
-}
+		else if ((len == OVS_KEY_LEN_NESTED) &&
+			 (key_lens == ovs_key_lens))
+				switch(nla_type(nla)) {
+				case OVS_KEY_ATTR_ENCAP:
+					nlattr_set(attr, val, ovs_key_lens);
+					return;
+
+				case OVS_KEY_ATTR_TUNNEL:
+					nlattr_set(attr, val,
+						   ovs_tunnel_key_lens);
+					return;
 
-static void mask_set_nlattr(struct nlattr *attr, u8 val)
-{
-	nlattr_set(attr, val, true);
+				}
+
+		BUG_ON(len != 0);
+	}
 }
 
 /**
@@ -925,7 +936,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
 			if (!newmask)
 				return -ENOMEM;
 
-			mask_set_nlattr(newmask, 0xff);
+			nlattr_set(newmask, 0xff, ovs_key_lens);
 
 			/* The userspace does not send tunnel attributes that
 			 * are 0, but we should not wildcard them nonetheless.
@@ -1529,7 +1540,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 +1672,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. */
-		static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
+		/* Expected argument lengths, or OVS_KEY_LEN_NETSTED for
+		 * nested actions attributes. */
+		static const int 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] = 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] = OVS_KEY_LEN_NESTED,
+			[OVS_ACTION_ATTR_SAMPLE] = OVS_KEY_LEN_NESTED,
 			[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
 		};
 		const struct ovs_action_push_vlan *vlan;
@@ -1680,7 +1694,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




More information about the dev mailing list