netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path
@ 2016-02-02 17:09 Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 1/6] vxlan: cleanup types Jiri Benc
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

This is first part of vxlan cleanup. It makes vxlan.h better organized and
removes code duplication in the tx path. There's no change in functionality.

This is in preparation for VXLAN-GPE support. Other sets will follow with
cleanup of rx path, modifications of vxlan interface setup and
cleanup/modification of fdb handling. The goal of these patchsets is to
consolidate VXLAN extension handling (right now it's scattered all around
the code) and allow vxlan to operate in L3 mode (i.e. without inner Ethernet
headers).

The full picture (all 30 patches) can be seen at:
https://github.com/jbenc/linux-vxlan/commits/master

Changelog:
v2: added patch 5/6, fixing the udp sum problem spotted by Paolo Abeni

Jiri Benc (6):
  vxlan: cleanup types
  vxlan: remove duplicated macros
  vxlan: restructure vxlan.h definitions
  vxlan: consolidate output route calculation
  vxlan: consolidate csum flag handling
  vxlan: consolidate vxlan_xmit_skb and vxlan6_xmit_skb

 drivers/net/vxlan.c | 231 ++++++++++++++++------------------------------------
 include/net/vxlan.h | 115 +++++++++++++++-----------
 2 files changed, 136 insertions(+), 210 deletions(-)

-- 
1.8.3.1

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

* [PATCH net-next v2 1/6] vxlan: cleanup types
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 2/6] vxlan: remove duplicated macros Jiri Benc
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

include/net/vxlan.h is a kernel header, no need to prefix fixed size types
with double underscore.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 include/net/vxlan.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 0fb86442544b..5c64250619c5 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -30,15 +30,15 @@
  * [0] https://tools.ietf.org/html/draft-smith-vxlan-group-policy
  */
 struct vxlanhdr_gbp {
-	__u8	vx_flags;
+	u8	vx_flags;
 #ifdef __LITTLE_ENDIAN_BITFIELD
-	__u8	reserved_flags1:3,
+	u8	reserved_flags1:3,
 		policy_applied:1,
 		reserved_flags2:2,
 		dont_learn:1,
 		reserved_flags3:1;
 #elif defined(__BIG_ENDIAN_BITFIELD)
-	__u8	reserved_flags1:1,
+	u8	reserved_flags1:1,
 		dont_learn:1,
 		reserved_flags2:2,
 		policy_applied:1,
@@ -138,10 +138,10 @@ struct vxlan_config {
 	int			remote_ifindex;
 	int			mtu;
 	__be16			dst_port;
-	__u16			port_min;
-	__u16			port_max;
-	__u8			tos;
-	__u8			ttl;
+	u16			port_min;
+	u16			port_max;
+	u8			tos;
+	u8			ttl;
 	u32			flags;
 	unsigned long		age_interval;
 	unsigned int		addrmax;
-- 
1.8.3.1

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

* [PATCH net-next v2 2/6] vxlan: remove duplicated macros
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 1/6] vxlan: cleanup types Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 3/6] vxlan: restructure vxlan.h definitions Jiri Benc
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

VNI_HASH_BITS and VNI_HASH_SIZE are defined twice. Remove the extra
definitions.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 include/net/vxlan.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 5c64250619c5..234bf1ef2737 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -9,9 +9,6 @@
 #include <linux/udp.h>
 #include <net/dst_metadata.h>
 
