[ovs-dev] [PATCH 2/2] datapath: Use nla_parse_strict() for netlink parsing.

Joe Stringer joestringer at nicira.com
Tue Aug 26 23:50:31 UTC 2014


Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
 datapath/flow_netlink.c |  219 +++++++++++++++++++----------------------------
 1 file changed, 86 insertions(+), 133 deletions(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 69d1919..aa2cece 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -287,161 +287,121 @@ size_t ovs_key_attr_size(void)
 }
 
 /* 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_PRIORITY] = sizeof(u32),
-	[OVS_KEY_ATTR_IN_PORT] = sizeof(u32),
-	[OVS_KEY_ATTR_SKB_MARK] = sizeof(u32),
-	[OVS_KEY_ATTR_ETHERNET] = sizeof(struct ovs_key_ethernet),
-	[OVS_KEY_ATTR_VLAN] = sizeof(__be16),
-	[OVS_KEY_ATTR_ETHERTYPE] = sizeof(__be16),
-	[OVS_KEY_ATTR_IPV4] = sizeof(struct ovs_key_ipv4),
-	[OVS_KEY_ATTR_IPV6] = sizeof(struct ovs_key_ipv6),
-	[OVS_KEY_ATTR_TCP] = sizeof(struct ovs_key_tcp),
-	[OVS_KEY_ATTR_TCP_FLAGS] = sizeof(__be16),
-	[OVS_KEY_ATTR_UDP] = sizeof(struct ovs_key_udp),
-	[OVS_KEY_ATTR_SCTP] = sizeof(struct ovs_key_sctp),
-	[OVS_KEY_ATTR_ICMP] = sizeof(struct ovs_key_icmp),
-	[OVS_KEY_ATTR_ICMPV6] = sizeof(struct ovs_key_icmpv6),
-	[OVS_KEY_ATTR_ARP] = sizeof(struct ovs_key_arp),
-	[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_MPLS] = sizeof(struct ovs_key_mpls),
+static const struct nla_policy ovs_key_policy[OVS_KEY_ATTR_MAX + 1] = {
+	[OVS_KEY_ATTR_ENCAP] = { .type = NLA_NESTED },
+	[OVS_KEY_ATTR_PRIORITY] = { .len = sizeof(u32) },
+	[OVS_KEY_ATTR_IN_PORT] = { .len = sizeof(u32) },
+	[OVS_KEY_ATTR_SKB_MARK] = { .len = sizeof(u32) },
+	[OVS_KEY_ATTR_ETHERNET] = { .len = sizeof(struct ovs_key_ethernet) },
+	[OVS_KEY_ATTR_VLAN] = { .len = sizeof(__be16) },
+	[OVS_KEY_ATTR_ETHERTYPE] = { .len = sizeof(__be16) },
+	[OVS_KEY_ATTR_IPV4] = { .len = sizeof(struct ovs_key_ipv4) },
+	[OVS_KEY_ATTR_IPV6] = { .len = sizeof(struct ovs_key_ipv6) },
+	[OVS_KEY_ATTR_TCP] = { .len = sizeof(struct ovs_key_tcp) },
+	[OVS_KEY_ATTR_TCP_FLAGS] = { .len = sizeof(__be16) },
+	[OVS_KEY_ATTR_UDP] = { .len = sizeof(struct ovs_key_udp) },
+	[OVS_KEY_ATTR_SCTP] = { .len = sizeof(struct ovs_key_sctp) },
+	[OVS_KEY_ATTR_ICMP] = { .len = sizeof(struct ovs_key_icmp) },
+	[OVS_KEY_ATTR_ICMPV6] = { .len = sizeof(struct ovs_key_icmpv6) },
+	[OVS_KEY_ATTR_ARP] = { .len = sizeof(struct ovs_key_arp) },
+	[OVS_KEY_ATTR_ND] = { .len = sizeof(struct ovs_key_nd) },
+	[OVS_KEY_ATTR_DP_HASH] = { .len = sizeof(u32) },
+	[OVS_KEY_ATTR_RECIRC_ID] = { .len = sizeof(u32) },
+	[OVS_KEY_ATTR_TUNNEL] = { .type = NLA_NESTED },
+	[OVS_KEY_ATTR_MPLS] = { .len = sizeof(struct ovs_key_mpls) },
 };
 
-static bool is_all_zero(const u8 *fp, size_t size)
+static int parse_nlattrs(const struct nlattr *attr,
+			 const struct nlattr *a[], int maxtype,
+			 const struct nla_policy *policy,
+			 u64 *attrsp, bool dup, bool nz)
 {
-	int i;
-
-	if (!fp)
-		return false;
-
-	for (i = 0; i < size; i++)
-		if (fp[i])
-			return false;
-
-	return true;
-}
-
-static int __parse_flow_nlattrs(const struct nlattr *attr,
-				const struct nlattr *a[],
-				u64 *attrsp, bool nz)
-{
-	const struct nlattr *nla;
-	u64 attrs;
-	int rem;
-
-	attrs = *attrsp;
-	nla_for_each_nested(nla, attr, rem) {
-		u16 type = nla_type(nla);
-		int expected_len;
-
-		if (type > OVS_KEY_ATTR_MAX) {
-			OVS_NLERR("Unknown key attribute (type=%d, max=%d).\n",
-				  type, OVS_KEY_ATTR_MAX);
-			return -EINVAL;
-		}
-
-		if (attrs & (1ULL << type)) {
-			OVS_NLERR("Duplicate key attribute (type %d).\n", type);
-			return -EINVAL;
-		}
-
-		expected_len = ovs_key_lens[type];
-		if (nla_len(nla) != expected_len && expected_len != -1) {
-			OVS_NLERR("Key attribute has unexpected length (type=%d"
-				  ", length=%d, expected=%d).\n", type,
-				  nla_len(nla), expected_len);
-			return -EINVAL;
-		}
+	u64 attrs = 0;
+	int i, err;
 
-		if (!nz || !is_all_zero(nla_data(nla), expected_len)) {
-			attrs |= 1ULL << type;
-			a[type] = nla;
-		}
-	}
-	if (rem) {
-		OVS_NLERR("Message has %d unknown bytes.\n", rem);
-		return -EINVAL;
-	}
+	err = nla_parse_strict(a, maxtype, nla_data(attr), nla_len(attr),
+			       policy, dup, nz);
+	if (err)
+		return err;
 
+	for (i = 0; i < maxtype; i++)
+		if (a[i])
+			attrs |= (1ULL << i);
 	*attrsp = attrs;
+
 	return 0;
 }
 
 static int parse_flow_mask_nlattrs(const struct nlattr *attr,
 				   const struct nlattr *a[], u64 *attrsp)
 {
-	return __parse_flow_nlattrs(attr, a, attrsp, true);
+	return parse_nlattrs(attr, a, OVS_KEY_ATTR_MAX, ovs_key_policy,
+			     attrsp, false, true);
 }
 
 static int parse_flow_nlattrs(const struct nlattr *attr,
 			      const struct nlattr *a[], u64 *attrsp)
 {
-	return __parse_flow_nlattrs(attr, a, attrsp, false);
+	return parse_nlattrs(attr, a, OVS_KEY_ATTR_MAX, ovs_key_policy,
+			     attrsp, false, false);
 }
 
 static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 				struct sw_flow_match *match, bool is_mask)
 {
-	struct nlattr *a;
-	int rem;
+	static const struct nla_policy ovs_tunnel_key_policy[OVS_TUNNEL_KEY_ATTR_MAX + 1] = {
+		[OVS_TUNNEL_KEY_ATTR_ID] = { .len = sizeof(u64) },
+		[OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = { .len = sizeof(u32) },
+		[OVS_TUNNEL_KEY_ATTR_IPV4_DST] = { .len = sizeof(u32) },
+		[OVS_TUNNEL_KEY_ATTR_TOS] = { .len = 1 },
+		[OVS_TUNNEL_KEY_ATTR_TTL] = { .len = 1 },
+		[OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT] = { .len = 0 },
+		[OVS_TUNNEL_KEY_ATTR_CSUM] = { .len = 0 },
+		[OVS_TUNNEL_KEY_ATTR_TP_SRC] = { .len = sizeof(u16) },
+		[OVS_TUNNEL_KEY_ATTR_TP_DST] = { .len = sizeof(u16) },
+		[OVS_TUNNEL_KEY_ATTR_OAM] = { .len = 0 },
+		[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = { .type = NLA_NESTED },
+	};
+	const struct nlattr *a[OVS_TUNNEL_KEY_ATTR_MAX + 1];
 	bool ttl = false;
 	__be16 tun_flags = 0;
+	enum ovs_tunnel_key_attr type;
+	u64 attrs = 0;
+	int err;
 
-	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_TP_SRC] = sizeof(u16),
-			[OVS_TUNNEL_KEY_ATTR_TP_DST] = sizeof(u16),
-			[OVS_TUNNEL_KEY_ATTR_OAM] = 0,
-			[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS] = -1,
-		};
+	err = parse_nlattrs(attr, a, OVS_TUNNEL_KEY_ATTR_MAX,
+			    ovs_tunnel_key_policy, &attrs, true, is_mask);
+	if (err)
+		return err;
 
-		if (type > OVS_TUNNEL_KEY_ATTR_MAX) {
-			OVS_NLERR("Unknown IPv4 tunnel attribute (type=%d, max=%d).\n",
-			type, OVS_TUNNEL_KEY_ATTR_MAX);
-			return -EINVAL;
-		}
+	for (type = 0; type <= OVS_TUNNEL_KEY_ATTR_MAX; type++) {
+		const struct nlattr *nla;
 
-		if (ovs_tunnel_key_lens[type] != nla_len(a) &&
-		    ovs_tunnel_key_lens[type] != -1) {
-			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]);
-			return -EINVAL;
-		}
+		if (!(attrs & (1ULL << type)))
+			continue;
 
+		nla = a[type];
 		switch (type) {
 		case OVS_TUNNEL_KEY_ATTR_ID:
 			SW_FLOW_KEY_PUT(match, tun_key.tun_id,
-					nla_get_be64(a), is_mask);
+					nla_get_be64(nla), is_mask);
 			tun_flags |= TUNNEL_KEY;
 			break;
 		case OVS_TUNNEL_KEY_ATTR_IPV4_SRC:
 			SW_FLOW_KEY_PUT(match, tun_key.ipv4_src,
-					nla_get_be32(a), is_mask);
+					nla_get_be32(nla), is_mask);
 			break;
 		case OVS_TUNNEL_KEY_ATTR_IPV4_DST:
 			SW_FLOW_KEY_PUT(match, tun_key.ipv4_dst,
-					nla_get_be32(a), is_mask);
+					nla_get_be32(nla), is_mask);
 			break;
 		case OVS_TUNNEL_KEY_ATTR_TOS:
 			SW_FLOW_KEY_PUT(match, tun_key.ipv4_tos,
-					nla_get_u8(a), is_mask);
+					nla_get_u8(nla), is_mask);
 			break;
 		case OVS_TUNNEL_KEY_ATTR_TTL:
 			SW_FLOW_KEY_PUT(match, tun_key.ipv4_ttl,
-					nla_get_u8(a), is_mask);
+					nla_get_u8(nla), is_mask);
 			ttl = true;
 			break;
 		case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT:
@@ -452,29 +412,29 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 			break;
 		case OVS_TUNNEL_KEY_ATTR_TP_SRC:
 			SW_FLOW_KEY_PUT(match, tun_key.tp_src,
-					nla_get_be16(a), is_mask);
+					nla_get_be16(nla), is_mask);
 			break;
 		case OVS_TUNNEL_KEY_ATTR_TP_DST:
 			SW_FLOW_KEY_PUT(match, tun_key.tp_dst,
-					nla_get_be16(a), is_mask);
+					nla_get_be16(nla), is_mask);
 			break;
 		case OVS_TUNNEL_KEY_ATTR_OAM:
 			tun_flags |= TUNNEL_OAM;
 			break;
 		case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
 			tun_flags |= TUNNEL_OPTIONS_PRESENT;
-			if (nla_len(a) > sizeof(match->key->tun_opts)) {
+			if (nla_len(nla) > sizeof(match->key->tun_opts)) {
 				OVS_NLERR("Geneve option length exceeds "
 					  "maximum size (len %d, max %zu).\n",
-					  nla_len(a),
+					  nla_len(nla),
 					  sizeof(match->key->tun_opts));
 				return -EINVAL;
 			}
 
-			if (nla_len(a) % 4 != 0) {
+			if (nla_len(nla) % 4 != 0) {
 				OVS_NLERR("Geneve option length is not "
 					  "a multiple of 4 (len %d).\n",
-					  nla_len(a));
+					  nla_len(nla));
 				return -EINVAL;
 			}
 
@@ -483,7 +443,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 			 * additional options will be silently matched.
 			 */
 			if (!is_mask) {
-				SW_FLOW_KEY_PUT(match, tun_opts_len, nla_len(a),
+				SW_FLOW_KEY_PUT(match, tun_opts_len, nla_len(nla),
 						false);
 			} else {
 				/* This is somewhat unusual because it looks at
@@ -496,10 +456,10 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 				 * variable length and we won't have the
 				 * information later.
 				 */
-				if (match->key->tun_opts_len != nla_len(a)) {
+				if (match->key->tun_opts_len != nla_len(nla)) {
 					OVS_NLERR("Geneve option key length (%d)"
 					   " is different from mask length (%d).",
-					   match->key->tun_opts_len, nla_len(a));
+					   match->key->tun_opts_len, nla_len(nla));
 					return -EINVAL;
 				}
 
@@ -509,8 +469,8 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 
 			SW_FLOW_KEY_MEMCPY_OFFSET(match,
 				(unsigned long)GENEVE_OPTS((struct sw_flow_key *)0,
-							   nla_len(a)),
-				nla_data(a), nla_len(a), is_mask);
+							   nla_len(nla)),
+				nla_data(nla), nla_len(nla), is_mask);
 			break;
 		default:
 			OVS_NLERR("Unknown IPv4 tunnel attribute (%d).\n", type);
