netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [rfc 0/3] IPVS: checksum updates
@ 2008-09-08  2:04 Simon Horman
  2008-09-08  2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
                   ` (4 more replies)
  0 siblings, 5 replies; 28+ messages in thread
From: Simon Horman @ 2008-09-08  2:04 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: Siim Põder, Julian Anastasov, Malcolm Turnbull,
	Julius Volz, Vince Busam, Herbert Xu

Hi,

The impetus for this series of patches is Julian Anastasov noting
that "load balance IPv4 connections from a local process" checks
for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
even on loopback traffic, but that rather partial checksums are
possible.

The first patch in this series is a proposed solution to handle
partial checksums for both TCP and UDP.

The other two patches clean things up a bit.

I have not tested this code beyond compilation yet.

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

* [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM
  2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
@ 2008-09-08  2:04 ` Simon Horman
  2008-09-08  7:24   ` Herbert Xu
  2008-09-08  2:04 ` [rfc 2/3] ipvs: Use inet_proto_csum_replace*() Simon Horman
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08  2:04 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: Siim Põder, Julian Anastasov, Malcolm Turnbull,
	Julius Volz, Vince Busam, Herbert Xu

[-- Attachment #1: ipvs-partial_csum-update.patch --]
[-- Type: text/plain, Size: 6335 bytes --]

Now that LVS can load balance locally generated traffic, packets may come
from the loopback device and thus may have a partial checksum.

The existing code allows for the case where there is no checksum at all for
TCP, however Herbert Xu has confirmed that this is not legal.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

This patch implements *_partial_csum_update() in the style
of the existing *_fast_csum_update() code. A subsequent patch
will reimplement these functions in terms of the more standard
inet_proto_csum_replace*() functions.

 net/ipv4/ipvs/ip_vs_proto_tcp.c |   37 +++++++++++++++++++++++++++++++++++--
 net/ipv4/ipvs/ip_vs_proto_udp.c |   37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 70 insertions(+), 4 deletions(-)
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 11:46:28.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 11:56:10.000000000 +1000
@@ -134,12 +134,34 @@ tcp_fast_csum_update(int af, struct tcph
 }
 
 
+static inline void
+tcp_partial_csum_update(int af, struct tcphdr *tcph,
+		     const union nf_inet_addr *oldip,
+		     const union nf_inet_addr *newip,
+		     __be16 oldlen, __be16 newlen)
+{
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6)
+		tcph->check =
+			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
+					 ip_vs_check_diff2(oldlen, newlen,
+						~csum_unfold(tcph->check))));
+	else
+#endif
+	tcph->check =
+		csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
+				ip_vs_check_diff2(oldlen, newlen,
+						~csum_unfold(tcph->check))));
+}
+
+
 static int
 tcp_snat_handler(struct sk_buff *skb,
 		 struct ip_vs_protocol *pp, struct ip_vs_conn *cp)
 {
 	struct tcphdr *tcph;
 	unsigned int tcphoff;
+	int oldlen;
 
 #ifdef CONFIG_IP_VS_IPV6
 	if (cp->af == AF_INET6)
@@ -147,6 +169,7 @@ tcp_snat_handler(struct sk_buff *skb,
 	else
 #endif
 		tcphoff = ip_hdrlen(skb);
+	oldlen = skb->len - tcphoff;
 
 	/* csum_check requires unshared skb */
 	if (!skb_make_writable(skb, tcphoff+sizeof(*tcph)))
@@ -166,7 +189,11 @@ tcp_snat_handler(struct sk_buff *skb,
 	tcph->source = cp->vport;
 
 	/* Adjust TCP checksums */
-	if (!cp->app && (tcph->check != 0)) {
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
+					htonl(oldlen),
+					htonl(skb->len - tcphoff));
+	} else if (!cp->app) {
 		/* Only port and addr are changed, do fast csum update */
 		tcp_fast_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
 				     cp->dport, cp->vport);
@@ -204,6 +231,7 @@ tcp_dnat_handler(struct sk_buff *skb,
 {
 	struct tcphdr *tcph;
 	unsigned int tcphoff;
+	int oldlen;
 
 #ifdef CONFIG_IP_VS_IPV6
 	if (cp->af == AF_INET6)
@@ -211,6 +239,7 @@ tcp_dnat_handler(struct sk_buff *skb,
 	else
 #endif
 		tcphoff = ip_hdrlen(skb);
+	oldlen = skb->len - tcphoff;
 
 	/* csum_check requires unshared skb */
 	if (!skb_make_writable(skb, tcphoff+sizeof(*tcph)))
@@ -235,7 +264,11 @@ tcp_dnat_handler(struct sk_buff *skb,
 	/*
 	 *	Adjust TCP checksums
 	 */
-	if (!cp->app && (tcph->check != 0)) {
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
+					htonl(oldlen),
+					htonl(skb->len - tcphoff));
+	} else if (!cp->app) {
 		/* Only port and addr are changed, do fast csum update */
 		tcp_fast_csum_update(cp->af, tcph, &cp->vaddr, &cp->daddr,
 				     cp->vport, cp->dport);
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 11:46:28.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 11:56:10.000000000 +1000
@@ -141,12 +141,34 @@ udp_fast_csum_update(int af, struct udph
 		uhdr->check = CSUM_MANGLED_0;
 }
 
+static inline void
+udp_partial_csum_update(int af, struct udphdr *uhdr,
+		     const union nf_inet_addr *oldip,
+		     const union nf_inet_addr *newip,
+		     __be16 oldlen, __be16 newlen)
+{
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6)
+		uhdr->check =
+			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
+					 ip_vs_check_diff2(oldlen, newlen,
+						~csum_unfold(uhdr->check))));
+	else
+#endif
+	uhdr->check =
+		csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
+				ip_vs_check_diff2(oldlen, newlen,
+						~csum_unfold(uhdr->check))));
+}
+
+
 static int
 udp_snat_handler(struct sk_buff *skb,
 		 struct ip_vs_protocol *pp, struct ip_vs_conn *cp)
 {
 	struct udphdr *udph;
 	unsigned int udphoff;
+	int oldlen;
 
 #ifdef CONFIG_IP_VS_IPV6
 	if (cp->af == AF_INET6)
@@ -154,6 +176,7 @@ udp_snat_handler(struct sk_buff *skb,
 	else
 #endif
 		udphoff = ip_hdrlen(skb);
+	oldlen = skb->len - udphoff;
 
 	/* csum_check requires unshared skb */
 	if (!skb_make_writable(skb, udphoff+sizeof(*udph)))
@@ -177,7 +200,11 @@ udp_snat_handler(struct sk_buff *skb,
 	/*
 	 *	Adjust UDP checksums
 	 */
-	if (!cp->app && (udph->check != 0)) {
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		udp_partial_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
+					htonl(oldlen),
+					htonl(skb->len - udphoff));
+	} else if (!cp->app && (udph->check != 0)) {
 		/* Only port and addr are changed, do fast csum update */
 		udp_fast_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
 				     cp->dport, cp->vport);
@@ -216,6 +243,7 @@ udp_dnat_handler(struct sk_buff *skb,
 {
 	struct udphdr *udph;
 	unsigned int udphoff;
+	int oldlen;
 
 #ifdef CONFIG_IP_VS_IPV6
 	if (cp->af == AF_INET6)
@@ -223,6 +251,7 @@ udp_dnat_handler(struct sk_buff *skb,
 	else
 #endif
 		udphoff = ip_hdrlen(skb);
+	oldlen = skb->len - udphoff;
 
 	/* csum_check requires unshared skb */
 	if (!skb_make_writable(skb, udphoff+sizeof(*udph)))
@@ -247,7 +276,11 @@ udp_dnat_handler(struct sk_buff *skb,
 	/*
 	 *	Adjust UDP checksums
 	 */
-	if (!cp->app && (udph->check != 0)) {
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		udp_partial_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
+					htonl(oldlen),
+					htonl(skb->len - udphoff));
+	} else if (!cp->app && (udph->check != 0)) {
 		/* Only port and addr are changed, do fast csum update */
 		udp_fast_csum_update(cp->af, udph, &cp->vaddr, &cp->daddr,
 				     cp->vport, cp->dport);

-- 

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

* [rfc 2/3] ipvs: Use inet_proto_csum_replace*()
  2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
  2008-09-08  2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
@ 2008-09-08  2:04 ` Simon Horman
  2008-09-08  2:04 ` [rfc 3/3] ipvs: Consolidate checksuming code Simon Horman
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 28+ messages in thread
From: Simon Horman @ 2008-09-08  2:04 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: Siim Põder, Julian Anastasov, Malcolm Turnbull,
	Julius Volz, Vince Busam, Herbert Xu

[-- Attachment #1: inet_proto_csum_replace16.patch --]
[-- Type: text/plain, Size: 10944 bytes --]

Implement IPVS checksuming functions using the common
inet_proto_csum_replace*() functions. This patch adds
inet_proto_csum_replace16() to handle IPv6, which isn't
provided by netfilter as it doesn't handle IPv6 NAT.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 include/net/ip_vs.h             |   31 +++---------------
 net/ipv4/ipvs/ip_vs_proto.c     |   23 +++++++++++++
 net/ipv4/ipvs/ip_vs_proto_tcp.c |   62 +++++++++++++++---------------------
 net/ipv4/ipvs/ip_vs_proto_udp.c |   67 +++++++++++++++++----------------------
 4 files changed, 86 insertions(+), 97 deletions(-)
Index: lvs-2.6/include/net/ip_vs.h
===================================================================
--- lvs-2.6.orig/include/net/ip_vs.h	2008-09-08 10:55:51.000000000 +1000
+++ lvs-2.6/include/net/ip_vs.h	2008-09-08 11:16:55.000000000 +1000
@@ -932,34 +932,15 @@ extern void ip_vs_nat_icmp(struct sk_buf
 #ifdef CONFIG_IP_VS_IPV6
 extern void ip_vs_nat_icmp_v6(struct sk_buff *skb, struct ip_vs_protocol *pp,
 			      struct ip_vs_conn *cp, int dir);
-#endif
-
-extern __sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset);
-
-static inline __wsum ip_vs_check_diff4(__be32 old, __be32 new, __wsum oldsum)
-{
-	__be32 diff[2] = { ~old, new };
-
-	return csum_partial((char *) diff, sizeof(diff), oldsum);
-}
 
-#ifdef CONFIG_IP_VS_IPV6
-static inline __wsum ip_vs_check_diff16(const __be32 *old, const __be32 *new,
-					__wsum oldsum)
-{
-	__be32 diff[8] = { ~old[3], ~old[2], ~old[1], ~old[0],
-			    new[3],  new[2],  new[1],  new[0] };
-
-	return csum_partial((char *) diff, sizeof(diff), oldsum);
-}
+#if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
+extern void inet_proto_csum_replace16(__sum16 *sum, struct sk_buff *skb,
+				      __be32 from[4], __be32 to[4],
+				      int pseudohdr);
+#endif
 #endif
 
-static inline __wsum ip_vs_check_diff2(__be16 old, __be16 new, __wsum oldsum)
-{
-	__be16 diff[2] = { ~old, new };
-
-	return csum_partial((char *) diff, sizeof(diff), oldsum);
-}
+extern __sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset);
 
 #endif /* __KERNEL__ */
 
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto.c	2008-09-08 10:55:51.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto.c	2008-09-08 11:16:55.000000000 +1000
@@ -286,3 +286,26 @@ void ip_vs_protocol_cleanup(void)
 			unregister_ip_vs_protocol(pp);
 	}
 }
+
+
+#if defined(CONFIG_IP_VS_IPV6) &&					\
+	(defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP))
+void
+inet_proto_csum_replace16(__sum16 *sum, struct sk_buff *skb,
+			  __be32 from[4], __be32 to[4], int pseudohdr)
+{
+	__be32 diff[8] = { ~from[3], ~from[2], ~from[1], ~from[0],
+			   to[3],  to[2],  to[1], to[0] };
+
+	if (skb->ip_summed != CHECKSUM_PARTIAL) {
+		*sum = csum_fold(csum_partial(diff, sizeof(diff),
+				~csum_unfold(*sum)));
+		if (skb->ip_summed == CHECKSUM_COMPLETE && pseudohdr)
+			skb->csum = ~csum_partial(diff, sizeof(diff),
+						~skb->csum);
+	} else if (pseudohdr)
+		*sum = ~csum_fold(csum_partial(diff, sizeof(diff),
+				csum_unfold(*sum)));
+}
+#endif
+
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 10:56:14.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 11:16:55.000000000 +1000
@@ -114,44 +114,36 @@ tcp_conn_schedule(int af, struct sk_buff
 
 
 static inline void
-tcp_fast_csum_update(int af, struct tcphdr *tcph,
-		     const union nf_inet_addr *oldip,
-		     const union nf_inet_addr *newip,
-		     __be16 oldport, __be16 newport)
+__tcp_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
+		     __be16 old2, __be16 new2, int pseudo2)
 {
 #ifdef CONFIG_IP_VS_IPV6
 	if (af == AF_INET6)
-		tcph->check =
-			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
-					 ip_vs_check_diff2(oldport, newport,
-						~csum_unfold(tcph->check))));
+		inet_proto_csum_replace16(sum, skb, oldip->ip6, newip->ip6, 1);
 	else
 #endif
-	tcph->check =
-		csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
-				 ip_vs_check_diff2(oldport, newport,
-						~csum_unfold(tcph->check))));
+		inet_proto_csum_replace4(sum, skb, oldip->ip, newip->ip, 1);
+	if (old2 != new2)
+		inet_proto_csum_replace2(sum, skb, old2, new2, pseudo2);
 }
 
 
 static inline void
