[ovs-dev] [PATCH] datapath: More flexible kernel/userspace tunneling attribute.

Pravin B Shelar pshelar at nicira.com
Sat Jan 12 01:23:00 UTC 2013


Following patch breaks down single ipv4_tunnel netlink attribute into
individual member attributes. It will help when we extend tunneling
parameters in future.

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
Bug #14611
---
 datapath/datapath.c         |  144 +++++++++++++++++++----------
 datapath/flow.c             |  141 +++++++++++++++++++++--------
 datapath/flow.h             |   13 +++
 datapath/tunnel.h           |    1 -
 include/linux/openvswitch.h |   36 ++++----
 lib/dpif-netdev.c           |    2 +-
 lib/flow.c                  |    1 -
 lib/flow.h                  |    4 +-
 lib/odp-util.c              |  211 +++++++++++++++++++++++++++----------------
 tests/test-bundle.c         |    1 -
 tests/test-multipath.c      |    1 -
 11 files changed, 373 insertions(+), 182 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 30e26a7..4ed00cf 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -423,11 +423,13 @@ static int flush_flows(struct datapath *dp)
 	return 0;
 }
 
-static int validate_actions(const struct nlattr *attr,
-				const struct sw_flow_key *key, int depth);
+static int validate_and_copy_actions(const struct nlattr *attr,
+				const struct sw_flow_key *key, int depth,
+				struct sw_flow_actions *sfa);
 
 static int validate_sample(const struct nlattr *attr,
-				const struct sw_flow_key *key, int depth)
+			   const struct sw_flow_key *key, int depth,
+			   struct sw_flow_actions *sfa)
 {
 	const struct nlattr *attrs[OVS_SAMPLE_ATTR_MAX + 1];
 	const struct nlattr *probability, *actions;
@@ -451,7 +453,7 @@ static int validate_sample(const struct nlattr *attr,
 	actions = attrs[OVS_SAMPLE_ATTR_ACTIONS];
 	if (!actions || (nla_len(actions) && nla_len(actions) < NLA_HDRLEN))
 		return -EINVAL;
-	return validate_actions(actions, key, depth + 1);
+	return validate_and_copy_actions(actions, key, depth + 1, sfa);
 }
 
 static int validate_tp_port(const struct sw_flow_key *flow_key)
@@ -467,8 +469,41 @@ static int validate_tp_port(const struct sw_flow_key *flow_key)
 	return -EINVAL;
 }
 