-#define VNI_HASH_BITS	10
-#define VNI_HASH_SIZE	(1<<VNI_HASH_BITS)
-
 /*
  * VXLAN Group Based Policy Extension:
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-- 
1.8.3.1

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

* [PATCH net-next v2 3/6] vxlan: restructure vxlan.h definitions
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 1/6] vxlan: cleanup types Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 2/6] vxlan: remove duplicated macros Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 4/6] vxlan: consolidate output route calculation Jiri Benc
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

RCO and GBP are VXLAN extensions, not specified in RFC 7348. Because of
that, they need to be explicitly enabled when creating vxlan interface. By
default, those extensions are not used and plain VXLAN header is sent and
received.

Reflect this in vxlan.h: first, the plain VXLAN header is defined. Following
it, RCO is documented and defined, and likewise for GBP.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 include/net/vxlan.h | 104 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 63 insertions(+), 41 deletions(-)

diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 234bf1ef2737..25bd919c9ef0 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -9,14 +9,71 @@
 #include <linux/udp.h>
 #include <net/dst_metadata.h>
 
+/* VXLAN protocol (RFC 7348) header:
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |R|R|R|R|I|R|R|R|               Reserved                        |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |                VXLAN Network Identifier (VNI) |   Reserved    |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * I = VXLAN Network Identifier (VNI) present.
+ */
+struct vxlanhdr {
+	__be32 vx_flags;
+	__be32 vx_vni;
+};
+
+/* VXLAN header flags. */
+#define VXLAN_HF_VNI BIT(27)
+
+#define VXLAN_N_VID     (1u << 24)
+#define VXLAN_VID_MASK  (VXLAN_N_VID - 1)
+#define VXLAN_VNI_MASK  (VXLAN_VID_MASK << 8)
+#define VXLAN_HLEN (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
+
+#define VNI_HASH_BITS	10
+#define VNI_HASH_SIZE	(1<<VNI_HASH_BITS)
+#define FDB_HASH_BITS	8
+#define FDB_HASH_SIZE	(1<<FDB_HASH_BITS)
+
+/* Remote checksum offload for VXLAN (VXLAN_F_REMCSUM_[RT]X):
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |R|R|R|R|I|R|R|R|R|R|C|              Reserved                   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |           VXLAN Network Identifier (VNI)      |O| Csum start  |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * C = Remote checksum offload bit. When set indicates that the
+ *     remote checksum offload data is present.
+ *
+ * O = Offset bit. Indicates the checksum offset relative to
+ *     checksum start.
+ *
+ * Csum start = Checksum start divided by two.
+ *
+ * http://tools.ietf.org/html/draft-herbert-vxlan-rco
+ */
+
+/* VXLAN-RCO header flags. */
+#define VXLAN_HF_RCO BIT(21)
+
+/* Remote checksum offload header option */
+#define VXLAN_RCO_MASK  0x7f    /* Last byte of vni field */
+#define VXLAN_RCO_UDP   0x80    /* Indicate UDP RCO (TCP when not set *) */
+#define VXLAN_RCO_SHIFT 1       /* Left shift of start */
+#define VXLAN_RCO_SHIFT_MASK ((1 << VXLAN_RCO_SHIFT) - 1)
+#define VXLAN_MAX_REMCSUM_START (VXLAN_RCO_MASK << VXLAN_RCO_SHIFT)
+
 /*
- * VXLAN Group Based Policy Extension:
+ * VXLAN Group Based Policy Extension (VXLAN_F_GBP):
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * |1|-|-|-|1|-|-|-|R|D|R|R|A|R|R|R|        Group Policy ID        |
+ * |G|R|R|R|I|R|R|R|R|D|R|R|A|R|R|R|        Group Policy ID        |
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  * |                VXLAN Network Identifier (VNI) |   Reserved    |
  * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  *
+ * G = Group Policy ID present.
+ *
  * D = Don't Learn bit. When set, this bit indicates that the egress
  *     VTEP MUST NOT learn the source address of the encapsulated frame.
  *
@@ -24,7 +81,7 @@
  *     this packet. Policies MUST NOT be applied by devices when the
  *     A bit is set.
  *
- * [0] https://tools.ietf.org/html/draft-smith-vxlan-group-policy
+ * https://tools.ietf.org/html/draft-smith-vxlan-group-policy
  */
 struct vxlanhdr_gbp {
 	u8	vx_flags;
@@ -47,6 +104,9 @@ struct vxlanhdr_gbp {
 	__be32	vx_vni;
 };
 
+/* VXLAN-GBP header flags. */
+#define VXLAN_HF_GBP BIT(31)
+
 #define VXLAN_GBP_USED_BITS (VXLAN_HF_GBP | 0xFFFFFF)
 
 /* skb->mark mapping
@@ -59,44 +119,6 @@ struct vxlanhdr_gbp {
 #define VXLAN_GBP_POLICY_APPLIED	(BIT(3) << 16)
 #define VXLAN_GBP_ID_MASK		(0xFFFF)
 
-/* VXLAN protocol header:
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * |G|R|R|R|I|R|R|C|               Reserved                        |
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- * |                VXLAN Network Identifier (VNI) |   Reserved    |
- * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
- *
- * G = 1	Group Policy (VXLAN-GBP)
- * I = 1	VXLAN Network Identifier (VNI) present
- * C = 1	Remote checksum offload (RCO)
- */
-struct vxlanhdr {
-	__be32 vx_flags;
-	__be32 vx_vni;
-};
-
-/* VXLAN header flags. */
-#define VXLAN_HF_RCO BIT(21)
-#define VXLAN_HF_VNI BIT(27)
-#define VXLAN_HF_GBP BIT(31)
-
-/* Remote checksum offload header option */
-#define VXLAN_RCO_MASK  0x7f    /* Last byte of vni field */
-#define VXLAN_RCO_UDP   0x80    /* Indicate UDP RCO (TCP when not set *) */
-#define VXLAN_RCO_SHIFT 1       /* Left shift of start */
-#define VXLAN_RCO_SHIFT_MASK ((1 << VXLAN_RCO_SHIFT) - 1)
-#define VXLAN_MAX_REMCSUM_START (VXLAN_RCO_MASK << VXLAN_RCO_SHIFT)
-
-#define VXLAN_N_VID     (1u << 24)
-#define VXLAN_VID_MASK  (VXLAN_N_VID - 1)
-#define VXLAN_VNI_MASK  (VXLAN_VID_MASK << 8)
-#define VXLAN_HLEN (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
-
-#define VNI_HASH_BITS	10
-#define VNI_HASH_SIZE	(1<<VNI_HASH_BITS)
-#define FDB_HASH_BITS	8
-#define FDB_HASH_SIZE	(1<<FDB_HASH_BITS)
-
 struct vxlan_metadata {
 	u32		gbp;
 };
-- 
1.8.3.1

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

* [PATCH net-next v2 4/6] vxlan: consolidate output route calculation
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
                   ` (2 preceding siblings ...)
  2016-02-02 17:09 ` [PATCH net-next v2 3/6] vxlan: restructure vxlan.h definitions Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 5/6] vxlan: consolidate csum flag handling Jiri Benc
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

