netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv5 net-next 0/2] net: erspan: add support for openvswitch
@ 2018-01-24 19:06 William Tu
  2018-01-24 19:06 ` [PATCHv5 net-next 1/2] net: erspan: use bitfield instead of mask and offset William Tu
  2018-01-24 19:06 ` [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support William Tu
  0 siblings, 2 replies; 6+ messages in thread
From: William Tu @ 2018-01-24 19:06 UTC (permalink / raw)
  To: netdev; +Cc: pshelar

The first patch refactors the erspan header definitions. 
Originally, the erspan fields are defined as a group into a __be16 field,
and use mask and offset to access each field.  This is more costly due to
calling ntohs/htons and error-prone.  The first patch changes it to use
bitfields.  The second patch introduces the new OVS tunnel key attribute
to program both v1 and v2 erspan tunnel for openvswitch.

William Tu (2):
  net: erspan: use bitfield instead of mask and offset
  openvswitch: add erspan version I and II support

 include/net/erspan.h             | 127 +++++++++++++++++++++++++++++----------
 include/uapi/linux/openvswitch.h |   2 +-
 net/ipv4/ip_gre.c                |  38 +++++-------
 net/ipv6/ip6_gre.c               |  36 ++++-------
 net/openvswitch/flow_netlink.c   |  90 ++++++++++++++++++++++++++-
 5 files changed, 211 insertions(+), 82 deletions(-)

-- 
v4->v5
  rather than passing individual members of erspan_metadata,
  just pass the whole binary structure between kernel and userspace,
  suggested by Pravin.

v3->v4
  change from be32 to u32 for OVS_ERSPAN_OPT_IDX, suggested by Jiri Benc.

v2->v3
  revert the "openvswitch: Add erspan tunnel support." commit ceaa001a170e.
  redesign the OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS as nested attribute

v1->v2
  Fix compatibility issue suggested by Pravin.

2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv5 net-next 1/2] net: erspan: use bitfield instead of mask and offset
  2018-01-24 19:06 [PATCHv5 net-next 0/2] net: erspan: add support for openvswitch William Tu