+static int ____nla_put(struct sw_flow_actions *sfa, int attrtype, void *data, int len)
+{
+	struct nlattr *a = (struct nlattr *) ((unsigned char *)sfa->actions + sfa->used);
+
+	if (NLA_ALIGN(nla_attr_size(len)) > (sfa->actions_len - sfa->used));
+		return -EMSGSIZE;
+
+	a->nla_type = attrtype;
+	a->nla_len = nla_attr_size(len);
+
+	memset((unsigned char *) a + a->nla_len, 0, nla_padlen(len));
+	memcpy(nla_data(a), data, len);
+	sfa->used += NLA_ALIGN(nla_attr_size(len));
+}
+
+static int validate_and_copy_set_tun(const struct nlattr *attr,
+				     struct sw_flow_actions *sfa)
+{
+	struct ovs_key_ipv4_tunnel tun_key;
+	int err;
+
+	err = ipv4_tun_from_nlattr(attr, &tun_key);
+	if (err)
+		return err;
+
+	err = ____nla_put(sfa, OVS_KEY_ATTR_IPV4_TUNNEL, &tun_key, sizeof(tun_key));
+	if (err)
+		return err;
+	return 0;
+}
+
 static int validate_set(const struct nlattr *a,
-			const struct sw_flow_key *flow_key)
+			const struct sw_flow_key *flow_key,
+			struct sw_flow_actions *sfa,
+			bool *set_tun)
 {
 	const struct nlattr *ovs_key = nla_data(a);
 	int key_type = nla_type(ovs_key);
@@ -478,13 +513,14 @@ static int validate_set(const struct nlattr *a,
 		return -EINVAL;
 
 	if (key_type > OVS_KEY_ATTR_MAX ||
-	    nla_len(ovs_key) != ovs_key_lens[key_type])
+	    (ovs_key_lens[key_type] != nla_len(ovs_key) &&
+	     ovs_key_lens[key_type] != -1))
 		return -EINVAL;
 
 	switch (key_type) {
 	const struct ovs_key_ipv4 *ipv4_key;
-	const struct ovs_key_ipv4_tunnel *tun_key;
 	const struct ovs_key_ipv6 *ipv6_key;
+	int err;
 
 	case OVS_KEY_ATTR_PRIORITY:
 	case OVS_KEY_ATTR_TUN_ID:
@@ -498,10 +534,11 @@ static int validate_set(const struct nlattr *a,
 #endif
 		break;
 
-	case OVS_KEY_ATTR_IPV4_TUNNEL:
-		tun_key = nla_data(ovs_key);
-		if (!tun_key->ipv4_dst)
-			return -EINVAL;
+	case OVS_KEY_ATTR_TUNNEL:
+		*set_tun = true;
+		err = validate_and_copy_set_tun(a, sfa);
+		if (err)
+			return err;
 		break;
 
 	case OVS_KEY_ATTR_IPV4:
@@ -579,8 +616,24 @@ static int validate_userspace(const struct nlattr *attr)
 	return 0;
 }
 
-static int validate_actions(const struct nlattr *attr,
-				const struct sw_flow_key *key,  int depth)
+static int copy_attr(const struct nlattr *from,
+		      struct sw_flow_actions *sfa)
+{
+	unsigned char *to = (unsigned char *)sfa->actions + sfa->used;
+	int totlen = NLA_ALIGN(from->nla_len);
+
+	if (totlen > (sfa->actions_len - sfa->used))
+		return -EMSGSIZE;
+
+	memcpy(to, from, totlen);
+	sfa->used += totlen;
+	return 0;
+}
+
+static int validate_and_copy_actions(const struct nlattr *attr,
+				const struct sw_flow_key *key,
+				int depth,
+				struct sw_flow_actions *sfa)
 {
 	const struct nlattr *a;
 	int rem, err;
@@ -600,12 +653,16 @@ static int validate_actions(const struct nlattr *attr,
 		};
 		const struct ovs_action_push_vlan *vlan;
 		int type = nla_type(a);
+		bool set_tun;
+
 
 		if (type > OVS_ACTION_ATTR_MAX ||
 		    (action_lens[type] != nla_len(a) &&
 		     action_lens[type] != (u32)-1))
 			return -EINVAL;
 
+		set_tun = false;
+
 		switch (type) {
 		case OVS_ACTION_ATTR_UNSPEC:
 			return -EINVAL;
@@ -634,13 +691,13 @@ static int validate_actions(const struct nlattr *attr,
 			break;
 
 		case OVS_ACTION_ATTR_SET:
-			err = validate_set(a, key);
+			err = validate_set(a, key, sfa, &set_tun);
 			if (err)
 				return err;
 			break;
 
 		case OVS_ACTION_ATTR_SAMPLE:
-			err = validate_sample(a, key, depth);
+			err = validate_sample(a, key, depth, sfa);
 			if (err)
 				return err;
 			break;
@@ -648,6 +705,11 @@ static int validate_actions(const struct nlattr *attr,
 		default:
 			return -EINVAL;
 		}
+		if (!set_tun) {
+			err = copy_attr(a, sfa);
+			if (err)
+				return err;
+		}
 	}
 
 	if (rem > 0)
@@ -716,17 +778,16 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_flow_metadata_from_nlattrs(flow, key_len, a[OVS_PACKET_ATTR_KEY]);
 	if (err)
 		goto err_flow_free;
-
-	err = validate_actions(a[OVS_PACKET_ATTR_ACTIONS], &flow->key, 0);
-	if (err)
-		goto err_flow_free;
-
 	acts = ovs_flow_actions_alloc(a[OVS_PACKET_ATTR_ACTIONS]);
 	err = PTR_ERR(acts);
 	if (IS_ERR(acts))
 		goto err_flow_free;
 	rcu_assign_pointer(flow->sf_acts, acts);
 
+	err = validate_and_copy_actions(a[OVS_PACKET_ATTR_ACTIONS], &flow->key, 0, acts);
+	if (err)
+		goto err_flow_free;
+
 	OVS_CB(packet)->flow = flow;
 	packet->priority = flow->key.phy.priority;
 	skb_set_mark(packet, flow->key.phy.skb_mark);
@@ -938,6 +999,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 	struct sk_buff *reply;
 	struct datapath *dp;
 	struct flow_table *table;
+	struct sw_flow_actions *acts = NULL;
 	int error;
 	int key_len;
 
@@ -951,9 +1013,16 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 
 	/* Validate actions. */
 	if (a[OVS_FLOW_ATTR_ACTIONS]) {
-		error = validate_actions(a[OVS_FLOW_ATTR_ACTIONS], &key,  0);
-		if (error)
+		acts = ovs_flow_actions_alloc(a[OVS_FLOW_ATTR_ACTIONS]);
+		error = PTR_ERR(acts);
+		if (IS_ERR(acts))
+			goto error;
+
+		error = validate_and_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &key,  0, acts);
+		if (error) {
+			ovs_flow_deferred_free_acts(acts);
 			goto error;
+		}
 	} else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
 		error = -EINVAL;
 		goto error;
@@ -967,8 +1036,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 	table = genl_dereference(dp->table);
 	flow = ovs_flow_tbl_lookup(table, &key, key_len);
 	if (!flow) {
-		struct sw_flow_actions *acts;
-
 		/* Bail out if we're not allowed to create a new flow. */
 		error = -ENOENT;
 		if (info->genlhdr->cmd == OVS_FLOW_CMD_SET)
@@ -994,11 +1061,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 		}
 		clear_stats(flow);
 
-		/* Obtain actions. */
-		acts = ovs_flow_actions_alloc(a[OVS_FLOW_ATTR_ACTIONS]);
-		error = PTR_ERR(acts);
-		if (IS_ERR(acts))
-			goto error_free_flow;
 		rcu_assign_pointer(flow->sf_acts, acts);
 
 		/* Put flow in bucket. */
@@ -1010,7 +1072,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 	} else {
 		/* We found a matching flow. */
 		struct sw_flow_actions *old_acts;
-		struct nlattr *acts_attrs;
 
 		/* Bail out if we're not allowed to modify an existing flow.
 		 * We accept NLM_F_CREATE in place of the intended NLM_F_EXCL
@@ -1026,21 +1087,14 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 		/* Update actions. */
 		old_acts = rcu_dereference_protected(flow->sf_acts,
 						     lockdep_genl_is_held());
-		acts_attrs = a[OVS_FLOW_ATTR_ACTIONS];
-		if (acts_attrs &&
-		   (old_acts->actions_len != nla_len(acts_attrs) ||
-		   memcmp(old_acts->actions, nla_data(acts_attrs),
-			  old_acts->actions_len))) {
-			struct sw_flow_actions *new_acts;
-
-			new_acts = ovs_flow_actions_alloc(acts_attrs);
-			error = PTR_ERR(new_acts);
-			if (IS_ERR(new_acts))
-				goto error;
-
-			rcu_assign_pointer(flow->sf_acts, new_acts);
+		if (acts &&
+		   (old_acts->actions_len != acts->actions_len ||
+		    memcmp(old_acts->actions, acts->actions,
+			   old_acts->actions_len))) {
+			rcu_assign_pointer(flow->sf_acts, acts);
 			ovs_flow_deferred_free_acts(old_acts);
-		}
+		} else
+			ovs_flow_deferred_free_acts(acts);
 
 		reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
 					       info->snd_seq, OVS_FLOW_CMD_NEW);
@@ -1062,8 +1116,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 				ovs_dp_flow_multicast_group.id,	PTR_ERR(reply));
 	return 0;
 
-error_free_flow:
-	ovs_flow_free(flow);
 error:
 	return error;
 }