The code for output route lookup is duplicated for ndo_start_xmit and
ndo_fill_metadata_dst. Move it to a common function.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
This particular patch was already submitted but felt through cracks.
https://patchwork.ozlabs.org/patch/552197/
---
 drivers/net/vxlan.c | 77 +++++++++++++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 40 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 65439188c582..d0f7723fd776 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1847,6 +1847,27 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 	return 0;
 }
 
+static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
+				      struct sk_buff *skb, int oif, u8 tos,
+				      __be32 daddr, __be32 *saddr)
+{
+	struct rtable *rt = NULL;
+	struct flowi4 fl4;
+
+	memset(&fl4, 0, sizeof(fl4));
+	fl4.flowi4_oif = oif;
+	fl4.flowi4_tos = RT_TOS(tos);
+	fl4.flowi4_mark = skb->mark;
+	fl4.flowi4_proto = IPPROTO_UDP;
+	fl4.daddr = daddr;
+	fl4.saddr = vxlan->cfg.saddr.sin.sin_addr.s_addr;
+
+	rt = ip_route_output_key(vxlan->net, &fl4);
+	if (!IS_ERR(rt))
+		*saddr = fl4.saddr;
+	return rt;
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
 					  struct sk_buff *skb, int oif,
@@ -1928,7 +1949,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	struct sock *sk;
 	struct rtable *rt = NULL;
 	const struct iphdr *old_iph;
-	struct flowi4 fl4;
 	union vxlan_addr *dst;
 	union vxlan_addr remote_ip;
 	struct vxlan_metadata _md;
@@ -1995,6 +2015,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	if (dst->sa.sa_family == AF_INET) {
+		__be32 saddr;
+
 		if (!vxlan->vn4_sock)
 			goto drop;
 		sk = vxlan->vn4_sock->sock->sk;
@@ -2009,15 +2031,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 				flags &= ~VXLAN_F_UDP_CSUM;
 		}
 
-		memset(&fl4, 0, sizeof(fl4));
-		fl4.flowi4_oif = rdst ? rdst->remote_ifindex : 0;
-		fl4.flowi4_tos = RT_TOS(tos);
-		fl4.flowi4_mark = skb->mark;
-		fl4.flowi4_proto = IPPROTO_UDP;
-		fl4.daddr = dst->sin.sin_addr.s_addr;
-		fl4.saddr = vxlan->cfg.saddr.sin.sin_addr.s_addr;
-
-		rt = ip_route_output_key(vxlan->net, &fl4);
+		rt = vxlan_get_route(vxlan, skb,
+				     rdst ? rdst->remote_ifindex : 0, tos,
+				     dst->sin.sin_addr.s_addr, &saddr);
 		if (IS_ERR(rt)) {
 			netdev_dbg(dev, "no route to %pI4\n",
 				   &dst->sin.sin_addr.s_addr);
@@ -2049,7 +2065,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 
 		tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
 		ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
-		err = vxlan_xmit_skb(rt, sk, skb, fl4.saddr,
+		err = vxlan_xmit_skb(rt, sk, skb, saddr,
 				     dst->sin.sin_addr.s_addr, tos, ttl, df,
 				     src_port, dst_port, htonl(vni << 8), md,
 				     !net_eq(vxlan->net, dev_net(vxlan->dev)),
@@ -2390,31 +2406,6 @@ static int vxlan_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
-static int egress_ipv4_tun_info(struct net_device *dev, struct sk_buff *skb,
-				struct ip_tunnel_info *info,
-				__be16 sport, __be16 dport)
-{
-	struct vxlan_dev *vxlan = netdev_priv(dev);
-	struct rtable *rt;
-	struct flowi4 fl4;
-
-	memset(&fl4, 0, sizeof(fl4));
-	fl4.flowi4_tos = RT_TOS(info->key.tos);
-	fl4.flowi4_mark = skb->mark;
-	fl4.flowi4_proto = IPPROTO_UDP;
-	fl4.daddr = info->key.u.ipv4.dst;
-
-	rt = ip_route_output_key(vxlan->net, &fl4);
-	if (IS_ERR(rt))
-		return PTR_ERR(rt);
-	ip_rt_put(rt);
-
-	info->key.u.ipv4.src = fl4.saddr;
-	info->key.tp_src = sport;
-	info->key.tp_dst = dport;
-	return 0;
-}
-
 static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
@@ -2426,9 +2417,16 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
 	dport = info->key.tp_dst ? : vxlan->cfg.dst_port;
 
 	if (ip_tunnel_info_af(info) == AF_INET) {
+		struct rtable *rt;
+
 		if (!vxlan->vn4_sock)
 			return -EINVAL;
-		return egress_ipv4_tun_info(dev, skb, info, sport, dport);
+		rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
+				     info->key.u.ipv4.dst,
+				     &info->key.u.ipv4.src);
+		if (IS_ERR(rt))
+			return PTR_ERR(rt);
+		ip_rt_put(rt);
 	} else {
 #if IS_ENABLED(CONFIG_IPV6)
 		struct dst_entry *ndst;
@@ -2441,13 +2439,12 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
 		if (IS_ERR(ndst))
 			return PTR_ERR(ndst);
 		dst_release(ndst);
-
-		info->key.tp_src = sport;
-		info->key.tp_dst = dport;
 #else /* !CONFIG_IPV6 */
 		return -EPFNOSUPPORT;
 #endif
 	}
+	info->key.tp_src = sport;
+	info->key.tp_dst = dport;
 	return 0;
 }
 
-- 
1.8.3.1

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

* [PATCH net-next v2 5/6] vxlan: consolidate csum flag handling
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
                   ` (3 preceding siblings ...)
  2016-02-02 17:09 ` [PATCH net-next v2 4/6] vxlan: consolidate output route calculation Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-02 17:09 ` [PATCH net-next v2 6/6] vxlan: consolidate vxlan_xmit_skb and vxlan6_xmit_skb Jiri Benc
  2016-02-07 18:51 ` [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

The flag for tx checksumming for tunneling over IPv4 and IPv6 is different.
Decide whether to do tx checksumming in vxlan_xmit_one and pass it on as
a separate flag. This will allow for tx path consolidation in the next
patch.

Unfortunately, gcc is not clever enough to see that udp_sum is always
initialized and gives an uninitialized variable warning. Set it to false to
silence the warning.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
New in v2.
---
 drivers/net/vxlan.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d0f7723fd776..fe3fd4808f4d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1690,12 +1690,13 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 			   struct net_device *dev, struct in6_addr *saddr,
 			   struct in6_addr *daddr, __u8 prio, __u8 ttl,
 			   __be16 src_port, __be16 dst_port, __be32 vni,
-			   struct vxlan_metadata *md, bool xnet, u32 vxflags)
+			   struct vxlan_metadata *md, bool xnet, u32 vxflags,
+			   bool udp_sum)
 {
 	struct vxlanhdr *vxh;
 	int min_headroom;
 	int err;
-	bool udp_sum = !(vxflags & VXLAN_F_UDP_ZERO_CSUM6_TX);
+	bool nocheck = !udp_sum;
 	int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 	u16 hdrlen = sizeof(struct vxlanhdr);
 
@@ -1763,8 +1764,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
 
 	udp_tunnel6_xmit_skb(dst, sk, skb, dev, saddr, daddr, prio,
-			     ttl, src_port, dst_port,
-			     !!(vxflags & VXLAN_F_UDP_ZERO_CSUM6_TX));
+			     ttl, src_port, dst_port, nocheck);
 	return 0;
 err:
 	dst_release(dst);
@@ -1775,12 +1775,13 @@ err:
 static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
 			  __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,
 			  __be16 src_port, __be16 dst_port, __be32 vni,
-			  struct vxlan_metadata *md, bool xnet, u32 vxflags)
+			  struct vxlan_metadata *md, bool xnet, u32 vxflags,
+			  bool udp_sum)
 {
 	struct vxlanhdr *vxh;
 	int min_headroom;
 	int err;
-	bool udp_sum = !!(vxflags & VXLAN_F_UDP_CSUM);
+	bool nocheck = !udp_sum;
 	int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 	u16 hdrlen = sizeof(struct vxlanhdr);
 
@@ -1842,8 +1843,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
 
 	udp_tunnel_xmit_skb(rt, sk, skb, src, dst, tos, ttl, df,
-			    src_port, dst_port, xnet,
-			    !(vxflags & VXLAN_F_UDP_CSUM));
+			    src_port, dst_port, xnet, nocheck);
 	return 0;
 }
 