-tcp_partial_csum_update(int af, struct tcphdr *tcph,
-		     const union nf_inet_addr *oldip,
-		     const union nf_inet_addr *newip,
+tcp_fast_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
+		     __be16 oldport, __be16 newport)
+{
+	__tcp_csum_update(af, sum, skb, oldip, newip, oldport, newport, 0);
+}
+
+
+static inline void
+tcp_partial_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
 		     __be16 oldlen, __be16 newlen)
 {
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6)
-		tcph->check =
-			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
-					 ip_vs_check_diff2(oldlen, newlen,
-						~csum_unfold(tcph->check))));
-	else
-#endif
-	tcph->check =
-		csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
-				ip_vs_check_diff2(oldlen, newlen,
-						~csum_unfold(tcph->check))));
+	__tcp_csum_update(af, sum, skb, oldip, newip, oldlen, newlen, 1);
 }
 
 
@@ -190,13 +182,13 @@ tcp_snat_handler(struct sk_buff *skb,
 
 	/* Adjust TCP checksums */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
-					htonl(oldlen),
+		tcp_partial_csum_update(cp->af, &tcph->check, skb, &cp->daddr,
+					&cp->vaddr, htonl(oldlen),
 					htonl(skb->len - tcphoff));
 	} else if (!cp->app) {
 		/* Only port and addr are changed, do fast csum update */
-		tcp_fast_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
-				     cp->dport, cp->vport);
+		tcp_fast_csum_update(cp->af, &tcph->check, skb, &cp->daddr,
+				     &cp->vaddr, cp->dport, cp->vport);
 		if (skb->ip_summed == CHECKSUM_COMPLETE)
 			skb->ip_summed = CHECKSUM_NONE;
 	} else {
@@ -265,13 +257,13 @@ tcp_dnat_handler(struct sk_buff *skb,
 	 *	Adjust TCP checksums
 	 */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
-					htonl(oldlen),
+		tcp_partial_csum_update(cp->af, &tcph->check, skb, &cp->daddr,
+					&cp->vaddr, htonl(oldlen),
 					htonl(skb->len - tcphoff));
 	} else if (!cp->app) {
 		/* Only port and addr are changed, do fast csum update */
-		tcp_fast_csum_update(cp->af, tcph, &cp->vaddr, &cp->daddr,
-				     cp->vport, cp->dport);
+		tcp_fast_csum_update(cp->af, &tcph->check, skb, &cp->vaddr,
+				     &cp->daddr, cp->vport, cp->dport);
 		if (skb->ip_summed == CHECKSUM_COMPLETE)
 			skb->ip_summed = CHECKSUM_NONE;
 	} else {
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 10:56:14.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 11:16:55.000000000 +1000
@@ -120,45 +120,38 @@ udp_conn_schedule(int af, struct sk_buff
 
 
 static inline void
-udp_fast_csum_update(int af, struct udphdr *uhdr,
-		     const union nf_inet_addr *oldip,
-		     const union nf_inet_addr *newip,
-		     __be16 oldport, __be16 newport)
+__udp_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		       union nf_inet_addr *oldip, union nf_inet_addr *newip,
+		       __be16 old2, __be16 new2, int pseudo2)
 {
 #ifdef CONFIG_IP_VS_IPV6
 	if (af == AF_INET6)
-		uhdr->check =
-			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
-					 ip_vs_check_diff2(oldport, newport,
-						~csum_unfold(uhdr->check))));
+		inet_proto_csum_replace16(sum, skb, oldip->ip6, newip->ip6, 1);
 	else
 #endif
-		uhdr->check =
-			csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
-					 ip_vs_check_diff2(oldport, newport,
-						~csum_unfold(uhdr->check))));
-	if (!uhdr->check)
-		uhdr->check = CSUM_MANGLED_0;
+		inet_proto_csum_replace4(sum, skb, oldip->ip, newip->ip, 1);
+	if (old2 != new2)
+		inet_proto_csum_replace2(sum, skb, old2, new2, pseudo2);
+	if (!*sum)
+		*sum = CSUM_MANGLED_0;
+}
+
+
+static inline void
+udp_fast_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
+		     __be16 oldport, __be16 newport)
+{
+	__udp_csum_update(af, sum, skb, oldip, newip, oldport, newport, 0);
 }
 