diff --git a/datapath/flow.c b/datapath/flow.c
index 63eef77..ce563f3 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -213,7 +213,7 @@ struct sw_flow_actions *ovs_flow_actions_alloc(const struct nlattr *actions)
 		return ERR_PTR(-ENOMEM);
 
 	sfa->actions_len = actions_len;
-	memcpy(sfa->actions, nla_data(actions), actions_len);
+	sfa->used = 0;
 	return sfa;
 }
 
@@ -848,7 +848,7 @@ const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
 	[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_IPV4_TUNNEL] = sizeof(struct ovs_key_ipv4_tunnel),
+	[OVS_KEY_ATTR_TUNNEL] = -1,
 
 	/* Not upstream. */
 	[OVS_KEY_ATTR_TUN_ID] = sizeof(__be64),
@@ -989,6 +989,81 @@ static int parse_flow_nlattrs(const struct nlattr *attr,
 	return 0;
 }
 
+int ipv4_tun_from_nlattr(const struct nlattr *attr,
+			 struct ovs_key_ipv4_tunnel *tun_key)
+{
+	struct nlattr *a;
+	int rem;
+
+	memset(tun_key, 0, sizeof(*tun_key));
+
+	nla_for_each_nested(a, attr, rem) {
+		int type = nla_type(a);
+
+		switch (type) {
+		case OVS_KEY_ATTR_TUN_ID:
+			memcpy(&tun_key->tun_id, nla_data(a), sizeof(__be64));
+		break;
+		case OVS_TUNNEL_IPV4_SRC:
+			memcpy(&tun_key->ipv4_src, nla_data(a), sizeof(__be32));
+		break;
+		case OVS_TUNNEL_IPV4_DST:
+			memcpy(&tun_key->ipv4_dst, nla_data(a), sizeof(__be32));
+		break;
+		case OVS_TUNNEL_TOS:
+			tun_key->ipv4_tos = nla_get_u8(a);
+		break;
+		case OVS_TUNNEL_TTL:
+			tun_key->ipv4_ttl = nla_get_u8(a);
+		break;
+		case OVS_TUNNEL_FLAGS:
+			tun_key->tun_flags = nla_get_u32(a);
+		break;
+		default:
+			return -EINVAL;
+
+		}
+	}
+	if (rem > 0)
+		return -EINVAL;
+
+	if (!tun_key->ipv4_dst)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ipv4_tun_to_nlattr(struct sk_buff *skb, const struct ovs_key_ipv4_tunnel *tun_key)
+{
+	struct nlattr *nla;
+
+	nla = nla_nest_start(skb, OVS_KEY_ATTR_TUNNEL);
+	if (!nla)
+		return -EMSGSIZE;
+
+	if (tun_key->tun_flags & OVS_TNL_F_KEY &&
+	    nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, tun_key->tun_id))
+		return -EMSGSIZE;
+	if (tun_key->ipv4_src &&
+	    nla_put_be32(skb, OVS_TUNNEL_IPV4_SRC, tun_key->ipv4_src))
+		return -EMSGSIZE;
+	if (tun_key->ipv4_dst &&
+	    nla_put_be32(skb, OVS_TUNNEL_IPV4_DST, tun_key->ipv4_dst))
+		return -EMSGSIZE;
+	if (tun_key->ipv4_tos &&
+	    nla_put_u8(skb, OVS_TUNNEL_TOS, tun_key->ipv4_tos))
+		return -EMSGSIZE;
+	if (tun_key->ipv4_ttl &&
+	    nla_put_u8(skb, OVS_TUNNEL_TTL, tun_key->ipv4_ttl))
+		return -EMSGSIZE;
+	if (tun_key->tun_flags &&
+	    nla_put_u32(skb, OVS_TUNNEL_FLAGS, tun_key->tun_flags))
+		return -EMSGSIZE;
+
+	nla_nest_end(skb, nla);
+	return 0;
+}
+
 /**
  * ovs_flow_from_nlattrs - parses Netlink attributes into a flow key.
  * @swkey: receives the extracted flow key.
@@ -1037,37 +1112,29 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
 	}
 
 	if (attrs & (1ULL << OVS_KEY_ATTR_TUN_ID) &&
-	    attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) {
-		struct ovs_key_ipv4_tunnel *tun_key;
+	    attrs & (1ULL << OVS_KEY_ATTR_TUNNEL)) {
 		__be64 tun_id;
 
-		tun_key = nla_data(a[OVS_KEY_ATTR_IPV4_TUNNEL]);
+		err = ipv4_tun_from_nlattr(a[OVS_KEY_ATTR_TUNNEL], &swkey->tun_key);
+		if (err)
+			return err;
 
-		if (!tun_key->ipv4_dst)
-			return -EINVAL;
-		if (!(tun_key->tun_flags & OVS_TNL_F_KEY))
+		if (!(swkey->tun_key.tun_flags & OVS_TNL_F_KEY))
 			return -EINVAL;
 
 		tun_id = nla_get_be64(a[OVS_KEY_ATTR_TUN_ID]);
-		if (tun_id != tun_key->tun_id)
+		if (tun_id != swkey->tun_key.tun_id)
 			return -EINVAL;
 
-		memcpy(&swkey->tun_key, tun_key, sizeof(swkey->tun_key));
-		memset(swkey->tun_key.pad, 0, sizeof(swkey->tun_key.pad));
-
 		attrs &= ~(1ULL << OVS_KEY_ATTR_TUN_ID);
-		attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4_TUNNEL);
-	} else if (attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) {
-		struct ovs_key_ipv4_tunnel *tun_key;
-		tun_key = nla_data(a[OVS_KEY_ATTR_IPV4_TUNNEL]);
-
-		if (!tun_key->ipv4_dst)
-			return -EINVAL;
+		attrs &= ~(1ULL << OVS_KEY_ATTR_TUNNEL);
+	} else if (attrs & (1ULL << OVS_KEY_ATTR_TUNNEL)) {
 
-		memcpy(&swkey->tun_key, tun_key, sizeof(swkey->tun_key));
-		memset(swkey->tun_key.pad, 0, sizeof(swkey->tun_key.pad));
+		err = ipv4_tun_from_nlattr(a[OVS_KEY_ATTR_TUNNEL], &swkey->tun_key);
+		if (err)
+			return err;
 
-		attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4_TUNNEL);
+		attrs &= ~(1ULL << OVS_KEY_ATTR_TUNNEL);
 	}
 
 	/* Data attributes. */