@@ -1959,6 +1959,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	__u8 tos, ttl;
 	int err;
 	u32 flags = vxlan->flags;
+	bool udp_sum = false;
 
 	info = skb_tunnel_info(skb);
 
@@ -2007,6 +2008,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	if (info) {
 		ttl = info->key.ttl;
 		tos = info->key.tos;
+		udp_sum = !!(info->key.tun_flags & TUNNEL_CSUM);
 
 		if (info->options_len)
 			md = ip_tunnel_info_opts(info);
@@ -2024,11 +2026,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		if (info) {
 			if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT)
 				df = htons(IP_DF);
-
-			if (info->key.tun_flags & TUNNEL_CSUM)
-				flags |= VXLAN_F_UDP_CSUM;
-			else
-				flags &= ~VXLAN_F_UDP_CSUM;
+		} else {
+			udp_sum = !!(flags & VXLAN_F_UDP_CSUM);
 		}
 
 		rt = vxlan_get_route(vxlan, skb,
@@ -2069,7 +2068,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 				     dst->sin.sin_addr.s_addr, tos, ttl, df,
 				     src_port, dst_port, htonl(vni << 8), md,
 				     !net_eq(vxlan->net, dev_net(vxlan->dev)),
-				     flags);
+				     flags, udp_sum);
 		if (err < 0) {
 			/* skb is already freed. */
 			skb = NULL;
@@ -2119,18 +2118,14 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 			return;
 		}
 