@ 2018-01-24 19:06 ` William Tu
  2018-01-24 19:06 ` [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support William Tu
  1 sibling, 0 replies; 6+ messages in thread
From: William Tu @ 2018-01-24 19:06 UTC (permalink / raw)
  To: netdev; +Cc: pshelar

Originally the erspan fields are defined as a group into a __be16 field,
and use mask and offset to access each field.  This is more costly due to
calling ntohs/htons.  The patch changes it to use bitfields.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 include/net/erspan.h | 127 ++++++++++++++++++++++++++++++++++++++-------------
 net/ipv4/ip_gre.c    |  38 ++++++---------
 net/ipv6/ip6_gre.c   |  36 ++++++---------
 3 files changed, 121 insertions(+), 80 deletions(-)

diff --git a/include/net/erspan.h b/include/net/erspan.h
index acdf6843095d..2b75821e2ebe 100644
--- a/include/net/erspan.h
+++ b/include/net/erspan.h
@@ -65,16 +65,30 @@
 #define GRA_MASK	0x0006
 #define O_MASK		0x0001
 
+#define HWID_OFFSET    4
+#define DIR_OFFSET     3
+
 /* ERSPAN version 2 metadata header */
 struct erspan_md2 {
 	__be32 timestamp;
 	__be16 sgt;	/* security group tag */
-	__be16 flags;
-#define P_OFFSET	15
-#define FT_OFFSET	10
-#define HWID_OFFSET	4
-#define DIR_OFFSET	3
-#define GRA_OFFSET	1
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+	__u8	hwid_upper:2,
+		ft:5,
+		p:1;
+	__u8	o:1,
+		gra:2,
+		dir:1,
+		hwid:4;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+	__u8	p:1,
+		ft:5,
+		hwid_upper:2;
+	__u8	hwid:4,
+		dir:1,
+		gra:2,
+		o:1;
+#endif
 };
 
 enum erspan_encap_type {
@@ -95,15 +109,62 @@ struct erspan_metadata {
 };
 
 struct erspan_base_hdr {
-	__be16 ver_vlan;
-#define VER_OFFSET  12
-	__be16 session_id;
-#define COS_OFFSET  13
-#define EN_OFFSET   11
-#define BSO_OFFSET  EN_OFFSET
-#define T_OFFSET    10
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+	__u8	vlan_upper:4,
+		ver:4;
+	__u8	vlan:8;
+	__u8	session_id_upper:2,
+		t:1,
+		en:2,
+		cos:3;
+	__u8	session_id:8;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+	__u8	ver: 4,
+		vlan_upper:4;
+	__u8	vlan:8;
+	__u8	cos:3,
+		en:2,
+		t:1,
+		session_id_upper:2;
+	__u8	session_id:8;
+#else
+#error "Please fix <asm/byteorder.h>"
+#endif
 };
 
+static inline void set_session_id(struct erspan_base_hdr *ershdr, u16 id)
+{
+	ershdr->session_id = id & 0xff;
+	ershdr->session_id_upper = (id >> 8) & 0x3;
+}
+
+static inline u16 get_session_id(const struct erspan_base_hdr *ershdr)
+{
+	return (ershdr->session_id_upper << 8) + ershdr->session_id;
+}
+
+static inline void set_vlan(struct erspan_base_hdr *ershdr, u16 vlan)
+{
+	ershdr->vlan = vlan & 0xff;
+	ershdr->vlan_upper = (vlan >> 8) & 0xf;
+}
+
+static inline u16 get_vlan(const struct erspan_base_hdr *ershdr)
+{
+	return (ershdr->vlan_upper << 8) + ershdr->vlan;
+}
+
+static inline void set_hwid(struct erspan_md2 *md2, u8 hwid)
+{
+	md2->hwid = hwid & 0xf;
+	md2->hwid_upper = (hwid >> 4) & 0x3;
+}
+
+static inline u8 get_hwid(const struct erspan_md2 *md2)
+{
+	return (md2->hwid_upper << 4) + md2->hwid;
+}
+
 static inline int erspan_hdr_len(int version)
 {
 	return sizeof(struct erspan_base_hdr) +
@@ -120,7 +181,7 @@ static inline u8 tos_to_cos(u8 tos)
 }
 
 static inline void erspan_build_header(struct sk_buff *skb,
-				__be32 id, u32 index,
+				u32 id, u32 index,
 				bool truncate, bool is_ipv4)
 {
 	struct ethhdr *eth = eth_hdr(skb);
@@ -154,12 +215,12 @@ static inline void erspan_build_header(struct sk_buff *skb,
 	memset(ershdr, 0, sizeof(*ershdr) + ERSPAN_V1_MDSIZE);
 
 	/* Build base header */
-	ershdr->ver_vlan = htons((vlan_tci & VLAN_MASK) |
-				 (ERSPAN_VERSION << VER_OFFSET));
-	ershdr->session_id = htons((u16)(ntohl(id) & ID_MASK) |
-			   ((tos_to_cos(tos) << COS_OFFSET) & COS_MASK) |
-			   (enc_type << EN_OFFSET & EN_MASK) |
-			   ((truncate << T_OFFSET) & T_MASK));
+	ershdr->ver = ERSPAN_VERSION;
+	ershdr->cos = tos_to_cos(tos);
+	ershdr->en = enc_type;
+	ershdr->t = truncate;
+	set_vlan(ershdr, vlan_tci);
+	set_session_id(ershdr, id);
 
 	/* Build metadata */
 	ersmd = (struct erspan_metadata *)(ershdr + 1);
@@ -187,7 +248,7 @@ static inline __be32 erspan_get_timestamp(void)
 }
 
 static inline void erspan_build_header_v2(struct sk_buff *skb,
-					  __be32 id, u8 direction, u16 hwid,
+					  u32 id, u8 direction, u16 hwid,
 					  bool truncate, bool is_ipv4)
 {
 	struct ethhdr *eth = eth_hdr(skb);
@@ -198,7 +259,6 @@ static inline void erspan_build_header_v2(struct sk_buff *skb,
 		__be16 tci;
 	} *qp;
 	u16 vlan_tci = 0;
-	u16 session_id;
 	u8 gra = 0; /* 100 usec */
 	u8 bso = 0; /* Bad/Short/Oversized */
 	u8 sgt = 0;
@@ -221,22 +281,23 @@ static inline void erspan_build_header_v2(struct sk_buff *skb,
 	memset(ershdr, 0, sizeof(*ershdr) + ERSPAN_V2_MDSIZE);
 
 	/* Build base header */
-	ershdr->ver_vlan = htons((vlan_tci & VLAN_MASK) |
-				 (ERSPAN_VERSION2 << VER_OFFSET));
-	session_id = (u16)(ntohl(id) & ID_MASK) |
-		     ((tos_to_cos(tos) << COS_OFFSET) & COS_MASK) |
-		     (bso << BSO_OFFSET & BSO_MASK) |
-		     ((truncate << T_OFFSET) & T_MASK);
-	ershdr->session_id = htons(session_id);
+	ershdr->ver = ERSPAN_VERSION2;
+	ershdr->cos = tos_to_cos(tos);
+	ershdr->en = bso;
+	ershdr->t = truncate;
+	set_vlan(ershdr, vlan_tci);
+	set_session_id(ershdr, id);
 
 	/* Build metadata */
 	md = (struct erspan_metadata *)(ershdr + 1);
 	md->u.md2.timestamp = erspan_get_timestamp();
 	md->u.md2.sgt = htons(sgt);
-	md->u.md2.flags = htons(((1 << P_OFFSET) & P_MASK) |
-				((hwid << HWID_OFFSET) & HWID_MASK) |
-				((direction << DIR_OFFSET) & DIR_MASK) |
-				((gra << GRA_OFFSET) & GRA_MASK));
+	md->u.md2.p = 1;
+	md->u.md2.ft = 0;
+	md->u.md2.dir = direction;
+	md->u.md2.gra = gra;
+	md->u.md2.o = 0;
+	set_hwid(&md->u.md2, hwid);
 }
 
 #endif
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index b61f2285816d..6ec670fbbbdd 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -114,7 +114,7 @@ MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
 static struct rtnl_link_ops ipgre_link_ops __read_mostly;
 static int ipgre_tunnel_init(struct net_device *dev);
 static void erspan_build_header(struct sk_buff *skb,
-				__be32 id, u32 index,
+				u32 id, u32 index,
 				bool truncate, bool is_ipv4);
 
 static unsigned int ipgre_net_id __read_mostly;
@@ -273,12 +273,12 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 
 	iph = ip_hdr(skb);
 	ershdr = (struct erspan_base_hdr *)(skb->data + gre_hdr_len);
-	ver = (ntohs(ershdr->ver_vlan) & VER_MASK) >> VER_OFFSET;
+	ver = ershdr->ver;
 
 	/* The original GRE header does not have key field,
 	 * Use ERSPAN 10-bit session ID as key.
 	 */
-	tpi->key = cpu_to_be32(ntohs(ershdr->session_id) & ID_MASK);
+	tpi->key = cpu_to_be32(get_session_id(ershdr));
 	tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex,
 				  tpi->flags | TUNNEL_KEY,
 				  iph->saddr, iph->daddr, tpi->key);
@@ -324,14 +324,8 @@ static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 			if (ver == 1) {
 				tunnel->index = ntohl(pkt_md->u.index);
 			} else {
-				u16 md2_flags;
-				u16 dir, hwid;
-
-				md2_flags = ntohs(pkt_md->u.md2.flags);
-				dir = (md2_flags & DIR_MASK) >> DIR_OFFSET;
-				hwid = (md2_flags & HWID_MASK) >> HWID_OFFSET;
-				tunnel->dir = dir;
-				tunnel->hwid = hwid;
+				tunnel->dir = pkt_md->u.md2.dir;
+				tunnel->hwid = get_hwid(&pkt_md->u.md2);
 			}
 
 		}
@@ -615,19 +609,14 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	if (version == 1) {
-		erspan_build_header(skb, tunnel_id_to_key32(key->tun_id),
+		erspan_build_header(skb, ntohl(tunnel_id_to_key32(key->tun_id)),
 				    ntohl(md->u.index), truncate, true);
 	} else if (version == 2) {
-		u16 md2_flags;
-		u8 direction;
-		u16 hwid;
-
-		md2_flags = ntohs(md->u.md2.flags);
-		direction = (md2_flags & DIR_MASK) >> DIR_OFFSET;
-		hwid = (md2_flags & HWID_MASK) >> HWID_OFFSET;
-
-		erspan_build_header_v2(skb, tunnel_id_to_key32(key->tun_id),
-				       direction, hwid,	truncate, true);
+		erspan_build_header_v2(skb,
+				       ntohl(tunnel_id_to_key32(key->tun_id)),
+				       md->u.md2.dir,
+				       get_hwid(&md->u.md2),
+				       truncate, true);
 	} else {
 		goto err_free_rt;
 	}
@@ -733,10 +722,11 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb,
 
 	/* Push ERSPAN header */
 	if (tunnel->erspan_ver == 1)
-		erspan_build_header(skb, tunnel->parms.o_key, tunnel->index,
+		erspan_build_header(skb, ntohl(tunnel->parms.o_key),
+				    tunnel->index,
 				    truncate, true);
 	else
-		erspan_build_header_v2(skb, tunnel->parms.o_key,
+		erspan_build_header_v2(skb, ntohl(tunnel->parms.o_key),
 				       tunnel->dir, tunnel->hwid,
 				       truncate, true);
 
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index db99446e0276..9f35087f93bc 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -512,8 +512,8 @@ static int ip6erspan_rcv(struct sk_buff *skb, int gre_hdr_len,
 
 	ipv6h = ipv6_hdr(skb);
 	ershdr = (struct erspan_base_hdr *)skb->data;
-	ver = (ntohs(ershdr->ver_vlan) & VER_MASK) >> VER_OFFSET;
-	tpi->key = cpu_to_be32(ntohs(ershdr->session_id) & ID_MASK);
+	ver = ershdr->ver;
+	tpi->key = cpu_to_be32(get_session_id(ershdr));
 
 	tunnel = ip6gre_tunnel_lookup(skb->dev,
 				      &ipv6h->saddr, &ipv6h->daddr, tpi->key,
@@ -564,14 +564,8 @@ static int ip6erspan_rcv(struct sk_buff *skb, int gre_hdr_len,
 			if (ver == 1) {
 				tunnel->parms.index = ntohl(pkt_md->u.index);
 			} else {
-				u16 md2_flags;
-				u16 dir, hwid;
-
-				md2_flags = ntohs(pkt_md->u.md2.flags);
-				dir = (md2_flags & DIR_MASK) >> DIR_OFFSET;
-				hwid = (md2_flags & HWID_MASK) >> HWID_OFFSET;
-				tunnel->parms.dir = dir;
-				tunnel->parms.hwid = hwid;
+				tunnel->parms.dir = pkt_md->u.md2.dir;
+				tunnel->parms.hwid = get_hwid(&pkt_md->u.md2);
 			}
 
 			ip6_tnl_rcv(tunnel, skb, tpi, NULL, log_ecn_error);
@@ -924,6 +918,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 		struct ip_tunnel_info *tun_info;
 		const struct ip_tunnel_key *key;
 		struct erspan_metadata *md;
+		__be32 tun_id;
 
 		tun_info = skb_tunnel_info(skb);
 		if (unlikely(!tun_info ||
@@ -943,23 +938,18 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 		if (!md)
 			goto tx_err;
 
+		tun_id = tunnel_id_to_key32(key->tun_id);
 		if (md->version == 1) {
 			erspan_build_header(skb,
-					    tunnel_id_to_key32(key->tun_id),
+					    ntohl(tun_id),
 					    ntohl(md->u.index), truncate,
 					    false);
 		} else if (md->version == 2) {
-			u16 md2_flags;
-			u16 dir, hwid;
-
-			md2_flags = ntohs(md->u.md2.flags);
-			dir = (md2_flags & DIR_MASK) >> DIR_OFFSET;
-			hwid = (md2_flags & HWID_MASK) >> HWID_OFFSET;
-
 			erspan_build_header_v2(skb,
-					       tunnel_id_to_key32(key->tun_id),
-					       dir, hwid, truncate,
-					       false);
+					       ntohl(tun_id),
+					       md->u.md2.dir,
+					       get_hwid(&md->u.md2),
+					       truncate, false);
 		}
 	} else {
 		switch (skb->protocol) {
@@ -981,11 +971,11 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 		}
 
 		if (t->parms.erspan_ver == 1)
-			erspan_build_header(skb, t->parms.o_key,
+			erspan_build_header(skb, ntohl(t->parms.o_key),
 					    t->parms.index,
 					    truncate, false);
 		else
-			erspan_build_header_v2(skb, t->parms.o_key,
+			erspan_build_header_v2(skb, ntohl(t->parms.o_key),
 					       t->parms.dir,
 					       t->parms.hwid,
 					       truncate, false);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support
  2018-01-24 19:06 [PATCHv5 net-next 0/2] net: erspan: add support for openvswitch William Tu
  2018-01-24 19:06 ` [PATCHv5 net-next 1/2] net: erspan: use bitfield instead of mask and offset William Tu
@ 2018-01-24 19:06 ` William Tu
  2018-01-25 17:32   ` Pravin Shelar
  1 sibling, 1 reply; 6+ messages in thread
From: William Tu @ 2018-01-24 19:06 UTC (permalink / raw)
  To: netdev; +Cc: pshelar

The patch adds support for openvswitch to configure erspan
v1 and v2.  The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added
to uapi as a binary blob to support all ERSPAN v1 and v2's
fields.  Note that Previous commit "openvswitch: Add erspan tunnel
support." was reverted since it does not design properly.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 include/uapi/linux/openvswitch.h |  2 +-
 net/openvswitch/flow_netlink.c   | 90 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dcfab5e3b55c..158c2e45c0a5 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -273,7 +273,6 @@ enum {
 
 #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
 
-
 /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
  */
 enum {
@@ -363,6 +362,7 @@ enum ovs_tunnel_key_attr {
 	OVS_TUNNEL_KEY_ATTR_IPV6_SRC,		/* struct in6_addr src IPv6 address. */
 	OVS_TUNNEL_KEY_ATTR_IPV6_DST,		/* struct in6_addr dst IPv6 address. */
 	OVS_TUNNEL_KEY_ATTR_PAD,
+	OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,	/* struct erspan_metadata */
 	__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index f143908b651d..9d00c24b2836 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -49,6 +49,7 @@
 #include <net/mpls.h>
 #include <net/vxlan.h>
 #include <net/tun_proto.h>
+#include <net/erspan.h>
 
 #include "flow_netlink.h"
 
@@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void)
 		+ nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_CSUM */
 		+ nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_OAM */
 		+ nla_total_size(256)  /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */
-		/* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with
+		/* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and
+		 * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with
 		 * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
 		 */
 		+ nla_total_size(2)    /* OVS_TUNNEL_KEY_ATTR_TP_SRC */
@@ -400,6 +402,7 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1]
 						.next = ovs_vxlan_ext_key_lens },
 	[OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = sizeof(struct in6_addr) },
 	[OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = sizeof(struct in6_addr) },
+	[OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
 };
 
 static const struct ovs_len_tbl
@@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr,
 	return 0;
 }
 
+static int erspan_tun_opt_from_nlattr(const struct nlattr *a,
+				      struct sw_flow_match *match, bool is_mask,
+				      bool log)
+{
+	unsigned long opt_key_offset;
+
+	BUILD_BUG_ON(sizeof(struct erspan_metadata) >
+		     sizeof(match->key->tun_opts));
+
+	if (nla_len(a) > sizeof(match->key->tun_opts)) {
+		OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).",
+			  nla_len(a), sizeof(match->key->tun_opts));
+		return -EINVAL;
+	}
+
+	if (!is_mask)
+		SW_FLOW_KEY_PUT(match, tun_opts_len,
+				sizeof(struct erspan_metadata), false);
+	else
+		SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true);
+
+	opt_key_offset = TUN_METADATA_OFFSET(nla_len(a));
+	SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a),
+				  nla_len(a), is_mask);
+	return 0;
+}
+
 static int ip_tun_from_nlattr(const struct nlattr *attr,
 			      struct sw_flow_match *match, bool is_mask,
 			      bool log)
@@ -738,6 +768,20 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
 			break;
 		case OVS_TUNNEL_KEY_ATTR_PAD:
 			break;
+		case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS:
+			if (opts_type) {
+				OVS_NLERR(log, "Multiple metadata blocks provided");
+				return -EINVAL;
+			}
+
+			err = erspan_tun_opt_from_nlattr(a, match, is_mask,
+							 log);
+			if (err)
+				return err;
+
+			tun_flags |= TUNNEL_ERSPAN_OPT;
+			opts_type = type;
+			break;
 		default:
 			OVS_NLERR(log, "Unknown IP tunnel attribute %d",
 				  type);
@@ -862,6 +906,10 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb,
 		else if (output->tun_flags & TUNNEL_VXLAN_OPT &&
 			 vxlan_opt_to_nlattr(skb, tun_opts, swkey_tun_opts_len))
 			return -EMSGSIZE;
+		else if (output->tun_flags & TUNNEL_ERSPAN_OPT &&
+			 nla_put(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,
+				 swkey_tun_opts_len, tun_opts))
+			return -EMSGSIZE;
 	}
 
 	return 0;
@@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key)
 	return 0;
 }
 
+static int validate_erspan_opts(struct sw_flow_key *key, bool log)
+{
+	int opts_len = key->tun_opts_len;
+	struct erspan_metadata *md;
+
+	md = (struct erspan_metadata *)TUN_METADATA_OPTS(key, opts_len);
+	if (md->version == 1) {
+		if (ntohl(md->u.index) & ~INDEX_MASK) {
+			OVS_NLERR(log,
+				  "ERSPAN index number %x too large.",
+				  ntohl(md->u.index));
+			return -EINVAL;
+		}
+	} else if (md->version == 2) {
+		struct erspan_md2 *md2 = &md->u.md2;
+		u8 hwid = get_hwid(md2);
+		u8 dir = md2->dir;
+
+		if (hwid & ~(HWID_MASK >> HWID_OFFSET)) {
+			OVS_NLERR(log, "ERSPAN hardware id %x invalid.", hwid);
+			return -EINVAL;
+		}
+
+		if (dir != 0 && dir != 1) {
+			OVS_NLERR(log, "ERSPAN direction %d invalid.", dir);
+			return -EINVAL;
+		}
+	} else {
+		OVS_NLERR(log, "ERSPAN version %d invalid.", md->version);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int validate_and_copy_set_tun(const struct nlattr *attr,
 				     struct sw_flow_actions **sfa, bool log)
 {
@@ -2486,6 +2569,11 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
 			break;
 		case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS:
 			break;
+		case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS:
+			err = validate_erspan_opts(&key, log);
+			if (err < 0)
+				return err;
+			break;
 		}
 	};
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support
  2018-01-24 19:06 ` [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support William Tu
@ 2018-01-25 17:32   ` Pravin Shelar
  2018-01-25 18:34     ` William Tu
  0 siblings, 1 reply; 6+ messages in thread
From: Pravin Shelar @ 2018-01-25 17:32 UTC (permalink / raw)
  To: William Tu; +Cc: Linux Kernel Network Developers

On Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote:
> The patch adds support for openvswitch to configure erspan
> v1 and v2.  The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added
> to uapi as a binary blob to support all ERSPAN v1 and v2's
> fields.  Note that Previous commit "openvswitch: Add erspan tunnel
> support." was reverted since it does not design properly.
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  include/uapi/linux/openvswitch.h |  2 +-
>  net/openvswitch/flow_netlink.c   | 90 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index dcfab5e3b55c..158c2e45c0a5 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -273,7 +273,6 @@ enum {
>
>  #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
>
> -
>  /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
>   */
>  enum {
> @@ -363,6 +362,7 @@ enum ovs_tunnel_key_attr {
>         OVS_TUNNEL_KEY_ATTR_IPV6_SRC,           /* struct in6_addr src IPv6 address. */
>         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
>         OVS_TUNNEL_KEY_ATTR_PAD,
> +       OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,        /* struct erspan_metadata */
>         __OVS_TUNNEL_KEY_ATTR_MAX
>  };
>
Since this is uapi, we need to define the struct erspan_metadata in a
UAPI header file.

Also lets move field 'version' to the begining of the struct for easy
expansion later.
struct erspan_metadata {
        int version;
        union {
                __be32 index;           /* Version 1 (type II)*/
                struct erspan_md2 md2;  /* Version 2 (type III) */
        } u;
};

> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index f143908b651d..9d00c24b2836 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -49,6 +49,7 @@
>  #include <net/mpls.h>
>  #include <net/vxlan.h>
>  #include <net/tun_proto.h>
> +#include <net/erspan.h>
>
>  #include "flow_netlink.h"
>
> @@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void)
>                 + nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_CSUM */
>                 + nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_OAM */
>                 + nla_total_size(256)  /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */
> -               /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with
> +               /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and
> +                * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with
>                  * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
>                  */
>                 + nla_total_size(2)    /* OVS_TUNNEL_KEY_ATTR_TP_SRC */
> @@ -400,6 +402,7 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1]
>                                                 .next = ovs_vxlan_ext_key_lens },
>         [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = sizeof(struct in6_addr) },
>         [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = sizeof(struct in6_addr) },
> +       [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
>  };
>
>  static const struct ovs_len_tbl
> @@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr,
>         return 0;
>  }
>
> +static int erspan_tun_opt_from_nlattr(const struct nlattr *a,
> +                                     struct sw_flow_match *match, bool is_mask,
> +                                     bool log)
> +{
> +       unsigned long opt_key_offset;
> +
> +       BUILD_BUG_ON(sizeof(struct erspan_metadata) >
> +                    sizeof(match->key->tun_opts));
> +
> +       if (nla_len(a) > sizeof(match->key->tun_opts)) {
> +               OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).",
> +                         nla_len(a), sizeof(match->key->tun_opts));
> +               return -EINVAL;
> +       }
> +
> +       if (!is_mask)
> +               SW_FLOW_KEY_PUT(match, tun_opts_len,
> +                               sizeof(struct erspan_metadata), false);
> +       else
> +               SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true);
> +
> +       opt_key_offset = TUN_METADATA_OFFSET(nla_len(a));
> +       SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a),
> +                                 nla_len(a), is_mask);
> +       return 0;
> +}
> +
...
> @@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key)
>         return 0;
>  }
>
> +static int validate_erspan_opts(struct sw_flow_key *key, bool log)
> +{
I do not see any need to validate ERSPAN key values, except the total
len, which should be less than 255.

> +
>  static int validate_and_copy_set_tun(const struct nlattr *attr,
>                                      struct sw_flow_actions **sfa, bool log)
>  {
> @@ -2486,6 +2569,11 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
>                         break;
>                 case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS:
>                         break;
> +               case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS:
> +                       err = validate_erspan_opts(&key, log);
> +                       if (err < 0)
> +                               return err;
> +                       break;
>                 }
>         };
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support
  2018-01-25 17:32   ` Pravin Shelar
@ 2018-01-25 18:34     ` William Tu
  2018-01-25 19:12       ` Pravin Shelar
  0 siblings, 1 reply; 6+ messages in thread
From: William Tu @ 2018-01-25 18:34 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: Linux Kernel Network Developers

Thanks for the review.

On Thu, Jan 25, 2018 at 9:32 AM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote:
>> The patch adds support for openvswitch to configure erspan
>> v1 and v2.  The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added
>> to uapi as a binary blob to support all ERSPAN v1 and v2's
>> fields.  Note that Previous commit "openvswitch: Add erspan tunnel
>> support." was reverted since it does not design properly.
>>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>> ---
>>  include/uapi/linux/openvswitch.h |  2 +-
>>  net/openvswitch/flow_netlink.c   | 90 +++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 90 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>> index dcfab5e3b55c..158c2e45c0a5 100644
>> --- a/include/uapi/linux/openvswitch.h
>> +++ b/include/uapi/linux/openvswitch.h
>> @@ -273,7 +273,6 @@ enum {
>>
>>  #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
>>
>> -
>>  /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
>>   */
>>  enum {
>> @@ -363,6 +362,7 @@ enum ovs_tunnel_key_attr {
>>         OVS_TUNNEL_KEY_ATTR_IPV6_SRC,           /* struct in6_addr src IPv6 address. */
>>         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
>>         OVS_TUNNEL_KEY_ATTR_PAD,
>> +       OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,        /* struct erspan_metadata */
>>         __OVS_TUNNEL_KEY_ATTR_MAX
>>  };
>>
> Since this is uapi, we need to define the struct erspan_metadata in a
> UAPI header file.

Should I define "struct erspan_metadata" in include/uapi/linux/openvswitch.h?

Or I'm planning to create a new file in uapi "include/uapi/linux/erspan.h",
define "struct erspan_metadata" there, and remove its duplicate at
include/net/erspan.h.

>
> Also lets move field 'version' to the begining of the struct for easy
> expansion later.
> struct erspan_metadata {
>         int version;
>         union {
>                 __be32 index;           /* Version 1 (type II)*/
>                 struct erspan_md2 md2;  /* Version 2 (type III) */
>         } u;
> };
>
Sure, will do it.

>> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
>> index f143908b651d..9d00c24b2836 100644
>> --- a/net/openvswitch/flow_netlink.c
>> +++ b/net/openvswitch/flow_netlink.c
>> @@ -49,6 +49,7 @@
>>  #include <net/mpls.h>
>>  #include <net/vxlan.h>
>>  #include <net/tun_proto.h>
>> +#include <net/erspan.h>
>>
>>  #include "flow_netlink.h"
>>
>> @@ -329,7 +330,8 @@ size_t ovs_tun_key_attr_size(void)
>>                 + nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_CSUM */
>>                 + nla_total_size(0)    /* OVS_TUNNEL_KEY_ATTR_OAM */
>>                 + nla_total_size(256)  /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */
>> -               /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS is mutually exclusive with
>> +               /* OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS and
>> +                * OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with
>>                  * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
>>                  */
>>                 + nla_total_size(2)    /* OVS_TUNNEL_KEY_ATTR_TP_SRC */
>> @@ -400,6 +402,7 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1]
>>                                                 .next = ovs_vxlan_ext_key_lens },
>>         [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = sizeof(struct in6_addr) },
>>         [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = sizeof(struct in6_addr) },
>> +       [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
>>  };
>>
>>  static const struct ovs_len_tbl
>> @@ -631,6 +634,33 @@ static int vxlan_tun_opt_from_nlattr(const struct nlattr *attr,
>>         return 0;
>>  }
>>
>> +static int erspan_tun_opt_from_nlattr(const struct nlattr *a,
>> +                                     struct sw_flow_match *match, bool is_mask,
>> +                                     bool log)
>> +{
>> +       unsigned long opt_key_offset;
>> +
>> +       BUILD_BUG_ON(sizeof(struct erspan_metadata) >
>> +                    sizeof(match->key->tun_opts));
>> +
>> +       if (nla_len(a) > sizeof(match->key->tun_opts)) {
>> +               OVS_NLERR(log, "ERSPAN option length err (len %d, max %zu).",
>> +                         nla_len(a), sizeof(match->key->tun_opts));
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (!is_mask)
>> +               SW_FLOW_KEY_PUT(match, tun_opts_len,
>> +                               sizeof(struct erspan_metadata), false);
>> +       else
>> +               SW_FLOW_KEY_PUT(match, tun_opts_len, 0xff, true);
>> +
>> +       opt_key_offset = TUN_METADATA_OFFSET(nla_len(a));
>> +       SW_FLOW_KEY_MEMCPY_OFFSET(match, opt_key_offset, nla_data(a),
>> +                                 nla_len(a), is_mask);
>> +       return 0;
>> +}
>> +
> ...
>> @@ -2461,6 +2509,41 @@ static int validate_geneve_opts(struct sw_flow_key *key)
>>         return 0;
>>  }
>>
>> +static int validate_erspan_opts(struct sw_flow_key *key, bool log)
>> +{
> I do not see any need to validate ERSPAN key values, except the total
> len, which should be less than 255.
>

OK, the total len has been checked at erspan_tun_opt_from_nlattr().
I will remove this extra validation.

Thanks
William

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support
  2018-01-25 18:34     ` William Tu
@ 2018-01-25 19:12       ` Pravin Shelar
  0 siblings, 0 replies; 6+ messages in thread
From: Pravin Shelar @ 2018-01-25 19:12 UTC (permalink / raw)
  To: William Tu; +Cc: Linux Kernel Network Developers

On Thu, Jan 25, 2018 at 10:34 AM, William Tu <u9012063@gmail.com> wrote:
> Thanks for the review.
>
> On Thu, Jan 25, 2018 at 9:32 AM, Pravin Shelar <pshelar@ovn.org> wrote:
>> On Wed, Jan 24, 2018 at 11:06 AM, William Tu <u9012063@gmail.com> wrote:
>>> The patch adds support for openvswitch to configure erspan
>>> v1 and v2.  The OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS attr is added
>>> to uapi as a binary blob to support all ERSPAN v1 and v2's
>>> fields.  Note that Previous commit "openvswitch: Add erspan tunnel
>>> support." was reverted since it does not design properly.
>>>
>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>> ---
>>>  include/uapi/linux/openvswitch.h |  2 +-
>>>  net/openvswitch/flow_netlink.c   | 90 +++++++++++++++++++++++++++++++++++++++-
>>>  2 files changed, 90 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
>>> index dcfab5e3b55c..158c2e45c0a5 100644
>>> --- a/include/uapi/linux/openvswitch.h
>>> +++ b/include/uapi/linux/openvswitch.h
>>> @@ -273,7 +273,6 @@ enum {
>>>
>>>  #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
>>>
>>> -
>>>  /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
>>>   */
>>>  enum {
>>> @@ -363,6 +362,7 @@ enum ovs_tunnel_key_attr {
>>>         OVS_TUNNEL_KEY_ATTR_IPV6_SRC,           /* struct in6_addr src IPv6 address. */
>>>         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
>>>         OVS_TUNNEL_KEY_ATTR_PAD,
>>> +       OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,        /* struct erspan_metadata */
>>>         __OVS_TUNNEL_KEY_ATTR_MAX
>>>  };
>>>
>> Since this is uapi, we need to define the struct erspan_metadata in a
>> UAPI header file.
>
> Should I define "struct erspan_metadata" in include/uapi/linux/openvswitch.h?
>
> Or I'm planning to create a new file in uapi "include/uapi/linux/erspan.h",
> define "struct erspan_metadata" there, and remove its duplicate at
> include/net/erspan.h.
>
Better to define separate header for ERSPAN.

Thanks,
Pravin.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-01-25 19:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 19:06 [PATCHv5 net-next 0/2] net: erspan: add support for openvswitch William Tu
2018-01-24 19:06 ` [PATCHv5 net-next 1/2] net: erspan: use bitfield instead of mask and offset William Tu
2018-01-24 19:06 ` [PATCHv5 net-next 2/2] openvswitch: add erspan version I and II support William Tu
2018-01-25 17:32   ` Pravin Shelar
2018-01-25 18:34     ` William Tu
2018-01-25 19:12       ` Pravin Shelar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).