@@ -1223,6 +1290,8 @@ int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow, int key_len, const stru
 		int type = nla_type(nla);
 
 		if (type <= OVS_KEY_ATTR_MAX && ovs_key_lens[type] > 0) {
+			int err;
+
 			if (nla_len(nla) != ovs_key_lens[type])
 				return -EINVAL;
 
@@ -1246,21 +1315,23 @@ int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow, int key_len, const stru
 
 				break;
 
-			case OVS_KEY_ATTR_IPV4_TUNNEL:
+			case OVS_KEY_ATTR_TUNNEL:
 				if (tun_key->tun_flags & OVS_TNL_F_KEY) {
 					tun_id = tun_key->tun_id;
+					err = ipv4_tun_from_nlattr(nla, tun_key);
+					if (err)
+						return err;
 
-					memcpy(tun_key, nla_data(nla), sizeof(*tun_key));
 					if (!(tun_key->tun_flags & OVS_TNL_F_KEY))
 						return -EINVAL;
 
 					if (tun_key->tun_id != tun_id)
 						return -EINVAL;
-				} else
-					memcpy(tun_key, nla_data(nla), sizeof(*tun_key));
-
-				if (!tun_key->ipv4_dst)
-					return -EINVAL;
+				} else {
+					err = ipv4_tun_from_nlattr(nla, tun_key);
+					if (err)
+						return err;
+				}
 				break;
 
 			case OVS_KEY_ATTR_IN_PORT:
@@ -1297,14 +1368,10 @@ int ovs_flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb)
 	    nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, swkey->phy.priority))
 		goto nla_put_failure;
 
-	if (swkey->tun_key.ipv4_dst) {
-		struct ovs_key_ipv4_tunnel *tun_key;
-		nla = nla_reserve(skb, OVS_KEY_ATTR_IPV4_TUNNEL, sizeof(*tun_key));
-		if (!nla)
-			goto nla_put_failure;
-		tun_key = nla_data(nla);
-		memcpy(tun_key, &swkey->tun_key, sizeof(*tun_key));
-	}
+	if (swkey->tun_key.ipv4_dst &&
+	    ipv4_tun_to_nlattr(skb, &swkey->tun_key))
+		goto nla_put_failure;
+
 	if ((swkey->tun_key.tun_flags & OVS_TNL_F_KEY) &&
 	    nla_put_be64(skb, OVS_KEY_ATTR_TUN_ID, swkey->tun_key.tun_id))
 		goto nla_put_failure;
diff --git a/datapath/flow.h b/datapath/flow.h
index 3f3624f..ae15024 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -37,9 +37,20 @@ struct sk_buff;
 struct sw_flow_actions {
 	struct rcu_head rcu;
 	u32 actions_len;
+	u32 used;
 	struct nlattr actions[];
 };
 