+
 static inline void
-udp_partial_csum_update(int af, struct udphdr *uhdr,
-		     const union nf_inet_addr *oldip,
-		     const union nf_inet_addr *newip,
+udp_partial_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
 		     __be16 oldlen, __be16 newlen)
 {
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6)
-		uhdr->check =
-			csum_fold(ip_vs_check_diff16(oldip->ip6, newip->ip6,
-					 ip_vs_check_diff2(oldlen, newlen,
-						~csum_unfold(uhdr->check))));
-	else
-#endif
-	uhdr->check =
-		csum_fold(ip_vs_check_diff4(oldip->ip, newip->ip,
-				ip_vs_check_diff2(oldlen, newlen,
-						~csum_unfold(uhdr->check))));
+	__udp_csum_update(af, sum, skb, oldip, newip, oldlen, newlen, 1);
 }
 
 
@@ -201,13 +194,13 @@ udp_snat_handler(struct sk_buff *skb,
 	 *	Adjust UDP checksums
 	 */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		udp_partial_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
-					htonl(oldlen),
+		udp_partial_csum_update(cp->af, &udph->check, skb, &cp->daddr,
+					&cp->vaddr, htonl(oldlen),
 					htonl(skb->len - udphoff));
 	} else if (!cp->app && (udph->check != 0)) {
 		/* Only port and addr are changed, do fast csum update */
-		udp_fast_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
-				     cp->dport, cp->vport);
+		udp_fast_csum_update(cp->af, &udph->check, skb, &cp->daddr,
+				     &cp->vaddr, cp->dport, cp->vport);
 		if (skb->ip_summed == CHECKSUM_COMPLETE)
 			skb->ip_summed = CHECKSUM_NONE;
 	} else {
@@ -277,13 +270,13 @@ udp_dnat_handler(struct sk_buff *skb,
 	 *	Adjust UDP checksums
 	 */
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		udp_partial_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
-					htonl(oldlen),
+		udp_partial_csum_update(cp->af, &udph->check, skb, &cp->daddr,
+					&cp->vaddr, htonl(oldlen),
 					htonl(skb->len - udphoff));
 	} else if (!cp->app && (udph->check != 0)) {
 		/* Only port and addr are changed, do fast csum update */
-		udp_fast_csum_update(cp->af, udph, &cp->vaddr, &cp->daddr,
-				     cp->vport, cp->dport);
+		udp_fast_csum_update(cp->af, &udph->check, skb, &cp->vaddr,
+				     &cp->daddr, cp->vport, cp->dport);
 		if (skb->ip_summed == CHECKSUM_COMPLETE)
 			skb->ip_summed = CHECKSUM_NONE;
 	} else {

-- 

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

* [rfc 3/3] ipvs: Consolidate checksuming code
  2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
  2008-09-08  2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
  2008-09-08  2:04 ` [rfc 2/3] ipvs: Use inet_proto_csum_replace*() Simon Horman
@ 2008-09-08  2:04 ` Simon Horman
  2008-09-08 10:03 ` [rfc 0/3] IPVS: checksum updates Julius Volz
  2008-09-08 23:40 ` Simon Horman
  4 siblings, 0 replies; 28+ messages in thread
From: Simon Horman @ 2008-09-08  2:04 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: Siim Põder, Julian Anastasov, Malcolm Turnbull,
	Julius Volz, Vince Busam, Herbert Xu

[-- Attachment #1: ip_vs_fast_csum_update.patch --]
[-- Type: text/plain, Size: 6678 bytes --]

I'm not sure if this is a good idea, but a lot
of code is dublicated both within and between
ip_vs_proto_udp.c and ip_vs_proto_tcp.c.

This removes a small ammount of duplication.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 include/net/ip_vs.h             |   10 ++++++----
 net/ipv4/ipvs/ip_vs_proto.c     |   23 ++++++++++++++++++++---
 net/ipv4/ipvs/ip_vs_proto_tcp.c |   25 +++----------------------
 net/ipv4/ipvs/ip_vs_proto_udp.c |   14 ++------------
 4 files changed, 31 insertions(+), 41 deletions(-)
Index: lvs-2.6/include/net/ip_vs.h
===================================================================
--- lvs-2.6.orig/include/net/ip_vs.h	2008-09-08 10:56:29.000000000 +1000
+++ lvs-2.6/include/net/ip_vs.h	2008-09-08 11:16:46.000000000 +1000
@@ -933,11 +933,13 @@ extern void ip_vs_nat_icmp(struct sk_buf
 extern void ip_vs_nat_icmp_v6(struct sk_buff *skb, struct ip_vs_protocol *pp,
 			      struct ip_vs_conn *cp, int dir);
 
-#if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
-extern void inet_proto_csum_replace16(__sum16 *sum, struct sk_buff *skb,
-				      __be32 from[4], __be32 to[4],
-				      int pseudohdr);
 #endif
+
+#if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
+extern void ip_vs_fast_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+				   union nf_inet_addr *oldip,
+				   union nf_inet_addr *newip,
+				   __be16 old2, __be16 new2, int pseudo2);
 #endif
 
 extern __sum16 ip_vs_checksum_complete(struct sk_buff *skb, int offset);
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto.c	2008-09-08 10:56:29.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto.c	2008-09-08 11:16:46.000000000 +1000
@@ -288,9 +288,9 @@ void ip_vs_protocol_cleanup(void)
 }
 
 
-#if defined(CONFIG_IP_VS_IPV6) &&					\
-	(defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP))
-void
+#if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
+#ifdef CONFIG_IP_VS_IPV6
+static void
 inet_proto_csum_replace16(__sum16 *sum, struct sk_buff *skb,
 			  __be32 from[4], __be32 to[4], int pseudohdr)
 {
@@ -309,3 +309,20 @@ inet_proto_csum_replace16(__sum16 *sum, 
 }
 #endif
 
+void
+ip_vs_fast_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
+		       union nf_inet_addr *oldip, union nf_inet_addr *newip,
+		       __be16 old2, __be16 new2, int pseudo2)
+{
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6)
+		inet_proto_csum_replace16(sum, skb, oldip->ip6, newip->ip6, 1);
+	else
+#endif
+		inet_proto_csum_replace4(sum, skb, oldip->ip, newip->ip, 1);
+	if (new2 != old2)
+		inet_proto_csum_replace2(sum, skb, old2, new2, pseudo2);
+	if (skb->ip_summed == CHECKSUM_COMPLETE)
+		skb->ip_summed = CHECKSUM_NONE;
+}
+#endif
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 11:06:15.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_udp.c	2008-09-08 11:16:46.000000000 +1000
@@ -124,14 +124,8 @@ __udp_csum_update(int af, __sum16 *sum, 
 		       union nf_inet_addr *oldip, union nf_inet_addr *newip,
 		       __be16 old2, __be16 new2, int pseudo2)
 {
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6)
-		inet_proto_csum_replace16(sum, skb, oldip->ip6, newip->ip6, 1);
-	else
-#endif
-		inet_proto_csum_replace4(sum, skb, oldip->ip, newip->ip, 1);
-	if (old2 != new2)
-		inet_proto_csum_replace2(sum, skb, old2, new2, pseudo2);
+	ip_vs_fast_csum_update(af, sum, skb, oldip, newip,
+			       old2, new2, pseudo2);
 	if (!*sum)
 		*sum = CSUM_MANGLED_0;
 }