-		if (info) {
-			if (info->key.tun_flags & TUNNEL_CSUM)
-				flags &= ~VXLAN_F_UDP_ZERO_CSUM6_TX;
-			else
-				flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
-		}
+		if (!info)
+			udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
 
 		ttl = ttl ? : ip6_dst_hoplimit(ndst);
 		err = vxlan6_xmit_skb(ndst, sk, skb, dev, &saddr, &dst->sin6.sin6_addr,
 				      0, ttl, src_port, dst_port, htonl(vni << 8), md,
 				      !net_eq(vxlan->net, dev_net(vxlan->dev)),
-				      flags);
+				      flags, udp_sum);
 #endif
 	}
 
-- 
1.8.3.1

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

* [PATCH net-next v2 6/6] vxlan: consolidate vxlan_xmit_skb and vxlan6_xmit_skb
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
                   ` (4 preceding siblings ...)
  2016-02-02 17:09 ` [PATCH net-next v2 5/6] vxlan: consolidate csum flag handling Jiri Benc
@ 2016-02-02 17:09 ` Jiri Benc
  2016-02-07 18:51 ` [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Jiri Benc @ 2016-02-02 17:09 UTC (permalink / raw)
  To: netdev

There's a lot of code duplication. Factor out the duplicate code to a new
function shared between IPv4 and IPv6 xmit path.

Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
v2: rebased on top of the added patch.
---
 drivers/net/vxlan.c | 137 ++++++++++------------------------------------------
 1 file changed, 26 insertions(+), 111 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index fe3fd4808f4d..65f52472a52c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1684,19 +1684,14 @@ static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, u32 vxflags,
 	gbp->policy_id = htons(md->gbp & VXLAN_GBP_ID_MASK);
 }
 