+struct ovs_key_ipv4_tunnel {
+	__be64 tun_id;
+	__u32  tun_flags;
+	__be32 ipv4_src;
+	__be32 ipv4_dst;
+	__u8   ipv4_tos;
+	__u8   ipv4_ttl;
+	__u8   pad[2];
+};
+
 struct sw_flow_key {
 	struct ovs_key_ipv4_tunnel tun_key;  /* Encapsulating tunnel key. */
 	struct {
@@ -203,5 +214,7 @@ void ovs_flow_tbl_remove(struct flow_table *table, struct sw_flow *flow);
 
 struct sw_flow *ovs_flow_tbl_next(struct flow_table *table, u32 *bucket, u32 *idx);
 extern const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1];
+int ipv4_tun_from_nlattr(const struct nlattr *attr,
+			 struct ovs_key_ipv4_tunnel *tun_key);
 
 #endif /* flow.h */
diff --git a/datapath/tunnel.h b/datapath/tunnel.h
index 7705475..b7de7a9 100644
--- a/datapath/tunnel.h
+++ b/datapath/tunnel.h
@@ -201,7 +201,6 @@ static inline void tnl_tun_key_init(struct ovs_key_ipv4_tunnel *tun_key,
 	tun_key->ipv4_tos = iph->tos;
 	tun_key->ipv4_ttl = iph->ttl;
 	tun_key->tun_flags = tun_flags;
-	memset(tun_key->pad, 0, sizeof(tun_key->pad));
 }
 
 static inline void tnl_get_param(const struct tnl_mutable_config *mutable,
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index 5e32965..96f8bcc 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -265,6 +265,21 @@ struct ovs_flow_stats {
 	__u64 n_bytes;           /* Number of matched bytes. */
 };
 
+
+/* Values for OVS_TUNNEL_FLAGS */
+#define OVS_TNL_F_DONT_FRAGMENT (1 << 0)
+#define OVS_TNL_F_CSUM (1 << 1)
+#define OVS_TNL_F_KEY (1 << 2)
+
+enum ovs_tunnel {
+	OVS_TUNNEL_IPV4_SRC,	/* Tunnel src ip address. */
+	OVS_TUNNEL_IPV4_DST,	/* Tunnel dst ip address. */
+	OVS_TUNNEL_TOS,		/* Tunnel ip TOS. */
+	OVS_TUNNEL_TTL,		/* Tunnel ip TTL. */
+	OVS_TUNNEL_FLAGS,	/* OVS_TNL_F_* flags. */
+	__OVS_TUNNEL_MAX
+};
+
 enum ovs_key_attr {
 	OVS_KEY_ATTR_UNSPEC,
 	OVS_KEY_ATTR_ENCAP,	/* Nested set of encapsulated attributes. */
@@ -282,8 +297,12 @@ enum ovs_key_attr {
 	OVS_KEY_ATTR_ARP,       /* struct ovs_key_arp */
 	OVS_KEY_ATTR_ND,        /* struct ovs_key_nd */
 	OVS_KEY_ATTR_SKB_MARK,  /* u32 skb mark */
-	OVS_KEY_ATTR_IPV4_TUNNEL,  /* struct ovs_key_ipv4_tunnel */
+	OVS_KEY_ATTR_TUNNEL,	/* Nested set of ovs_tunnel attributes */
 	OVS_KEY_ATTR_TUN_ID = 63,  /* be64 tunnel ID */
+
+#ifdef __KERNEL__
+	OVS_KEY_ATTR_IPV4_TUNNEL,  /* struct ovs_key_ipv4_tunnel */
+#endif
 	__OVS_KEY_ATTR_MAX
 };
 
@@ -365,21 +384,6 @@ struct ovs_key_nd {
 	__u8  nd_tll[6];
 };
 
-/* Values for ovs_key_ipv4_tunnel->tun_flags */
-#define OVS_TNL_F_DONT_FRAGMENT (1 << 0)
-#define OVS_TNL_F_CSUM (1 << 1)
-#define OVS_TNL_F_KEY (1 << 2)
-
-struct ovs_key_ipv4_tunnel {
-	__be64 tun_id;
-	__u32  tun_flags;
-	__be32 ipv4_src;
-	__be32 ipv4_dst;
-	__u8   ipv4_tos;
-	__u8   ipv4_ttl;
-	__u8   pad[2];
-};
-
 /**
  * enum ovs_flow_attr - attributes for %OVS_FLOW_* commands.
  * @OVS_FLOW_ATTR_KEY: Nested %OVS_KEY_ATTR_* attributes specifying the flow
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 682dfc9..67a5333 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1193,7 +1193,7 @@ execute_set_action(struct ofpbuf *packet, const struct nlattr *a)
     case OVS_KEY_ATTR_TUN_ID:
     case OVS_KEY_ATTR_PRIORITY:
     case OVS_KEY_ATTR_SKB_MARK:
-    case OVS_KEY_ATTR_IPV4_TUNNEL:
+    case OVS_KEY_ATTR_TUNNEL:
         /* not implemented */
         break;
 
diff --git a/lib/flow.c b/lib/flow.c
index a13519e..2cd6b2f 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -579,7 +579,6 @@ void
 flow_wildcards_init_exact(struct flow_wildcards *wc)
 {
     memset(&wc->masks, 0xff, sizeof wc->masks);
-    memset(wc->masks.zeros, 0, sizeof wc->masks.zeros);
 }
 
 /* Returns true if 'wc' matches every packet, false if 'wc' fixes any bits or
diff --git a/lib/flow.h b/lib/flow.h
index 8e79e62..323b248 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -63,9 +63,10 @@ struct flow_tnl {
     ovs_be64 tun_id;
     ovs_be32 ip_src;
     ovs_be32 ip_dst;
-    uint16_t flags;
+    uint32_t flags;
     uint8_t ip_tos;
     uint8_t ip_ttl;
+    uint8_t zeros[2];
 };
 
 /*
@@ -103,7 +104,6 @@ struct flow {
     uint8_t arp_tha[6];         /* ARP/ND target hardware address. */
     uint8_t nw_ttl;             /* IP TTL/Hop Limit. */
     uint8_t nw_frag;            /* FLOW_FRAG_* flags. */
-    uint8_t zeros[4];
 };
 BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index e2f21da..4cb07fc 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -95,7 +95,7 @@ ovs_key_attr_to_string(enum ovs_key_attr attr)
     case OVS_KEY_ATTR_PRIORITY: return "skb_priority";
     case OVS_KEY_ATTR_SKB_MARK: return "skb_mark";
     case OVS_KEY_ATTR_TUN_ID: return "tun_id";
-    case OVS_KEY_ATTR_IPV4_TUNNEL: return "ipv4_tunnel";
+    case OVS_KEY_ATTR_TUNNEL: return "ipv4_tunnel";
     case OVS_KEY_ATTR_IN_PORT: return "in_port";
     case OVS_KEY_ATTR_ETHERNET: return "eth";
     case OVS_KEY_ATTR_VLAN: return "vlan";
@@ -617,7 +617,7 @@ odp_flow_key_attr_len(uint16_t type)
     case OVS_KEY_ATTR_PRIORITY: return 4;
     case OVS_KEY_ATTR_SKB_MARK: return 4;
     case OVS_KEY_ATTR_TUN_ID: return 8;
-    case OVS_KEY_ATTR_IPV4_TUNNEL: return sizeof(struct ovs_key_ipv4_tunnel);
+    case OVS_KEY_ATTR_TUNNEL: return -2;
     case OVS_KEY_ATTR_IN_PORT: return 4;
     case OVS_KEY_ATTR_ETHERNET: return sizeof(struct ovs_key_ethernet);
     case OVS_KEY_ATTR_VLAN: return sizeof(ovs_be16);
@@ -687,6 +687,112 @@ odp_tun_flag_to_string(uint32_t flags)
     }
 }
 