@@ -520,11 +480,6 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 
 	SW_FLOW_KEY_PUT(match, tun_key.tun_flags, tun_flags, is_mask);
 
-	if (rem > 0) {
-		OVS_NLERR("IPv4 tunnel attribute has %d unknown bytes.\n", rem);
-		return -EINVAL;
-	}
-
 	if (!is_mask) {
 		if (!match->key->tun_key.ipv4_dst) {
 			OVS_NLERR("IPv4 tunnel destination address is zero.\n");
@@ -903,10 +858,8 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
 
 	/* 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)
+		if (is_attr_mask_key &&
+		    ovs_key_policy[nla_type(nla)].type == NLA_NESTED)
 			nlattr_set(nla, val, false);
 		else
 			memset(nla_data(nla), val, nla_len(nla));
@@ -1605,8 +1558,8 @@ static int validate_set(const struct nlattr *a,
 		return -EINVAL;
 
 	if (key_type > OVS_KEY_ATTR_MAX ||
-	    (ovs_key_lens[key_type] != nla_len(ovs_key) &&
-	     ovs_key_lens[key_type] != -1))
+	    (ovs_key_policy[key_type].len != nla_len(ovs_key) &&
+	     ovs_key_policy[key_type].type != NLA_NESTED))
 		return -EINVAL;
 
 	switch (key_type) {
-- 
1.7.10.4




More information about the dev mailing list