-#if IS_ENABLED(CONFIG_IPV6)
-static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
-			   struct sk_buff *skb,
-			   struct net_device *dev, struct in6_addr *saddr,
-			   struct in6_addr *daddr, __u8 prio, __u8 ttl,
-			   __be16 src_port, __be16 dst_port, __be32 vni,
-			   struct vxlan_metadata *md, bool xnet, u32 vxflags,
+static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
+			   int iphdr_len, __be32 vni,
+			   struct vxlan_metadata *md, u32 vxflags,
 			   bool udp_sum)
 {
 	struct vxlanhdr *vxh;
 	int min_headroom;
 	int err;
-	bool nocheck = !udp_sum;
 	int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 	u16 hdrlen = sizeof(struct vxlanhdr);
 
@@ -1713,93 +1708,8 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk,
 		}
 	}
 
-	skb_scrub_packet(skb, xnet);
-
 	min_headroom = LL_RESERVED_SPACE(dst->dev) + dst->header_len
-			+ VXLAN_HLEN + sizeof(struct ipv6hdr)
-			+ (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0);
-
-	/* Need space for new headers (invalidates iph ptr) */
-	err = skb_cow_head(skb, min_headroom);
-	if (unlikely(err)) {
-		kfree_skb(skb);
-		goto err;
-	}
-
-	skb = vlan_hwaccel_push_inside(skb);
-	if (WARN_ON(!skb)) {
-		err = -ENOMEM;
-		goto err;
-	}
-
-	skb = iptunnel_handle_offloads(skb, udp_sum, type);
-	if (IS_ERR(skb)) {
-		err = -EINVAL;
-		goto err;
-	}
-
-	vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
-	vxh->vx_flags = htonl(VXLAN_HF_VNI);
-	vxh->vx_vni = vni;
-
-	if (type & SKB_GSO_TUNNEL_REMCSUM) {
-		u32 data = (skb_checksum_start_offset(skb) - hdrlen) >>
-			   VXLAN_RCO_SHIFT;
-
-		if (skb->csum_offset == offsetof(struct udphdr, check))
-			data |= VXLAN_RCO_UDP;
-
-		vxh->vx_vni |= htonl(data);
-		vxh->vx_flags |= htonl(VXLAN_HF_RCO);
-
-		if (!skb_is_gso(skb)) {
-			skb->ip_summed = CHECKSUM_NONE;
-			skb->encapsulation = 0;
-		}
-	}
-
-	if (vxflags & VXLAN_F_GBP)
-		vxlan_build_gbp_hdr(vxh, vxflags, md);
-
-	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
-
-	udp_tunnel6_xmit_skb(dst, sk, skb, dev, saddr, daddr, prio,
-			     ttl, src_port, dst_port, nocheck);
-	return 0;
-err:
-	dst_release(dst);
-	return err;
-}
-#endif
-
-static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb,
-			  __be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,
-			  __be16 src_port, __be16 dst_port, __be32 vni,
-			  struct vxlan_metadata *md, bool xnet, u32 vxflags,
-			  bool udp_sum)
-{
-	struct vxlanhdr *vxh;
-	int min_headroom;
-	int err;
-	bool nocheck = !udp_sum;
-	int type = udp_sum ? SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
-	u16 hdrlen = sizeof(struct vxlanhdr);
-
-	if ((vxflags & VXLAN_F_REMCSUM_TX) &&
-	    skb->ip_summed == CHECKSUM_PARTIAL) {
-		int csum_start = skb_checksum_start_offset(skb);
-
-		if (csum_start <= VXLAN_MAX_REMCSUM_START &&
-		    !(csum_start & VXLAN_RCO_SHIFT_MASK) &&
-		    (skb->csum_offset == offsetof(struct udphdr, check) ||
-		     skb->csum_offset == offsetof(struct tcphdr, check))) {
-			udp_sum = false;
-			type |= SKB_GSO_TUNNEL_REMCSUM;
-		}
-	}
-
-	min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
-			+ VXLAN_HLEN + sizeof(struct iphdr)
+			+ VXLAN_HLEN + iphdr_len
 			+ (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0);
 
 	/* Need space for new headers (invalidates iph ptr) */
@@ -1841,9 +1751,6 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk
 		vxlan_build_gbp_hdr(vxh, vxflags, md);
 
 	skb_set_inner_protocol(skb, htons(ETH_P_TEB));
-
-	udp_tunnel_xmit_skb(rt, sk, skb, src, dst, tos, ttl, df,
-			    src_port, dst_port, xnet, nocheck);
 	return 0;
 }
 