+/* The set of kernel flags we understand. Used to detect if ODP_FIT_TOO_MUCH */
+#define OVS_TNL_F_KNOWN_MASK \
+    (OVS_TNL_F_DONT_FRAGMENT | OVS_TNL_F_CSUM | OVS_TNL_F_KEY)
+
+/* These allow the flow/kernel view of the flags to change in future */
+static uint32_t
+flow_to_odp_flags(uint32_t flags)
+{
+    return (flags & FLOW_TNL_F_DONT_FRAGMENT ? OVS_TNL_F_DONT_FRAGMENT : 0)
+        | (flags & FLOW_TNL_F_CSUM ? OVS_TNL_F_CSUM : 0)
+        | (flags & FLOW_TNL_F_KEY ? OVS_TNL_F_KEY : 0);
+}
+
+static uint32_t
+odp_to_flow_flags(uint32_t tun_flags)
+{
+    return (tun_flags & OVS_TNL_F_DONT_FRAGMENT ? FLOW_TNL_F_DONT_FRAGMENT : 0)
+        | (tun_flags & OVS_TNL_F_CSUM ? FLOW_TNL_F_CSUM : 0)
+        | (tun_flags & OVS_TNL_F_KEY ? FLOW_TNL_F_KEY : 0);
+}
+
+/* Returns OVS_TNL_* flags. */
+static int
+tun_key_from_attr(const struct nlattr *attr, struct flow_tnl *tun)
+{
+    static const struct nl_policy ipv4_tun_key_policy[] = {
+        [OVS_KEY_ATTR_TUN_ID] = { .type = NL_A_BE64, .optional = true },
+        [OVS_TUNNEL_IPV4_SRC] = { .type = NL_A_BE32, .optional = true },
+        [OVS_TUNNEL_IPV4_DST] = { .type = NL_A_BE32 },
+        [OVS_TUNNEL_TOS] = { .type = NL_A_U8, .optional = true },
+        [OVS_TUNNEL_TTL] = { .type = NL_A_U8, .optional = true },
+        [OVS_TUNNEL_FLAGS] = { .type = NL_A_U32, .optional = true },
+    };
+    struct nlattr *a[ARRAY_SIZE(ipv4_tun_key_policy)];
+
+    if (!nl_parse_nested(attr, ipv4_tun_key_policy, a, ARRAY_SIZE(a))) {
+        return -EINVAL;
+    }
+
+    tun->ip_dst = nl_attr_get_be32(a[OVS_TUNNEL_IPV4_DST]);
+
+    if (a[OVS_KEY_ATTR_TUN_ID]) {
+        tun->tun_id = nl_attr_get_be64(a[OVS_KEY_ATTR_TUN_ID]);
+    } else {
+        tun->tun_id = 0;
+    }
+    if (a[OVS_TUNNEL_IPV4_SRC]) {
+        tun->ip_src = nl_attr_get_be32(a[OVS_TUNNEL_IPV4_SRC]);
+    } else {
+        tun->ip_src = 0;
+    }
+
+    if (a[OVS_TUNNEL_TOS]) {
+        tun->ip_tos = nl_attr_get_u8(a[OVS_TUNNEL_TOS]);
+    } else {
+        tun->ip_tos = 0;
+    }
+    if (a[OVS_TUNNEL_TTL]) {
+        tun->ip_ttl = nl_attr_get_u8(a[OVS_TUNNEL_TTL]);
+    } else {
+        tun->ip_ttl = 0;
+    }
+    if (a[OVS_TUNNEL_FLAGS]) {
+        tun->flags = nl_attr_get_u32(a[OVS_TUNNEL_FLAGS]);
+    } else {
+        tun->flags = 0;
+    }
+    memset(tun->zeros, 0, sizeof tun->zeros);
+    return 0;
+}
+
+static void
+tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key, bool odp_flags)
+{
+    size_t tun_key_ofs;
+
+    tun_key_ofs = nl_msg_start_nested(a, OVS_KEY_ATTR_TUNNEL);
+
+    /* layouts differ, flags has different size */
+    if (tun_key->tun_id) {
+        nl_msg_put_be64(a, OVS_KEY_ATTR_TUN_ID, tun_key->tun_id);
+    }
+    if (tun_key->ip_src) {
+        nl_msg_put_be32(a, OVS_TUNNEL_IPV4_SRC, tun_key->ip_src);
+    }
+    if (tun_key->ip_dst) {
+        nl_msg_put_be32(a, OVS_TUNNEL_IPV4_DST, tun_key->ip_dst);
+    }
+    if (tun_key->ip_tos) {
+        nl_msg_put_u8(a, OVS_TUNNEL_TOS, tun_key->ip_tos);
+    }
+    if (tun_key->ip_ttl) {
+        nl_msg_put_u8(a, OVS_TUNNEL_TTL, tun_key->ip_ttl);
+    }
+
+    if (tun_key->flags) {
+        if (odp_flags) {
+            nl_msg_put_u32(a, OVS_TUNNEL_FLAGS, flow_to_odp_flags(tun_key->flags));
+        } else {
+            nl_msg_put_u32(a, OVS_TUNNEL_FLAGS, tun_key->flags);
+        }
+    }
+
+    nl_msg_end_nested(a, tun_key_ofs);
+}
+
 static void
 format_odp_key_attr(const struct nlattr *a, struct ds *ds)
 {
@@ -699,7 +805,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
     const struct ovs_key_icmpv6 *icmpv6_key;
     const struct ovs_key_arp *arp_key;
     const struct ovs_key_nd *nd_key;
-    const struct ovs_key_ipv4_tunnel *ipv4_tun_key;
+    struct flow_tnl tun_key;
     enum ovs_key_attr attr = nl_attr_type(a);
     int expected_len;
 
@@ -734,17 +840,17 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
         ds_put_format(ds, "(%#"PRIx64")", ntohll(nl_attr_get_be64(a)));
         break;
 
-    case OVS_KEY_ATTR_IPV4_TUNNEL:
-        ipv4_tun_key = nl_attr_get(a);
+    case OVS_KEY_ATTR_TUNNEL:
+        tun_key_from_attr(a, &tun_key);
         ds_put_format(ds, "(tun_id=0x%"PRIx64",src="IP_FMT",dst="IP_FMT","
                       "tos=0x%"PRIx8",ttl=%"PRIu8",flags(",
-                      ntohll(ipv4_tun_key->tun_id),
-                      IP_ARGS(ipv4_tun_key->ipv4_src),
-                      IP_ARGS(ipv4_tun_key->ipv4_dst),
-                      ipv4_tun_key->ipv4_tos, ipv4_tun_key->ipv4_ttl);
+                      ntohll(tun_key.tun_id),
+                      IP_ARGS(tun_key.ip_src),
+                      IP_ARGS(tun_key.ip_dst),
+                      tun_key.ip_tos, tun_key.ip_ttl);
 
         format_flags(ds, odp_tun_flag_to_string,
-                     ipv4_tun_key->tun_flags, ',');
+                     tun_key.flags, ',');
         ds_put_format(ds, "))");
         break;
 