@@ -201,8 +195,6 @@ udp_snat_handler(struct sk_buff *skb,
 		/* Only port and addr are changed, do fast csum update */
 		udp_fast_csum_update(cp->af, &udph->check, skb, &cp->daddr,
 				     &cp->vaddr, cp->dport, cp->vport);
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		/* full checksum calculation */
 		udph->check = 0;
@@ -277,8 +269,6 @@ udp_dnat_handler(struct sk_buff *skb,
 		/* Only port and addr are changed, do fast csum update */
 		udp_fast_csum_update(cp->af, &udph->check, skb, &cp->vaddr,
 				     &cp->daddr, cp->vport, cp->dport);
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		/* full checksum calculation */
 		udph->check = 0;
Index: lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 11:03:30.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-08 11:16:46.000000000 +1000
@@ -114,27 +114,12 @@ tcp_conn_schedule(int af, struct sk_buff
 
 
 static inline void
-__tcp_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
-		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
-		     __be16 old2, __be16 new2, int pseudo2)
-{
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6)
-		inet_proto_csum_replace16(sum, skb, oldip->ip6, newip->ip6, 1);
-	else
-#endif
-		inet_proto_csum_replace4(sum, skb, oldip->ip, newip->ip, 1);
-	if (old2 != new2)
-		inet_proto_csum_replace2(sum, skb, old2, new2, pseudo2);
-}
-
-
-static inline void
 tcp_fast_csum_update(int af, __sum16 *sum, struct sk_buff *skb,
 		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
 		     __be16 oldport, __be16 newport)
 {
-	__tcp_csum_update(af, sum, skb, oldip, newip, oldport, newport, 0);
+	ip_vs_fast_csum_update(af, sum, skb, oldip, newip,
+			       oldport, newport, 0);
 }
 
 
@@ -143,7 +128,7 @@ tcp_partial_csum_update(int af, __sum16 
 		     union nf_inet_addr *oldip, union nf_inet_addr *newip,
 		     __be16 oldlen, __be16 newlen)
 {
-	__tcp_csum_update(af, sum, skb, oldip, newip, oldlen, newlen, 1);
+	ip_vs_fast_csum_update(af, sum, skb, oldip, newip, oldlen, newlen, 1);
 }
 
 
@@ -189,8 +174,6 @@ tcp_snat_handler(struct sk_buff *skb,
 		/* Only port and addr are changed, do fast csum update */
 		tcp_fast_csum_update(cp->af, &tcph->check, skb, &cp->daddr,
 				     &cp->vaddr, cp->dport, cp->vport);
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		/* full checksum calculation */
 		tcph->check = 0;
@@ -264,8 +247,6 @@ tcp_dnat_handler(struct sk_buff *skb,
 		/* Only port and addr are changed, do fast csum update */
 		tcp_fast_csum_update(cp->af, &tcph->check, skb, &cp->vaddr,
 				     &cp->daddr, cp->vport, cp->dport);
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		/* full checksum calculation */
 		tcph->check = 0;

-- 

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

* Re: [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM
  2008-09-08  2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
@ 2008-09-08  7:24   ` Herbert Xu
  2008-09-08  9:05     ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Herbert Xu @ 2008-09-08  7:24 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Julius Volz, Vince Busam

On Mon, Sep 08, 2008 at 12:04:21PM +1000, Simon Horman wrote:
>
>  	/* Adjust TCP checksums */
> -	if (!cp->app && (tcph->check != 0)) {
> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
> +					htonl(oldlen),
> +					htonl(skb->len - tcphoff));

I don't know what cp->app is but should we be updating the checksum
when it's set? The previous code seems to want to compute a full
checksum instead.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM
  2008-09-08  7:24   ` Herbert Xu
@ 2008-09-08  9:05     ` Simon Horman
  2008-09-08  9:54       ` Herbert Xu
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08  9:05 UTC (permalink / raw)
  To: Herbert Xu
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Julius Volz, Vince Busam

On Mon, Sep 08, 2008 at 05:24:40PM +1000, Herbert Xu wrote:
> On Mon, Sep 08, 2008 at 12:04:21PM +1000, Simon Horman wrote:
> >
> >  	/* Adjust TCP checksums */
> > -	if (!cp->app && (tcph->check != 0)) {
> > +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > +		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
> > +					htonl(oldlen),
> > +					htonl(skb->len - tcphoff));
> 
> I don't know what cp->app is but should we be updating the checksum
> when it's set? The previous code seems to want to compute a full
> checksum instead.

Hi Herbert,

If cp->app is not present, then only the destiantion IP address
and possibly port will have been altered. If it is present,
then other parts of the packet may also have been altered.

In the case where (skb->ip_summed == CHECKSUM_PARTIAL)
as we are only concerned with checkumming the pseudo header,
the only changes that nat could make that we care about are the
address or the length. The latter may change if cp->app is set,
but I think that my code handles this in tcp_partial_csum_update().

So I think that is safe to use tcp_partial_csum_update() if
(skb->ip_summed == CHECKSUM_PARTIAL), regarless of if cp->app is set or not.

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

* Re: [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM
  2008-09-08  9:05     ` Simon Horman
@ 2008-09-08  9:54       ` Herbert Xu
  0 siblings, 0 replies; 28+ messages in thread
From: Herbert Xu @ 2008-09-08  9:54 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Julius Volz, Vince Busam

On Mon, Sep 08, 2008 at 07:05:27PM +1000, Simon Horman wrote:
> 
> If cp->app is not present, then only the destiantion IP address
> and possibly port will have been altered. If it is present,
> then other parts of the packet may also have been altered.

Right, when cp->app is present, and we're using partial checksums,
you don't need to update anything apart from the IP address because
the hardware will compute the checksum later.

Thanks for the explanation!

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
                   ` (2 preceding siblings ...)
  2008-09-08  2:04 ` [rfc 3/3] ipvs: Consolidate checksuming code Simon Horman
@ 2008-09-08 10:03 ` Julius Volz
  2008-09-08 10:41   ` Simon Horman
  2008-09-08 23:40 ` Simon Horman
  4 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 10:03 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> Hi,
>
> The impetus for this series of patches is Julian Anastasov noting
> that "load balance IPv4 connections from a local process" checks
> for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> even on loopback traffic, but that rather partial checksums are
> possible.
>
> The first patch in this series is a proposed solution to handle
> partial checksums for both TCP and UDP.
>
> The other two patches clean things up a bit.
>
> I have not tested this code beyond compilation yet.

After some first tests, remote connections are still working, but not
local ones from the director. The TCP handshake works and the
connection is established, but all following packets arriving at the
real server have an incorrect TCP checksum.

Btw., this happens both with and without this last series of patches,
so I can't get the local client feature working at all. Looking at it
further...

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 10:03 ` [rfc 0/3] IPVS: checksum updates Julius Volz
@ 2008-09-08 10:41   ` Simon Horman
  2008-09-08 11:42     ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 10:41 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
> On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> > Hi,
> >
> > The impetus for this series of patches is Julian Anastasov noting
> > that "load balance IPv4 connections from a local process" checks
> > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> > even on loopback traffic, but that rather partial checksums are
> > possible.
> >
> > The first patch in this series is a proposed solution to handle
> > partial checksums for both TCP and UDP.
> >
> > The other two patches clean things up a bit.
> >
> > I have not tested this code beyond compilation yet.
> 
> After some first tests, remote connections are still working, but not
> local ones from the director. The TCP handshake works and the
> connection is established, but all following packets arriving at the
> real server have an incorrect TCP checksum.
> 
> Btw., this happens both with and without this last series of patches,
> so I can't get the local client feature working at all. Looking at it
> further...

Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
patch in this series applied?

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 10:41   ` Simon Horman
@ 2008-09-08 11:42     ` Julius Volz
  2008-09-08 11:57       ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 11:42 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 08:41:22PM +1000, Simon Horman wrote:
> On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
> > On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> > > Hi,
> > >
> > > The impetus for this series of patches is Julian Anastasov noting
> > > that "load balance IPv4 connections from a local process" checks
> > > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> > > even on loopback traffic, but that rather partial checksums are
> > > possible.
> > >
> > > The first patch in this series is a proposed solution to handle
> > > partial checksums for both TCP and UDP.
> > >
> > > The other two patches clean things up a bit.
> > >
> > > I have not tested this code beyond compilation yet.
> > 
> > After some first tests, remote connections are still working, but not
> > local ones from the director. The TCP handshake works and the
> > connection is established, but all following packets arriving at the
> > real server have an incorrect TCP checksum.
> > 
> > Btw., this happens both with and without this last series of patches,
> > so I can't get the local client feature working at all. Looking at it
> > further...
> 
> Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
> patch in this series applied?

It's for both, although I only tested IPv4 at first. Here is a complete
test matrix of what works when:

CR = connection refused
T = connection timeout
C = connection established, but not working afterwards
OK = working

			remote client |	local client
COMMIT			v4	v6    |	v4	v6
======================================|=================
CSUM 3/3		OK	T     |	C	T
CSUM 2/3		OK	T     |	C	T
CSUM 1/3		OK	T     |	OK	T
W/O CSUM		OK	T     |	C	T
...				      |
f2428ed5		OK	T     |	CR	CR
4856c84c		OK	CR    |	CR	CR
f94fd041 (my last one)	OK	OK    |	CR	CR

So the last time that IPv6 was working _at all_ was at my last commit of
the big v6 series...

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 11:42     ` Julius Volz
@ 2008-09-08 11:57       ` Simon Horman
  2008-09-08 12:04         ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 11:57 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 01:42:59PM +0200, Julius Volz wrote:
> On Mon, Sep 08, 2008 at 08:41:22PM +1000, Simon Horman wrote:
> > On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
> > > On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> > > > Hi,
> > > >
> > > > The impetus for this series of patches is Julian Anastasov noting
> > > > that "load balance IPv4 connections from a local process" checks
> > > > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> > > > even on loopback traffic, but that rather partial checksums are
> > > > possible.
> > > >
> > > > The first patch in this series is a proposed solution to handle
> > > > partial checksums for both TCP and UDP.
> > > >
> > > > The other two patches clean things up a bit.
> > > >
> > > > I have not tested this code beyond compilation yet.
> > > 
> > > After some first tests, remote connections are still working, but not
> > > local ones from the director. The TCP handshake works and the
> > > connection is established, but all following packets arriving at the
> > > real server have an incorrect TCP checksum.
> > > 
> > > Btw., this happens both with and without this last series of patches,
> > > so I can't get the local client feature working at all. Looking at it
> > > further...
> > 
> > Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
> > patch in this series applied?
> 
> It's for both, although I only tested IPv4 at first. Here is a complete
> test matrix of what works when:
> 
> CR = connection refused
> T = connection timeout
> C = connection established, but not working afterwards
> OK = working
> 
> 			remote client |	local client
> COMMIT			v4	v6    |	v4	v6
> ======================================|=================
> CSUM 3/3		OK	T     |	C	T
> CSUM 2/3		OK	T     |	C	T
> CSUM 1/3		OK	T     |	OK	T
> W/O CSUM		OK	T     |	C	T
> ...				      |
> f2428ed5		OK	T     |	CR	CR
> 4856c84c		OK	CR    |	CR	CR
> f94fd041 (my last one)	OK	OK    |	CR	CR
> 
> So the last time that IPv6 was working _at all_ was at my last commit of
> the big v6 series...

Ok, I'm really sorry about that :-(

Do you want me to revert f2428ed5 & 4856c84c until this has been tracked down?


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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 11:57       ` Simon Horman
@ 2008-09-08 12:04         ` Simon Horman
  2008-09-08 12:14           ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 12:04 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 09:57:35PM +1000, Simon Horman wrote:
> On Mon, Sep 08, 2008 at 01:42:59PM +0200, Julius Volz wrote:
> > On Mon, Sep 08, 2008 at 08:41:22PM +1000, Simon Horman wrote:
> > > On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
> > > > On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> > > > > Hi,
> > > > >
> > > > > The impetus for this series of patches is Julian Anastasov noting
> > > > > that "load balance IPv4 connections from a local process" checks
> > > > > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> > > > > even on loopback traffic, but that rather partial checksums are
> > > > > possible.
> > > > >
> > > > > The first patch in this series is a proposed solution to handle
> > > > > partial checksums for both TCP and UDP.
> > > > >
> > > > > The other two patches clean things up a bit.
> > > > >
> > > > > I have not tested this code beyond compilation yet.
> > > > 
> > > > After some first tests, remote connections are still working, but not
> > > > local ones from the director. The TCP handshake works and the
> > > > connection is established, but all following packets arriving at the
> > > > real server have an incorrect TCP checksum.
> > > > 
> > > > Btw., this happens both with and without this last series of patches,
> > > > so I can't get the local client feature working at all. Looking at it
> > > > further...
> > > 
> > > Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
> > > patch in this series applied?
> > 
> > It's for both, although I only tested IPv4 at first. Here is a complete
> > test matrix of what works when:
> > 
> > CR = connection refused
> > T = connection timeout
> > C = connection established, but not working afterwards
> > OK = working
> > 
> > 			remote client |	local client
> > COMMIT			v4	v6    |	v4	v6
> > ======================================|=================
> > CSUM 3/3		OK	T     |	C	T
> > CSUM 2/3		OK	T     |	C	T
> > CSUM 1/3		OK	T     |	OK	T
> > W/O CSUM		OK	T     |	C	T
> > ...				      |
> > f2428ed5		OK	T     |	CR	CR
> > 4856c84c		OK	CR    |	CR	CR
> > f94fd041 (my last one)	OK	OK    |	CR	CR
> > 
> > So the last time that IPv6 was working _at all_ was at my last commit of
> > the big v6 series...
> 
> Ok, I'm really sorry about that :-(
> 
> Do you want me to revert f2428ed5 & 4856c84c until this has been tracked down?
> 

Hi,

Does 4856c84c + the following change (which you pointed out over the
weekend) work for remote IPv6 ?

diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
index 26e3d99..c413444 100644
--- a/net/ipv4/ipvs/ip_vs_core.c
+++ b/net/ipv4/ipvs/ip_vs_core.c
@@ -1282,7 +1282,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb,
 	 *	Don't handle local packets on IPv6 for now
 	 */
 	if (unlikely(skb->pkt_type != PACKET_HOST ||
-		     (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
+		     (af == AF_INET6 && (skb->dev->flags & IFF_LOOPBACK ||
 					 skb->sk)))) {
 		IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s ignored\n",
 			      skb->pkt_type,

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 12:04         ` Simon Horman
@ 2008-09-08 12:14           ` Julius Volz
  2008-09-08 12:34             ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 12:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 8, 2008 at 2:04 PM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Sep 08, 2008 at 09:57:35PM +1000, Simon Horman wrote:
>> On Mon, Sep 08, 2008 at 01:42:59PM +0200, Julius Volz wrote:
>> > On Mon, Sep 08, 2008 at 08:41:22PM +1000, Simon Horman wrote:
>> > > On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
>> > > > On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
>> > > > > Hi,
>> > > > >
>> > > > > The impetus for this series of patches is Julian Anastasov noting
>> > > > > that "load balance IPv4 connections from a local process" checks
>> > > > > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
>> > > > > even on loopback traffic, but that rather partial checksums are
>> > > > > possible.
>> > > > >
>> > > > > The first patch in this series is a proposed solution to handle
>> > > > > partial checksums for both TCP and UDP.
>> > > > >
>> > > > > The other two patches clean things up a bit.
>> > > > >
>> > > > > I have not tested this code beyond compilation yet.
>> > > >
>> > > > After some first tests, remote connections are still working, but not
>> > > > local ones from the director. The TCP handshake works and the
>> > > > connection is established, but all following packets arriving at the
>> > > > real server have an incorrect TCP checksum.
>> > > >
>> > > > Btw., this happens both with and without this last series of patches,
>> > > > so I can't get the local client feature working at all. Looking at it
>> > > > further...
>> > >
>> > > Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
>> > > patch in this series applied?
>> >
>> > It's for both, although I only tested IPv4 at first. Here is a complete
>> > test matrix of what works when:
>> >
>> > CR = connection refused
>> > T = connection timeout
>> > C = connection established, but not working afterwards
>> > OK = working
>> >
>> >                     remote client | local client
>> > COMMIT                      v4      v6    | v4      v6
>> > ======================================|=================
>> > CSUM 3/3            OK      T     | C       T
>> > CSUM 2/3            OK      T     | C       T
>> > CSUM 1/3            OK      T     | OK      T
>> > W/O CSUM            OK      T     | C       T
>> > ...                               |
>> > f2428ed5            OK      T     | CR      CR
>> > 4856c84c            OK      CR    | CR      CR
>> > f94fd041 (my last one)      OK      OK    | CR      CR
>> >
>> > So the last time that IPv6 was working _at all_ was at my last commit of
>> > the big v6 series...
>>
>> Ok, I'm really sorry about that :-(
>>
>> Do you want me to revert f2428ed5 & 4856c84c until this has been tracked down?
>>
>
> Hi,
>
> Does 4856c84c + the following change (which you pointed out over the
> weekend) work for remote IPv6 ?
>
> diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
> index 26e3d99..c413444 100644
> --- a/net/ipv4/ipvs/ip_vs_core.c
> +++ b/net/ipv4/ipvs/ip_vs_core.c
> @@ -1282,7 +1282,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb,
>         *      Don't handle local packets on IPv6 for now
>         */
>        if (unlikely(skb->pkt_type != PACKET_HOST ||
> -                    (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
> +                    (af == AF_INET6 && (skb->dev->flags & IFF_LOOPBACK ||
>                                         skb->sk)))) {
>                IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s ignored\n",
>                              skb->pkt_type,
>

No, that just changes the behavior from "connection refused" to timeout...

I'm actually looking at that case now (4856c84c1358b, but with the fix
above). It seems that the NAT isn't working (DR works, by the way!).
At least the first packet arriving at the real server still has the
client's IP as the source (in the v6 case)...

Let's wait with reverting the local client patches until tomorrow...
maybe I can find the problem until then.

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 12:14           ` Julius Volz
@ 2008-09-08 12:34             ` Simon Horman
  2008-09-08 13:12               ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 12:34 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 02:14:12PM +0200, Julius Volz wrote:
> On Mon, Sep 8, 2008 at 2:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Sep 08, 2008 at 09:57:35PM +1000, Simon Horman wrote:
> >> On Mon, Sep 08, 2008 at 01:42:59PM +0200, Julius Volz wrote:
> >> > On Mon, Sep 08, 2008 at 08:41:22PM +1000, Simon Horman wrote:
> >> > > On Mon, Sep 08, 2008 at 12:03:04PM +0200, Julius Volz wrote:
> >> > > > On Mon, Sep 8, 2008 at 4:04 AM, Simon Horman wrote:
> >> > > > > Hi,
> >> > > > >
> >> > > > > The impetus for this series of patches is Julian Anastasov noting
> >> > > > > that "load balance IPv4 connections from a local process" checks
> >> > > > > for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> >> > > > > even on loopback traffic, but that rather partial checksums are
> >> > > > > possible.
> >> > > > >
> >> > > > > The first patch in this series is a proposed solution to handle
> >> > > > > partial checksums for both TCP and UDP.
> >> > > > >
> >> > > > > The other two patches clean things up a bit.
> >> > > > >
> >> > > > > I have not tested this code beyond compilation yet.
> >> > > >
> >> > > > After some first tests, remote connections are still working, but not
> >> > > > local ones from the director. The TCP handshake works and the
> >> > > > connection is established, but all following packets arriving at the
> >> > > > real server have an incorrect TCP checksum.
> >> > > >
> >> > > > Btw., this happens both with and without this last series of patches,
> >> > > > so I can't get the local client feature working at all. Looking at it
> >> > > > further...
> >> > >
> >> > > Ok, is this for both IPv4 & IPv6? Does it still occur with just the first
> >> > > patch in this series applied?
> >> >
> >> > It's for both, although I only tested IPv4 at first. Here is a complete
> >> > test matrix of what works when:
> >> >
> >> > CR = connection refused
> >> > T = connection timeout
> >> > C = connection established, but not working afterwards
> >> > OK = working
> >> >
> >> >                     remote client | local client
> >> > COMMIT                      v4      v6    | v4      v6
> >> > ======================================|=================
> >> > CSUM 3/3            OK      T     | C       T
> >> > CSUM 2/3            OK      T     | C       T
> >> > CSUM 1/3            OK      T     | OK      T
> >> > W/O CSUM            OK      T     | C       T
> >> > ...                               |
> >> > f2428ed5            OK      T     | CR      CR
> >> > 4856c84c            OK      CR    | CR      CR
> >> > f94fd041 (my last one)      OK      OK    | CR      CR
> >> >
> >> > So the last time that IPv6 was working _at all_ was at my last commit of
> >> > the big v6 series...
> >>
> >> Ok, I'm really sorry about that :-(
> >>
> >> Do you want me to revert f2428ed5 & 4856c84c until this has been tracked down?
> >>
> >
> > Hi,
> >
> > Does 4856c84c + the following change (which you pointed out over the
> > weekend) work for remote IPv6 ?
> >
> > diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
> > index 26e3d99..c413444 100644
> > --- a/net/ipv4/ipvs/ip_vs_core.c
> > +++ b/net/ipv4/ipvs/ip_vs_core.c
> > @@ -1282,7 +1282,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb,
> >         *      Don't handle local packets on IPv6 for now
> >         */
> >        if (unlikely(skb->pkt_type != PACKET_HOST ||
> > -                    (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
> > +                    (af == AF_INET6 && (skb->dev->flags & IFF_LOOPBACK ||
> >                                         skb->sk)))) {
> >                IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s ignored\n",
> >                              skb->pkt_type,
> >
> 
> No, that just changes the behavior from "connection refused" to timeout...
> 
> I'm actually looking at that case now (4856c84c1358b, but with the fix
> above). It seems that the NAT isn't working (DR works, by the way!).
> At least the first packet arriving at the real server still has the
> client's IP as the source (in the v6 case)...

Ok, I'm looking at NAT with 4856c84c1358b + that fix too too,
but on v4 :-)

> Let's wait with reverting the local client patches until tomorrow...
> maybe I can find the problem until then.

Ok, I was just concerned that this might hold up merging
your code into Dave's tree for too long, thats all.

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 12:34             ` Simon Horman
@ 2008-09-08 13:12               ` Julius Volz
  2008-09-08 13:20                 ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 13:12 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 8, 2008 at 2:34 PM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Sep 08, 2008 at 02:14:12PM +0200, Julius Volz wrote:
>> I'm actually looking at that case now (4856c84c1358b, but with the fix
>> above). It seems that the NAT isn't working (DR works, by the way!).
>> At least the first packet arriving at the real server still has the
>> client's IP as the source (in the v6 case)...
>
> Ok, I'm looking at NAT with 4856c84c1358b + that fix too too,
> but on v4 :-)
>
>> Let's wait with reverting the local client patches until tomorrow...
>> maybe I can find the problem until then.
>
> Ok, I was just concerned that this might hold up merging
> your code into Dave's tree for too long, thats all.

Ok, with my last patch (and your two/three last ones reverted), I get
everything working except IPv6 with a local client...

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 13:12               ` Julius Volz
@ 2008-09-08 13:20                 ` Simon Horman
  2008-09-08 13:42                   ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 13:20 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 03:12:39PM +0200, Julius Volz wrote:
> On Mon, Sep 8, 2008 at 2:34 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Sep 08, 2008 at 02:14:12PM +0200, Julius Volz wrote:
> >> I'm actually looking at that case now (4856c84c1358b, but with the fix
> >> above). It seems that the NAT isn't working (DR works, by the way!).
> >> At least the first packet arriving at the real server still has the
> >> client's IP as the source (in the v6 case)...
> >
> > Ok, I'm looking at NAT with 4856c84c1358b + that fix too too,
> > but on v4 :-)
> >
> >> Let's wait with reverting the local client patches until tomorrow...
> >> maybe I can find the problem until then.
> >
> > Ok, I was just concerned that this might hold up merging
> > your code into Dave's tree for too long, thats all.
> 
> Ok, with my last patch (and your two/three last ones reverted), I get
> everything working except IPv6 with a local client...

Ok, that is good news :-)

Sorry for the stuff up. That bugus ip_route_me_harder() call
was caused by me trying to merge the IPv6 patches with the local
client patches.

I'm not so concerned about IPv6 local not working for now -
its a combination of two new features. And hopefully we
can iron it out in the not to distant future anyway.

I'll drop CSUM2/3 and 3/3 for now, they aren't essential and
it really ought to be a matter of (me) sitting down and fixing
some correctness issues.


Its the end of my day now. I'll try and update lvs-next-2.6 into
a working state in the morning.

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 13:20                 ` Simon Horman
@ 2008-09-08 13:42                   ` Julius Volz
  2008-09-08 15:32                     ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 13:42 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 8, 2008 at 3:20 PM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Sep 08, 2008 at 03:12:39PM +0200, Julius Volz wrote:
>> On Mon, Sep 8, 2008 at 2:34 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Mon, Sep 08, 2008 at 02:14:12PM +0200, Julius Volz wrote:
>> >> I'm actually looking at that case now (4856c84c1358b, but with the fix
>> >> above). It seems that the NAT isn't working (DR works, by the way!).
>> >> At least the first packet arriving at the real server still has the
>> >> client's IP as the source (in the v6 case)...
>> >
>> > Ok, I'm looking at NAT with 4856c84c1358b + that fix too too,
>> > but on v4 :-)
>> >
>> >> Let's wait with reverting the local client patches until tomorrow...
>> >> maybe I can find the problem until then.
>> >
>> > Ok, I was just concerned that this might hold up merging
>> > your code into Dave's tree for too long, thats all.
>>
>> Ok, with my last patch (and your two/three last ones reverted), I get
>> everything working except IPv6 with a local client...
>
> Ok, that is good news :-)
>
> Sorry for the stuff up. That bugus ip_route_me_harder() call
> was caused by me trying to merge the IPv6 patches with the local
> client patches.

No problem!

> I'm not so concerned about IPv6 local not working for now -
> its a combination of two new features. And hopefully we
> can iron it out in the not to distant future anyway.

Yes, probably just some minor bug...

> I'll drop CSUM2/3 and 3/3 for now, they aren't essential and
> it really ought to be a matter of (me) sitting down and fixing
> some correctness issues.

Ok!

> Its the end of my day now. I'll try and update lvs-next-2.6 into
> a working state in the morning.

Yes, let's stabilize it a bit... have a nice evening!

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 13:42                   ` Julius Volz
@ 2008-09-08 15:32                     ` Julius Volz
  2008-09-08 23:22                       ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-08 15:32 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 8, 2008 at 3:42 PM, Julius Volz <juliusv@google.com> wrote:
> On Mon, Sep 8, 2008 at 3:20 PM, Simon Horman <horms@verge.net.au> wrote:
>> I'm not so concerned about IPv6 local not working for now -
>> its a combination of two new features. And hopefully we
>> can iron it out in the not to distant future anyway.
>
> Yes, probably just some minor bug...

Actually, it wasn't even a kernel bug, but a configuration error - I
was using LVS/NAT, but still had the dummy0 interface with the IPv6
VIP configured on the real server (from the DR case). And in the local
client case, the source address is the VIP, so the real server just
delivered the response to itself.

Now I can confirm that everything is working (without your last two
patches and with the one removing the wrong ip_route_me_harder)!

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 15:32                     ` Julius Volz
@ 2008-09-08 23:22                       ` Simon Horman
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Horman @ 2008-09-08 23:22 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 05:32:47PM +0200, Julius Volz wrote:
> On Mon, Sep 8, 2008 at 3:42 PM, Julius Volz <juliusv@google.com> wrote:
> > On Mon, Sep 8, 2008 at 3:20 PM, Simon Horman <horms@verge.net.au> wrote:
> >> I'm not so concerned about IPv6 local not working for now -
> >> its a combination of two new features. And hopefully we
> >> can iron it out in the not to distant future anyway.
> >
> > Yes, probably just some minor bug...
> 
> Actually, it wasn't even a kernel bug, but a configuration error - I
> was using LVS/NAT, but still had the dummy0 interface with the IPv6
> VIP configured on the real server (from the DR case). And in the local
> client case, the source address is the VIP, so the real server just
> delivered the response to itself.
> 
> Now I can confirm that everything is working (without your last two
> patches and with the one removing the wrong ip_route_me_harder)!

Great!

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
                   ` (3 preceding siblings ...)
  2008-09-08 10:03 ` [rfc 0/3] IPVS: checksum updates Julius Volz
@ 2008-09-08 23:40 ` Simon Horman
  2008-09-09  9:30   ` Julius Volz
  4 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-08 23:40 UTC (permalink / raw)
  To: lvs-devel, netdev
  Cc: Siim Põder, Julian Anastasov, Malcolm Turnbull, Julius Volz,
	Vince Busam, Herbert Xu

On Mon, Sep 08, 2008 at 12:04:20PM +1000, Simon Horman wrote:
> Hi,
> 
> The impetus for this series of patches is Julian Anastasov noting
> that "load balance IPv4 connections from a local process" checks
> for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> even on loopback traffic, but that rather partial checksums are
> possible.
> 
> The first patch in this series is a proposed solution to handle
> partial checksums for both TCP and UDP.
> 
> The other two patches clean things up a bit.
> 
> I have not tested this code beyond compilation yet.

After extensive testing by Julius Volz and limited testing by myself, I
have applied the first patch, which does indeed allow packets with
PARTIAL_CHECKSUM to work, to lvs-next-2.6. I have dropped the second two
patches which produce bogus checksums.

I will revisit the second to patches at some point and try and produce
versions that work.


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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-08 23:40 ` Simon Horman
@ 2008-09-09  9:30   ` Julius Volz
  2008-09-09 11:31     ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-09  9:30 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Tue, Sep 9, 2008 at 1:40 AM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Sep 08, 2008 at 12:04:20PM +1000, Simon Horman wrote:
>> Hi,
>>
>> The impetus for this series of patches is Julian Anastasov noting
>> that "load balance IPv4 connections from a local process" checks
>> for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
>> even on loopback traffic, but that rather partial checksums are
>> possible.
>>
>> The first patch in this series is a proposed solution to handle
>> partial checksums for both TCP and UDP.
>>
>> The other two patches clean things up a bit.
>>
>> I have not tested this code beyond compilation yet.
>
> After extensive testing by Julius Volz and limited testing by myself, I
> have applied the first patch, which does indeed allow packets with
> PARTIAL_CHECKSUM to work, to lvs-next-2.6. I have dropped the second two
> patches which produce bogus checksums.

Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
NAT+DSR+TUN in all combinations that are expected to be working and
found no problems.

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-09  9:30   ` Julius Volz
@ 2008-09-09 11:31     ` Simon Horman
  2008-09-10 17:30       ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-09 11:31 UTC (permalink / raw)
  To: Julius Volz
  Cc: lvs-devel, netdev, Siim Põder, Julian Anastasov,
	Malcolm Turnbull, Vince Busam, Herbert Xu

On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
> On Tue, Sep 9, 2008 at 1:40 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Sep 08, 2008 at 12:04:20PM +1000, Simon Horman wrote:
> >> Hi,
> >>
> >> The impetus for this series of patches is Julian Anastasov noting
> >> that "load balance IPv4 connections from a local process" checks
> >> for 0 TCP checksums. Herbert Xu confirmed that this is not legal,
> >> even on loopback traffic, but that rather partial checksums are
> >> possible.
> >>
> >> The first patch in this series is a proposed solution to handle
> >> partial checksums for both TCP and UDP.
> >>
> >> The other two patches clean things up a bit.
> >>
> >> I have not tested this code beyond compilation yet.
> >
> > After extensive testing by Julius Volz and limited testing by myself, I
> > have applied the first patch, which does indeed allow packets with
> > PARTIAL_CHECKSUM to work, to lvs-next-2.6. I have dropped the second two
> > patches which produce bogus checksums.
> 
> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
> NAT+DSR+TUN in all combinations that are expected to be working and
> found no problems.

Thanks once again for your testing. I'll send a pull request to Dave in
the morning.


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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-09 11:31     ` Simon Horman
@ 2008-09-10 17:30       ` Julius Volz
  2008-09-10 23:29         ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-10 17:30 UTC (permalink / raw)
  To: Simon Horman; +Cc: lvs-devel, netdev, Vince Busam, wensong

On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
> On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
>> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
>> NAT+DSR+TUN in all combinations that are expected to be working and
>> found no problems.
>
> Thanks once again for your testing. I'll send a pull request to Dave in
> the morning.

Thanks very much!

(CC: Wensong)
I guess it would make sense to start hosting the new ipvsadm version
somewhere on http://www.linuxvirtualserver.org/ soon. It could still
be marked as unstable and saying, "If you intend to use the
experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
will need this new version of ipvsadm: ...". Also, we can then point
to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
think?

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-10 17:30       ` Julius Volz
@ 2008-09-10 23:29         ` Simon Horman
  2008-09-11 13:07           ` Wensong Zhang
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-10 23:29 UTC (permalink / raw)
  To: Julius Volz; +Cc: lvs-devel, netdev, Vince Busam, wensong

On Wed, Sep 10, 2008 at 07:30:52PM +0200, Julius Volz wrote:
> On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
> >> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
> >> NAT+DSR+TUN in all combinations that are expected to be working and
> >> found no problems.
> >
> > Thanks once again for your testing. I'll send a pull request to Dave in
> > the morning.
> 
> Thanks very much!
> 
> (CC: Wensong)
> I guess it would make sense to start hosting the new ipvsadm version
> somewhere on http://www.linuxvirtualserver.org/ soon. It could still
> be marked as unstable and saying, "If you intend to use the
> experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
> will need this new version of ipvsadm: ...". Also, we can then point
> to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
> think?

Yes I agree. It would be good to start hosting tarballs of the
new/unstable/experimental version of ipvsadm on www.linuxvirtualserver.org.

Wensong, can we also host some sort of repository for ipvsadm on
linuxvirtualserver.org? If not, I guess the next most logical choice
would be to use kernel.org.


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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-10 23:29         ` Simon Horman
@ 2008-09-11 13:07           ` Wensong Zhang
  2008-09-11 13:45             ` Simon Horman
  0 siblings, 1 reply; 28+ messages in thread
From: Wensong Zhang @ 2008-09-11 13:07 UTC (permalink / raw)
  To: Simon Horman; +Cc: Julius Volz, lvs-devel, netdev, Vince Busam

Simon Horman wrote:
> On Wed, Sep 10, 2008 at 07:30:52PM +0200, Julius Volz wrote:
>   
>> On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
>>     
>>> On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
>>>       
>>>> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
>>>> NAT+DSR+TUN in all combinations that are expected to be working and
>>>> found no problems.
>>>>         
>>> Thanks once again for your testing. I'll send a pull request to Dave in
>>> the morning.
>>>       
>> Thanks very much!
>>
>> (CC: Wensong)
>> I guess it would make sense to start hosting the new ipvsadm version
>> somewhere on http://www.linuxvirtualserver.org/ soon. It could still
>> be marked as unstable and saying, "If you intend to use the
>> experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
>> will need this new version of ipvsadm: ...". Also, we can then point
>> to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
>> think?
>>     
>
> Yes I agree. It would be good to start hosting tarballs of the
> new/unstable/experimental version of ipvsadm on www.linuxvirtualserver.org.
>
> Wensong, can we also host some sort of repository for ipvsadm on
> linuxvirtualserver.org? If not, I guess the next most logical choice
> would be to use kernel.org.
>
>   
Hi Horms and Julius,

I have created a svn repository for ipvsadm at 
http://svn.linuxvirtualserver.org/repos/ipvsadm/ before, will create the 
check-in accounts for you right now.

Thanks,

Wensong



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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-11 13:07           ` Wensong Zhang
@ 2008-09-11 13:45             ` Simon Horman
  2008-09-11 13:55               ` Julius Volz
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Horman @ 2008-09-11 13:45 UTC (permalink / raw)
  To: Wensong Zhang; +Cc: Julius Volz, lvs-devel, netdev, Vince Busam

On Thu, Sep 11, 2008 at 09:07:22PM +0800, Wensong Zhang wrote:
> Simon Horman wrote:
>> On Wed, Sep 10, 2008 at 07:30:52PM +0200, Julius Volz wrote:
>>   
>>> On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
>>>     
>>>> On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
>>>>       
>>>>> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
>>>>> NAT+DSR+TUN in all combinations that are expected to be working and
>>>>> found no problems.
>>>>>         
>>>> Thanks once again for your testing. I'll send a pull request to Dave in
>>>> the morning.
>>>>       
>>> Thanks very much!
>>>
>>> (CC: Wensong)
>>> I guess it would make sense to start hosting the new ipvsadm version
>>> somewhere on http://www.linuxvirtualserver.org/ soon. It could still
>>> be marked as unstable and saying, "If you intend to use the
>>> experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
>>> will need this new version of ipvsadm: ...". Also, we can then point
>>> to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
>>> think?
>>>     
>>
>> Yes I agree. It would be good to start hosting tarballs of the
>> new/unstable/experimental version of ipvsadm on www.linuxvirtualserver.org.
>>
>> Wensong, can we also host some sort of repository for ipvsadm on
>> linuxvirtualserver.org? If not, I guess the next most logical choice
>> would be to use kernel.org.
>>
>>   
> Hi Horms and Julius,
>
> I have created a svn repository for ipvsadm at  
> http://svn.linuxvirtualserver.org/repos/ipvsadm/ before, will create the  
> check-in accounts for you right now.

Great. I think that Vince needs an account too, he has done most (all?)
of the relevant work so far.

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en


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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-11 13:45             ` Simon Horman
@ 2008-09-11 13:55               ` Julius Volz
  2008-09-11 14:43                 ` Wensong Zhang
  0 siblings, 1 reply; 28+ messages in thread
From: Julius Volz @ 2008-09-11 13:55 UTC (permalink / raw)
  To: Simon Horman; +Cc: Wensong Zhang, lvs-devel, netdev, Vince Busam

On Thu, Sep 11, 2008 at 3:45 PM, Simon Horman <horms@verge.net.au> wrote:
> On Thu, Sep 11, 2008 at 09:07:22PM +0800, Wensong Zhang wrote:
>> Simon Horman wrote:
>>> On Wed, Sep 10, 2008 at 07:30:52PM +0200, Julius Volz wrote:
>>>
>>>> On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
>>>>
>>>>> On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
>>>>>
>>>>>> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
>>>>>> NAT+DSR+TUN in all combinations that are expected to be working and
>>>>>> found no problems.
>>>>>>
>>>>> Thanks once again for your testing. I'll send a pull request to Dave in
>>>>> the morning.
>>>>>
>>>> Thanks very much!
>>>>
>>>> (CC: Wensong)
>>>> I guess it would make sense to start hosting the new ipvsadm version
>>>> somewhere on http://www.linuxvirtualserver.org/ soon. It could still
>>>> be marked as unstable and saying, "If you intend to use the
>>>> experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
>>>> will need this new version of ipvsadm: ...". Also, we can then point
>>>> to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
>>>> think?
>>>>
>>>
>>> Yes I agree. It would be good to start hosting tarballs of the
>>> new/unstable/experimental version of ipvsadm on www.linuxvirtualserver.org.
>>>
>>> Wensong, can we also host some sort of repository for ipvsadm on
>>> linuxvirtualserver.org? If not, I guess the next most logical choice
>>> would be to use kernel.org.
>>>
>>>
>> Hi Horms and Julius,
>>
>> I have created a svn repository for ipvsadm at
>> http://svn.linuxvirtualserver.org/repos/ipvsadm/ before, will create the
>> check-in accounts for you right now.

Thanks, cool!

> Great. I think that Vince needs an account too, he has done most (all?)
> of the relevant work so far.

Yes, Vince did all of the ipvsadm porting!

Julius

-- 
Julius Volz - Corporate Operations - SysOps

Google Switzerland GmbH - Identification No.: CH-020.4.028.116-1

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

* Re: [rfc 0/3] IPVS: checksum updates
  2008-09-11 13:55               ` Julius Volz
@ 2008-09-11 14:43                 ` Wensong Zhang
  0 siblings, 0 replies; 28+ messages in thread
From: Wensong Zhang @ 2008-09-11 14:43 UTC (permalink / raw)
  To: Julius Volz; +Cc: Simon Horman, lvs-devel, netdev, Vince Busam

Julius Volz wrote:
> On Thu, Sep 11, 2008 at 3:45 PM, Simon Horman <horms@verge.net.au> wrote:
>   
>> On Thu, Sep 11, 2008 at 09:07:22PM +0800, Wensong Zhang wrote:
>>     
>>> Simon Horman wrote:
>>>       
>>>> On Wed, Sep 10, 2008 at 07:30:52PM +0200, Julius Volz wrote:
>>>>
>>>>         
>>>>> On Tue, Sep 9, 2008 at 1:31 PM, Simon Horman <horms@verge.net.au> wrote:
>>>>>
>>>>>           
>>>>>> On Tue, Sep 09, 2008 at 11:30:29AM +0200, Julius Volz wrote:
>>>>>>
>>>>>>             
>>>>>>> Great, thanks! I have tested TCP+UDP, local+remote clients, v4+v6,
>>>>>>> NAT+DSR+TUN in all combinations that are expected to be working and
>>>>>>> found no problems.
>>>>>>>
>>>>>>>               
>>>>>> Thanks once again for your testing. I'll send a pull request to Dave in
>>>>>> the morning.
>>>>>>
>>>>>>             
>>>>> Thanks very much!
>>>>>
>>>>> (CC: Wensong)
>>>>> I guess it would make sense to start hosting the new ipvsadm version
>>>>> somewhere on http://www.linuxvirtualserver.org/ soon. It could still
>>>>> be marked as unstable and saying, "If you intend to use the
>>>>> experimental IPv6 support for IPVS introduced in kernel 2.6.XX, you
>>>>> will need this new version of ipvsadm: ...". Also, we can then point
>>>>> to it in the Kconfig help text in CONFIG_IP_VS_IPV6. What do you
>>>>> think?
>>>>>
>>>>>           
>>>> Yes I agree. It would be good to start hosting tarballs of the
>>>> new/unstable/experimental version of ipvsadm on www.linuxvirtualserver.org.
>>>>
>>>> Wensong, can we also host some sort of repository for ipvsadm on
>>>> linuxvirtualserver.org? If not, I guess the next most logical choice
>>>> would be to use kernel.org.
>>>>
>>>>
>>>>         
>>> Hi Horms and Julius,
>>>
>>> I have created a svn repository for ipvsadm at
>>> http://svn.linuxvirtualserver.org/repos/ipvsadm/ before, will create the
>>> check-in accounts for you right now.
>>>       
>
> Thanks, cool!
>
>   
>> Great. I think that Vince needs an account too, he has done most (all?)
>> of the relevant work so far.
>>     
>
> Yes, Vince did all of the ipvsadm porting!
>
> Julius
>
>   
I'll create an account for Vince right now.

Thank you very much,

Wensong

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

end of thread, other threads:[~2008-09-11 14:43 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-08  2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
2008-09-08  2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
2008-09-08  7:24   ` Herbert Xu
2008-09-08  9:05     ` Simon Horman
2008-09-08  9:54       ` Herbert Xu
2008-09-08  2:04 ` [rfc 2/3] ipvs: Use inet_proto_csum_replace*() Simon Horman
2008-09-08  2:04 ` [rfc 3/3] ipvs: Consolidate checksuming code Simon Horman
2008-09-08 10:03 ` [rfc 0/3] IPVS: checksum updates Julius Volz
2008-09-08 10:41   ` Simon Horman
2008-09-08 11:42     ` Julius Volz
2008-09-08 11:57       ` Simon Horman
2008-09-08 12:04         ` Simon Horman
2008-09-08 12:14           ` Julius Volz
2008-09-08 12:34             ` Simon Horman
2008-09-08 13:12               ` Julius Volz
2008-09-08 13:20                 ` Simon Horman
2008-09-08 13:42                   ` Julius Volz
2008-09-08 15:32                     ` Julius Volz
2008-09-08 23:22                       ` Simon Horman
2008-09-08 23:40 ` Simon Horman
2008-09-09  9:30   ` Julius Volz
2008-09-09 11:31     ` Simon Horman
2008-09-10 17:30       ` Julius Volz
2008-09-10 23:29         ` Simon Horman
2008-09-11 13:07           ` Wensong Zhang
2008-09-11 13:45             ` Simon Horman
2008-09-11 13:55               ` Julius Volz
2008-09-11 14:43                 ` Wensong Zhang

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