@@ -1960,6 +1867,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	int err;
 	u32 flags = vxlan->flags;
 	bool udp_sum = false;
+	bool xnet = !net_eq(vxlan->net, dev_net(vxlan->dev));
 
 	info = skb_tunnel_info(skb);
 
@@ -2064,16 +1972,14 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 
 		tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
 		ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
-		err = vxlan_xmit_skb(rt, sk, skb, saddr,
-				     dst->sin.sin_addr.s_addr, tos, ttl, df,
-				     src_port, dst_port, htonl(vni << 8), md,
-				     !net_eq(vxlan->net, dev_net(vxlan->dev)),
-				     flags, udp_sum);
-		if (err < 0) {
-			/* skb is already freed. */
-			skb = NULL;
-			goto rt_tx_error;
-		}
+		err = vxlan_build_skb(skb, &rt->dst, sizeof(struct iphdr),
+				      htonl(vni << 8), md, flags, udp_sum);
+		if (err < 0)
+			goto xmit_tx_error;
+
+		udp_tunnel_xmit_skb(rt, sk, skb, saddr,
+				    dst->sin.sin_addr.s_addr, tos, ttl, df,
+				    src_port, dst_port, xnet, !udp_sum);
 #if IS_ENABLED(CONFIG_IPV6)
 	} else {
 		struct dst_entry *ndst;
@@ -2122,10 +2028,16 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 			udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
 
 		ttl = ttl ? : ip6_dst_hoplimit(ndst);
-		err = vxlan6_xmit_skb(ndst, sk, skb, dev, &saddr, &dst->sin6.sin6_addr,
-				      0, ttl, src_port, dst_port, htonl(vni << 8), md,
-				      !net_eq(vxlan->net, dev_net(vxlan->dev)),
-				      flags, udp_sum);
+		skb_scrub_packet(skb, xnet);
+		err = vxlan_build_skb(skb, ndst, sizeof(struct ipv6hdr),
+				      htonl(vni << 8), md, flags, udp_sum);
+		if (err < 0) {
+			dst_release(ndst);
+			return;
+		}
+		udp_tunnel6_xmit_skb(ndst, sk, skb, dev,
+				     &saddr, &dst->sin6.sin6_addr,
+				     0, ttl, src_port, dst_port, !udp_sum);
 #endif
 	}
 