@@ -974,23 +1080,23 @@ parse_odp_key_attr(const char *s, const struct simap *port_names,
     {
         char tun_id_s[32];
         int tos, ttl;
-        struct ovs_key_ipv4_tunnel tun_key;
+        struct flow_tnl tun_key;
         int n = -1;
 
         if (sscanf(s, "ipv4_tunnel(tun_id=%31[x0123456789abcdefABCDEF],"
                    "src="IP_SCAN_FMT",dst="IP_SCAN_FMT
                    ",tos=%i,ttl=%i,flags%n", tun_id_s,
-                    IP_SCAN_ARGS(&tun_key.ipv4_src),
-                    IP_SCAN_ARGS(&tun_key.ipv4_dst), &tos, &ttl,
+                    IP_SCAN_ARGS(&tun_key.ip_src),
+                    IP_SCAN_ARGS(&tun_key.ip_dst), &tos, &ttl,
                     &n) > 0 && n > 0) {
             int res;
 
             tun_key.tun_id = htonll(strtoull(tun_id_s, NULL, 0));
-            tun_key.ipv4_tos = tos;
-            tun_key.ipv4_ttl = ttl;
+            tun_key.ip_tos = tos;
+            tun_key.ip_ttl = ttl;
 
             res = parse_flags(&s[n], odp_tun_flag_to_string,
-                              &tun_key.tun_flags);
+                              &tun_key.flags);
             if (res < 0) {
                 return res;
             }
@@ -1000,9 +1106,7 @@ parse_odp_key_attr(const char *s, const struct simap *port_names,
             }
             n++;
 
-            memset(&tun_key.pad, 0, sizeof tun_key.pad);
-            nl_msg_put_unspec(key, OVS_KEY_ATTR_IPV4_TUNNEL, &tun_key,
-                              sizeof tun_key);
+            tun_key_to_attr(key, &tun_key, false);
             return n;
         }
     }
@@ -1342,27 +1446,6 @@ ovs_to_odp_frag(uint8_t nw_frag)
           : OVS_FRAG_TYPE_LATER);
 }
 