@@ -2135,6 +2047,9 @@ drop:
 	dev->stats.tx_dropped++;
 	goto tx_free;
 
+xmit_tx_error:
+	/* skb is already freed. */
+	skb = NULL;
 rt_tx_error:
 	ip_rt_put(rt);
 tx_error:
-- 
1.8.3.1

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

* Re: [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path
  2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
                   ` (5 preceding siblings ...)
  2016-02-02 17:09 ` [PATCH net-next v2 6/6] vxlan: consolidate vxlan_xmit_skb and vxlan6_xmit_skb Jiri Benc
@ 2016-02-07 18:51 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-02-07 18:51 UTC (permalink / raw)
  To: jbenc; +Cc: netdev

From: Jiri Benc <jbenc@redhat.com>
Date: Tue,  2 Feb 2016 18:09:10 +0100

> This is first part of vxlan cleanup. It makes vxlan.h better organized and
> removes code duplication in the tx path. There's no change in functionality.
> 
> This is in preparation for VXLAN-GPE support. Other sets will follow with
> cleanup of rx path, modifications of vxlan interface setup and
> cleanup/modification of fdb handling. The goal of these patchsets is to
> consolidate VXLAN extension handling (right now it's scattered all around
> the code) and allow vxlan to operate in L3 mode (i.e. without inner Ethernet
> headers).
> 
> The full picture (all 30 patches) can be seen at:
> https://github.com/jbenc/linux-vxlan/commits/master
> 
> Changelog:
> v2: added patch 5/6, fixing the udp sum problem spotted by Paolo Abeni

Applied, thanks.

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

end of thread, other threads:[~2016-02-07 18:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-02 17:09 [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 1/6] vxlan: cleanup types Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 2/6] vxlan: remove duplicated macros Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 3/6] vxlan: restructure vxlan.h definitions Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 4/6] vxlan: consolidate output route calculation Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 5/6] vxlan: consolidate csum flag handling Jiri Benc
2016-02-02 17:09 ` [PATCH net-next v2 6/6] vxlan: consolidate vxlan_xmit_skb and vxlan6_xmit_skb Jiri Benc
2016-02-07 18:51 ` [PATCH net-next v2 0/6] vxlan: cleanup headers and tx path David Miller

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).