-/* The set of kernel flags we understand. Used to detect if ODP_FIT_TOO_MUCH */
-#define OVS_TNL_F_KNOWN_MASK \
-    (OVS_TNL_F_DONT_FRAGMENT | OVS_TNL_F_CSUM | OVS_TNL_F_KEY)
-
-/* These allow the flow/kernel view of the flags to change in future */
-static uint32_t
-flow_to_odp_flags(uint16_t flags)
-{
-    return (flags & FLOW_TNL_F_DONT_FRAGMENT ? OVS_TNL_F_DONT_FRAGMENT : 0)
-        | (flags & FLOW_TNL_F_CSUM ? OVS_TNL_F_CSUM : 0)
-        | (flags & FLOW_TNL_F_KEY ? OVS_TNL_F_KEY : 0);
-}
-
-static uint16_t
-odp_to_flow_flags(uint32_t tun_flags)
-{
-    return (tun_flags & OVS_TNL_F_DONT_FRAGMENT ? FLOW_TNL_F_DONT_FRAGMENT : 0)
-        | (tun_flags & OVS_TNL_F_CSUM ? FLOW_TNL_F_CSUM : 0)
-        | (tun_flags & OVS_TNL_F_KEY ? FLOW_TNL_F_KEY : 0);
-}
-
 /* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'.
  * 'flow->in_port' is ignored (since it is likely to be an OpenFlow port
  * number rather than a datapath port number).  Instead, if 'odp_in_port'
@@ -1383,18 +1466,7 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow,
     }
 
     if (flow->tunnel.ip_dst) {
-        struct ovs_key_ipv4_tunnel *ipv4_tun_key;
-
-        ipv4_tun_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_IPV4_TUNNEL,
-                                            sizeof *ipv4_tun_key);
-        /* layouts differ, flags has different size */
-        ipv4_tun_key->tun_id = flow->tunnel.tun_id;
-        ipv4_tun_key->tun_flags = flow_to_odp_flags(flow->tunnel.flags);
-        ipv4_tun_key->ipv4_src = flow->tunnel.ip_src;
-        ipv4_tun_key->ipv4_dst = flow->tunnel.ip_dst;
-        ipv4_tun_key->ipv4_tos = flow->tunnel.ip_tos;
-        ipv4_tun_key->ipv4_ttl = flow->tunnel.ip_ttl;
-        memset(ipv4_tun_key->pad, 0, sizeof ipv4_tun_key->pad);
+        tun_key_to_attr(buf, &flow->tunnel, true);
     } else if (flow->tunnel.tun_id != htonll(0)) {
         nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tunnel.tun_id);
     }
@@ -1905,23 +1977,17 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
         expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TUN_ID;
     }
 
-    if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL)) {
-        const struct ovs_key_ipv4_tunnel *ipv4_tun_key;
+    if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_TUNNEL)) {
 
-        ipv4_tun_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4_TUNNEL]);
-
-        flow->tunnel.tun_id = ipv4_tun_key->tun_id;
-        flow->tunnel.ip_src = ipv4_tun_key->ipv4_src;
-        flow->tunnel.ip_dst = ipv4_tun_key->ipv4_dst;
-        flow->tunnel.flags = odp_to_flow_flags(ipv4_tun_key->tun_flags);
-        flow->tunnel.ip_tos = ipv4_tun_key->ipv4_tos;
-        flow->tunnel.ip_ttl = ipv4_tun_key->ipv4_ttl;
+        if (tun_key_from_attr(attrs[OVS_KEY_ATTR_TUNNEL], &flow->tunnel))
+            return ODP_FIT_ERROR;
 
+        flow->tunnel.flags = odp_to_flow_flags(flow->tunnel.flags);
         /* Allow this to show up as unexpected, if there are unknown flags,
          * eventually resulting in ODP_FIT_TOO_MUCH.
          * OVS_TNL_F_KNOWN_MASK defined locally above. */
-        if (!(ipv4_tun_key->tun_flags & ~OVS_TNL_F_KNOWN_MASK)) {
-            expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL;
+        if (!(flow->tunnel.flags & ~OVS_TNL_F_KNOWN_MASK)) {
+            expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TUNNEL;
         }
     }
 
@@ -2017,18 +2083,11 @@ commit_set_tunnel_action(const struct flow *flow, struct flow *base,
 
     /* A valid IPV4_TUNNEL must have non-zero ip_dst. */
     if (flow->tunnel.ip_dst) {
-        struct ovs_key_ipv4_tunnel ipv4_tun_key;
-
-        ipv4_tun_key.tun_id = base->tunnel.tun_id;
-        ipv4_tun_key.tun_flags = flow_to_odp_flags(base->tunnel.flags);
-        ipv4_tun_key.ipv4_src = base->tunnel.ip_src;
-        ipv4_tun_key.ipv4_dst = base->tunnel.ip_dst;
-        ipv4_tun_key.ipv4_tos = base->tunnel.ip_tos;
-        ipv4_tun_key.ipv4_ttl = base->tunnel.ip_ttl;
-        memset(&ipv4_tun_key.pad, 0, sizeof ipv4_tun_key.pad);
-
-        commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4_TUNNEL,
-                          &ipv4_tun_key, sizeof ipv4_tun_key);
+        size_t offset;
+
+        offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
+        tun_key_to_attr(odp_actions, &base->tunnel, true);
+        nl_msg_end_nested(odp_actions, offset);
     } else {
         commit_set_action(odp_actions, OVS_KEY_ATTR_TUN_ID,
                           &base->tunnel.tun_id, sizeof base->tunnel.tun_id);
diff --git a/tests/test-bundle.c b/tests/test-bundle.c
index aa8b6f0..f2d9b82 100644
--- a/tests/test-bundle.c
+++ b/tests/test-bundle.c
@@ -136,7 +136,6 @@ main(int argc, char *argv[])
     flows = xmalloc(N_FLOWS * sizeof *flows);
     for (i = 0; i < N_FLOWS; i++) {
         random_bytes(&flows[i], sizeof flows[i]);
-        memset(flows[i].zeros, 0, sizeof flows[i].zeros);
         flows[i].regs[0] = OFPP_NONE;
     }
 
diff --git a/tests/test-multipath.c b/tests/test-multipath.c
index b990c13..8a35567 100644
--- a/tests/test-multipath.c
+++ b/tests/test-multipath.c
@@ -60,7 +60,6 @@ main(int argc, char *argv[])
             struct flow flow;
 
             random_bytes(&flow, sizeof flow);
-            memset(flow.zeros, 0, sizeof flow.zeros);
 
             mp.max_link = n - 1;
             multipath_execute(&mp, &flow);
-- 
1.7.10




More information about the dev mailing list