[PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Simon Horman]

From: Malcolm Turnbull <malcolm@loadbalancer.org>

ipvs: load balance IPv4 connections from a local process

This allows IPVS to load balance connections made by a local process.
For example a proxy server running locally.

External client --> pound:443 -> Local:443 --> IPVS:80 --> RealServer

Signed-off-by: Siim Põder <siim@p6drad-teel.net>
Signed-off-by: Malcolm Turnbull <malcolm@loadbalancer.org>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 net/ipv4/ipvs/ip_vs_core.c      |  224 ++++++++++++++++++++++-----------------
 net/ipv4/ipvs/ip_vs_proto_tcp.c |    4 
 2 files changed, 134 insertions(+), 94 deletions(-)

* Simon Horman, Wed, 03 Sep 2008 14:50:36 +1000

  I have updated this patch so that it will apply on top
  of the current IPv6 patches.

  http://marc.info/?l=linux-netdev&m=122036407428246&w=2

  I have also updated the patch so that it does not handle IPv6 packets.

  I have an additional patch that I will provide to exetend
  the code to handle IPv6 connections.

* Simon Horman, Fri, 05 Sep 2008 11:32:38 +1000

  I have applied this patch to the net-next-2.6 branck of lvs-2.6

  git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-2.6.git

Index: lvs-2.6/net/ipv4/ipvs/ip_vs_core.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_core.c	2008-09-03 11:01:38.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_core.c	2008-09-03 12:17:49.000000000 +1000
@@ -651,12 +651,53 @@ void ip_vs_nat_icmp_v6(struct sk_buff *s
 }
 #endif
 
+/* Handle relevant response ICMP messages - forward to the right
+ * destination host. Used for NAT and local client.
+ */
+static int handle_response_icmp(struct sk_buff *skb, struct iphdr *iph,
+				struct iphdr *cih, struct ip_vs_conn *cp,
+				struct ip_vs_protocol *pp,
+				unsigned int offset, unsigned int ihl)
+{
+	unsigned int verdict = NF_DROP;
+
+	if (IP_VS_FWD_METHOD(cp) != 0) {
+		IP_VS_ERR("shouldn't reach here, because the box is on the "
+			  "half connection in the tun/dr module.\n");
+	}
+
+	/* Ensure the checksum is correct */
+	if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
+		/* Failed checksum! */
+		IP_VS_DBG(1,
+			  "Forward ICMP: failed checksum from %d.%d.%d.%d!\n",
+			  NIPQUAD(iph->saddr));
+		goto out;
+	}
+
+	if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol)
+		offset += 2 * sizeof(__u16);
+	if (!skb_make_writable(skb, offset))
+		goto out;
+
+	ip_vs_nat_icmp(skb, pp, cp, 1);
+
+	/* do the statistics and put it back */
+	ip_vs_out_stats(cp, skb);
+
+	skb->ipvs_property = 1;
+	verdict = NF_ACCEPT;
+
+out:
+	__ip_vs_conn_put(cp);
+
+	return verdict;
+}
+
 /*
  *	Handle ICMP messages in the inside-to-outside direction (outgoing).
- *	Find any that might be relevant, check against existing connections,
- *	forward to the right destination host if relevant.
+ *	Find any that might be relevant, check against existing connections.
  *	Currently handles error types - unreachable, quench, ttl exceeded.
- *	(Only used in VS/NAT)
  */
 static int ip_vs_out_icmp(struct sk_buff *skb, int *related)
 {
@@ -666,7 +707,7 @@ static int ip_vs_out_icmp(struct sk_buff
 	struct ip_vs_iphdr ciph;
 	struct ip_vs_conn *cp;
 	struct ip_vs_protocol *pp;
-	unsigned int offset, ihl, verdict;
+	unsigned int offset, ihl;
 
 	*related = 1;
 
@@ -725,38 +766,7 @@ static int ip_vs_out_icmp(struct sk_buff
 	if (!cp)
 		return NF_ACCEPT;
 
-	verdict = NF_DROP;
-
-	if (IP_VS_FWD_METHOD(cp) != 0) {
-		IP_VS_ERR("shouldn't reach here, because the box is on the "
-			  "half connection in the tun/dr module.\n");
-	}
-
-	/* Ensure the checksum is correct */
-	if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
-		/* Failed checksum! */
-		IP_VS_DBG(1, "Forward ICMP: failed checksum from %d.%d.%d.%d!\n",
-			  NIPQUAD(iph->saddr));
-		goto out;
-	}
-
-	if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol)
-		offset += 2 * sizeof(__u16);
-	if (!skb_make_writable(skb, offset))
-		goto out;
-
-	ip_vs_nat_icmp(skb, pp, cp, 1);
-
-	/* do the statistics and put it back */
-	ip_vs_out_stats(cp, skb);
-
-	skb->ipvs_property = 1;
-	verdict = NF_ACCEPT;
-
-  out:
-	__ip_vs_conn_put(cp);
-
-	return verdict;
+	return handle_response_icmp(skb, iph, cih, cp, pp, offset, ihl);
 }
 
 #ifdef CONFIG_IP_VS_IPV6
@@ -875,10 +885,76 @@ static inline int is_tcp_reset(const str
 	return th->rst;
 }
 
+/* Handle response packets: rewrite addresses and send away...
+ * Used for NAT and local client.
+ */
+static unsigned int
+handle_response(int af, struct sk_buff *skb, struct ip_vs_protocol *pp,
+		struct ip_vs_conn *cp, int ihl)
+{
+	IP_VS_DBG_PKT(11, pp, skb, 0, "Outgoing packet");
+
+	if (!skb_make_writable(skb, ihl))
+		goto drop;
+
+	/* mangle the packet */
+	if (pp->snat_handler && !pp->snat_handler(skb, pp, cp))
+		goto drop;
+
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6)
+		ipv6_hdr(skb)->saddr = cp->vaddr.in6;
+	else
+#endif
+	{
+		ip_hdr(skb)->saddr = cp->vaddr.ip;
+		ip_send_check(ip_hdr(skb));
+	}
+
+	/* For policy routing, packets originating from this
+	 * machine itself may be routed differently to packets
+	 * passing through.  We want this packet to be routed as
+	 * if it came from this machine itself.  So re-compute
+	 * the routing information.
+	 */
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6) {
+		if (ip6_route_me_harder(skb) != 0)
+			goto drop;
+	} else
+#endif
+		if (ip_route_me_harder(skb, RTN_LOCAL) != 0)
+			goto drop;
+
+	/* For policy routing, packets originating from this
+	 * machine itself may be routed differently to packets
+	 * passing through.  We want this packet to be routed as
+	 * if it came from this machine itself.  So re-compute
+	 * the routing information.
+	 */
+	if (ip_route_me_harder(skb, RTN_LOCAL) != 0)
+		goto drop;
+
+	IP_VS_DBG_PKT(10, pp, skb, 0, "After SNAT");
+
+	ip_vs_out_stats(cp, skb);
+	ip_vs_set_state(cp, IP_VS_DIR_OUTPUT, skb, pp);
+	ip_vs_conn_put(cp);
+
+	skb->ipvs_property = 1;
+
+	LeaveFunction(11);
+	return NF_ACCEPT;
+
+drop:
+	ip_vs_conn_put(cp);
+	kfree_skb(skb);
+	return NF_STOLEN;
+}
+
 /*
  *	It is hooked at the NF_INET_FORWARD chain, used only for VS/NAT.
- *	Check if outgoing packet belongs to the established ip_vs_conn,
- *      rewrite addresses of the packet and send it on its way...
+ *	Check if outgoing packet belongs to the established ip_vs_conn.
  */
 static unsigned int
 ip_vs_out(unsigned int hooknum, struct sk_buff *skb,
@@ -987,55 +1063,7 @@ ip_vs_out(unsigned int hooknum, struct s
 		return NF_ACCEPT;
 	}
 
-	IP_VS_DBG_PKT(11, pp, skb, 0, "Outgoing packet");
-
-	if (!skb_make_writable(skb, iph.len))
-		goto drop;
-
-	/* mangle the packet */
-	if (pp->snat_handler && !pp->snat_handler(skb, pp, cp))
-		goto drop;
-
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6)
-		ipv6_hdr(skb)->saddr = cp->vaddr.in6;
-	else
-#endif
-	{
-		ip_hdr(skb)->saddr = cp->vaddr.ip;
-		ip_send_check(ip_hdr(skb));
-	}
-
-	/* For policy routing, packets originating from this
-	 * machine itself may be routed differently to packets
-	 * passing through.  We want this packet to be routed as
-	 * if it came from this machine itself.  So re-compute
-	 * the routing information.
-	 */
-#ifdef CONFIG_IP_VS_IPV6
-	if (af == AF_INET6) {
-		if (ip6_route_me_harder(skb) != 0)
-			goto drop;
-	} else
-#endif
-		if (ip_route_me_harder(skb, RTN_LOCAL) != 0)
-			goto drop;
-
-	IP_VS_DBG_PKT(10, pp, skb, 0, "After SNAT");
-
-	ip_vs_out_stats(cp, skb);
-	ip_vs_set_state(cp, IP_VS_DIR_OUTPUT, skb, pp);
-	ip_vs_conn_put(cp);
-
-	skb->ipvs_property = 1;
-
-	LeaveFunction(11);
-	return NF_ACCEPT;
-
-  drop:
-	ip_vs_conn_put(cp);
-	kfree_skb(skb);
-	return NF_STOLEN;
+	return handle_response(af, skb, pp, cp, iph.len);
 }
 
 
@@ -1111,8 +1139,14 @@ ip_vs_in_icmp(struct sk_buff *skb, int *
 	ip_vs_fill_iphdr(AF_INET, cih, &ciph);
 	/* The embedded headers contain source and dest in reverse order */
 	cp = pp->conn_in_get(AF_INET, skb, pp, &ciph, offset, 1);
-	if (!cp)
+	if (!cp) {
+		/* The packet could also belong to a local client */
+		cp = pp->conn_out_get(AF_INET, skb, pp, &ciph, offset, 1);
+		if (cp)
+			return handle_response_icmp(skb, iph, cih, cp, pp,
+						    offset, ihl);
 		return NF_ACCEPT;
+	}
 
 	verdict = NF_DROP;
 
@@ -1244,11 +1278,12 @@ ip_vs_in(unsigned int hooknum, struct sk
 	ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
 
 	/*
-	 *	Big tappo: only PACKET_HOST (neither loopback nor mcasts)
-	 *	... don't know why 1st test DOES NOT include 2nd (?)
+	 *	Big tappo: only PACKET_HOST, including loopback for local client
+	 *	Don't handle local packets on IPv6 for now
 	 */
-	if (unlikely(skb->pkt_type != PACKET_HOST
-		     || skb->dev->flags & IFF_LOOPBACK || skb->sk)) {
+	if (unlikely(skb->pkt_type != PACKET_HOST ||
+		     (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,
 			      iph.protocol,
@@ -1277,6 +1312,11 @@ ip_vs_in(unsigned int hooknum, struct sk
 	if (unlikely(!cp)) {
 		int v;
 
+		/* For local client packets, it could be a response */
+		cp = pp->conn_out_get(af, skb, pp, &iph, iph.len, 0);
+		if (cp)
+			return handle_response(af, skb, pp, cp, iph.len);
+
 		if (!pp->conn_schedule(af, skb, pp, &v, &cp))
 			return v;
 	}
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-03 10:56:05.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-03 11:24:26.000000000 +1000
@@ -166,7 +166,7 @@ tcp_snat_handler(struct sk_buff *skb,
 	tcph->source = cp->vport;
 
 	/* Adjust TCP checksums */
-	if (!cp->app) {
+	if (!cp->app && (tcph->check != 0)) {
 		/* 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);
@@ -235,7 +235,7 @@ tcp_dnat_handler(struct sk_buff *skb,
 	/*
 	 *	Adjust TCP checksums
 	 */
-	if (!cp->app) {
+	if (!cp->app && (tcph->check != 0)) {
 		/* 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);

[PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Simon Horman]

This allows IPVS to load balance IPv6 connections made by a local process.
For example a proxy server running locally.

External client --> pound:443 -> Local:443 --> IPVS:80 --> RealServer

This is an extenstion to the IPv4 work done in this area
by Siim Põder and Malcolm Turnbull.

Cc: Siim Põder <siim@p6drad-teel.net>
Cc: Malcolm Turnbull <malcolm@loadbalancer.org>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 net/ipv4/ipvs/ip_vs_core.c |   91 +++++++++++++++++++-------------------------
 1 file changed, 41 insertions(+), 50 deletions(-)

* Fri, 05 Sep 2008 11:32:38 +1000

  I have applied this patch to the net-next-2.6 branck of lvs-2.6

  git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-2.6.git

Index: lvs-2.6/net/ipv4/ipvs/ip_vs_core.c
===================================================================
--- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_core.c	2008-09-03 13:39:34.000000000 +1000
+++ lvs-2.6/net/ipv4/ipvs/ip_vs_core.c	2008-09-03 14:39:17.000000000 +1000
@@ -654,8 +654,9 @@ void ip_vs_nat_icmp_v6(struct sk_buff *s
 /* Handle relevant response ICMP messages - forward to the right
  * destination host. Used for NAT and local client.
  */
-static int handle_response_icmp(struct sk_buff *skb, struct iphdr *iph,
-				struct iphdr *cih, struct ip_vs_conn *cp,
+static int handle_response_icmp(int af, struct sk_buff *skb,
+				union nf_inet_addr *snet,
+				__u8 protocol, struct ip_vs_conn *cp,
 				struct ip_vs_protocol *pp,
 				unsigned int offset, unsigned int ihl)
 {
@@ -669,18 +670,22 @@ static int handle_response_icmp(struct s
 	/* Ensure the checksum is correct */
 	if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
 		/* Failed checksum! */
-		IP_VS_DBG(1,
-			  "Forward ICMP: failed checksum from %d.%d.%d.%d!\n",
-			  NIPQUAD(iph->saddr));
+		IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n",
+			      IP_VS_DBG_ADDR(af, snet));
 		goto out;
 	}
 
-	if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol)
+	if (IPPROTO_TCP == protocol || IPPROTO_UDP == protocol)
 		offset += 2 * sizeof(__u16);
 	if (!skb_make_writable(skb, offset))
 		goto out;
 
-	ip_vs_nat_icmp(skb, pp, cp, 1);
+#ifdef CONFIG_IP_VS_IPV6
+	if (af == AF_INET6)
+		ip_vs_nat_icmp_v6(skb, pp, cp, 1);
+	else
+#endif
+		ip_vs_nat_icmp(skb, pp, cp, 1);
 
 	/* do the statistics and put it back */
 	ip_vs_out_stats(cp, skb);
@@ -708,6 +713,7 @@ static int ip_vs_out_icmp(struct sk_buff
 	struct ip_vs_conn *cp;
 	struct ip_vs_protocol *pp;
 	unsigned int offset, ihl;
+	union nf_inet_addr snet;
 
 	*related = 1;
 
@@ -766,7 +772,9 @@ static int ip_vs_out_icmp(struct sk_buff
 	if (!cp)
 		return NF_ACCEPT;
 
-	return handle_response_icmp(skb, iph, cih, cp, pp, offset, ihl);
+	snet.ip = iph->saddr;
+	return handle_response_icmp(AF_INET, skb, &snet, cih->protocol, cp,
+				    pp, offset, ihl);
 }
 
 #ifdef CONFIG_IP_VS_IPV6
@@ -779,7 +787,8 @@ static int ip_vs_out_icmp_v6(struct sk_b
 	struct ip_vs_iphdr ciph;
 	struct ip_vs_conn *cp;
 	struct ip_vs_protocol *pp;
-	unsigned int offset, verdict;
+	unsigned int offset;
+	union nf_inet_addr snet;
 
 	*related = 1;
 
@@ -838,40 +847,9 @@ static int ip_vs_out_icmp_v6(struct sk_b
 	if (!cp)
 		return NF_ACCEPT;
 
-	verdict = NF_DROP;
-
-	if (IP_VS_FWD_METHOD(cp) != 0) {
-		IP_VS_ERR("shouldn't reach here, because the box is on the "
-			  "half connection in the tun/dr module.\n");
-	}
-
-	/* Ensure the checksum is correct */
-	if (!skb_csum_unnecessary(skb)
-	    && ip_vs_checksum_complete(skb, sizeof(struct ipv6hdr))) {
-		/* Failed checksum! */
-		IP_VS_DBG(1, "Forward ICMPv6: failed checksum from "
-			  NIP6_FMT "!\n",
-			  NIP6(iph->saddr));
-		goto out;
-	}
-
-	if (IPPROTO_TCP == cih->nexthdr || IPPROTO_UDP == cih->nexthdr)
-		offset += 2 * sizeof(__u16);
-	if (!skb_make_writable(skb, offset))
-		goto out;
-
-	ip_vs_nat_icmp_v6(skb, pp, cp, 1);
-
-	/* do the statistics and put it back */
-	ip_vs_out_stats(cp, skb);
-
-	skb->ipvs_property = 1;
-	verdict = NF_ACCEPT;
-
-out:
-	__ip_vs_conn_put(cp);
-
-	return verdict;
+	snet.in6 = iph->saddr;
+	return handle_response_icmp(AF_INET6, skb, &snet, cih->nexthdr, cp,
+				    pp, offset, sizeof(struct ipv6hdr));
 }
 #endif
 
@@ -1055,7 +1033,7 @@ ip_vs_out(unsigned int hooknum, struct s
 							  ICMP_DEST_UNREACH,
 							  ICMP_PORT_UNREACH, 0);
 					return NF_DROP;
-				}
+			}
 			}
 		}
 		IP_VS_DBG_PKT(12, pp, skb, 0,
@@ -1083,6 +1061,7 @@ ip_vs_in_icmp(struct sk_buff *skb, int *
 	struct ip_vs_conn *cp;
 	struct ip_vs_protocol *pp;
 	unsigned int offset, ihl, verdict;
+	union nf_inet_addr snet;
 
 	*related = 1;
 
@@ -1142,9 +1121,12 @@ ip_vs_in_icmp(struct sk_buff *skb, int *
 	if (!cp) {
 		/* The packet could also belong to a local client */
 		cp = pp->conn_out_get(AF_INET, skb, pp, &ciph, offset, 1);
-		if (cp)
-			return handle_response_icmp(skb, iph, cih, cp, pp,
+		if (cp) {
+			snet.ip = iph->saddr;
+			return handle_response_icmp(AF_INET, skb, &snet,
+						    cih->protocol, cp, pp,
 						    offset, ihl);
+		}
 		return NF_ACCEPT;
 	}
 
@@ -1183,6 +1165,7 @@ ip_vs_in_icmp_v6(struct sk_buff *skb, in
 	struct ip_vs_conn *cp;
 	struct ip_vs_protocol *pp;
 	unsigned int offset, verdict;
+	union nf_inet_addr snet;
 
 	*related = 1;
 
@@ -1240,8 +1223,18 @@ ip_vs_in_icmp_v6(struct sk_buff *skb, in
 	ip_vs_fill_iphdr(AF_INET6, cih, &ciph);
 	/* The embedded headers contain source and dest in reverse order */
 	cp = pp->conn_in_get(AF_INET6, skb, pp, &ciph, offset, 1);
-	if (!cp)
+	if (!cp) {
+		/* The packet could also belong to a local client */
+		cp = pp->conn_out_get(AF_INET6, skb, pp, &ciph, offset, 1);
+		if (cp) {
+			snet.in6 = iph->saddr;
+			return handle_response_icmp(AF_INET6, skb, &snet,
+						    cih->nexthdr,
+						    cp, pp, offset,
+						    sizeof(struct ipv6hdr));
+		}
 		return NF_ACCEPT;
+	}
 
 	verdict = NF_DROP;
 
@@ -1281,9 +1274,7 @@ ip_vs_in(unsigned int hooknum, struct sk
 	 *	Big tappo: only PACKET_HOST, including loopback for local client
 	 *	Don't handle local packets on IPv6 for now
 	 */
-	if (unlikely(skb->pkt_type != PACKET_HOST ||
-		     (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
-					 skb->sk)))) {
+	if (unlikely(skb->pkt_type != PACKET_HOST)) {
 		IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s ignored\n",
 			      skb->pkt_type,
 			      iph.protocol,

Re: [PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Julian Anastasov]

	Hello,

	TCP checksum is not optional, why this change appeared?

On Fri, 5 Sep 2008, Simon Horman wrote:

> 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-03 10:56:05.000000000 +1000
> +++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-03 11:24:26.000000000 +1000
> @@ -166,7 +166,7 @@ tcp_snat_handler(struct sk_buff *skb,
>  	tcph->source = cp->vport;
>  
>  	/* Adjust TCP checksums */
> -	if (!cp->app) {
> +	if (!cp->app && (tcph->check != 0)) {
>  		/* 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);
> @@ -235,7 +235,7 @@ tcp_dnat_handler(struct sk_buff *skb,
>  	/*
>  	 *	Adjust TCP checksums
>  	 */
> -	if (!cp->app) {
> +	if (!cp->app && (tcph->check != 0)) {
>  		/* 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);
> --

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Siim Põder]

Hi

> 	TCP checksum is not optional, why this change appeared?

The new packets that we handle are on the loopback device and no checksums
appear to be generated there. I initially changed the condition to check
for loopback device (which we could do), but checking udp code found that
it already handled it by checking zero checksum, hence the same
implementation in tcp code.

>> 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-03
>> 10:56:05.000000000 +1000
>> +++ lvs-2.6/net/ipv4/ipvs/ip_vs_proto_tcp.c	2008-09-03
>> 11:24:26.000000000 +1000
>> @@ -166,7 +166,7 @@ tcp_snat_handler(struct sk_buff *skb,
>>  	tcph->source = cp->vport;
>>
>>  	/* Adjust TCP checksums */
>> -	if (!cp->app) {
>> +	if (!cp->app && (tcph->check != 0)) {
>>  		/* 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);
>> @@ -235,7 +235,7 @@ tcp_dnat_handler(struct sk_buff *skb,
>>  	/*
>>  	 *	Adjust TCP checksums
>>  	 */
>> -	if (!cp->app) {
>> +	if (!cp->app && (tcph->check != 0)) {
>>  		/* 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);
>> --

-- 
Siiralt Teie,
Elektromehaaniline Põder

Re: [PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Julius Volz]

On Fri, Sep 5, 2008 at 3:36 AM, Simon Horman <horms@verge.net.au> wrote:
> @@ -1244,11 +1278,12 @@ ip_vs_in(unsigned int hooknum, struct sk
>        ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
>
>        /*
> -        *      Big tappo: only PACKET_HOST (neither loopback nor mcasts)
> -        *      ... don't know why 1st test DOES NOT include 2nd (?)
> +        *      Big tappo: only PACKET_HOST, including loopback for local client
> +        *      Don't handle local packets on IPv6 for now
>         */
> -       if (unlikely(skb->pkt_type != PACKET_HOST
> -                    || skb->dev->flags & IFF_LOOPBACK || skb->sk)) {
> +       if (unlikely(skb->pkt_type != PACKET_HOST ||
> +                    (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
> +                                        skb->sk)))) {

Hm, shouldn't this be (af == AF_INET6 && ...) instead of "||"? The
current expression just NF_ACCEPTs _any_ incoming IPv6 packets, even
non-local ones.

Julius

-- 
Julius Volz - Corporate Operations - SysOps

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

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Julius Volz]

Hi,

Just some minor things:

On Fri, Sep 5, 2008 at 3:37 AM, Simon Horman <horms@verge.net.au> wrote:
> This allows IPVS to load balance IPv6 connections made by a local process.
> For example a proxy server running locally.
>
> External client --> pound:443 -> Local:443 --> IPVS:80 --> RealServer
>
> This is an extenstion to the IPv4 work done in this area
> by Siim Põder and Malcolm Turnbull.
>
> Cc: Siim Põder <siim@p6drad-teel.net>
> Cc: Malcolm Turnbull <malcolm@loadbalancer.org>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
>  net/ipv4/ipvs/ip_vs_core.c |   91 +++++++++++++++++++-------------------------
>  1 file changed, 41 insertions(+), 50 deletions(-)
>
> * Fri, 05 Sep 2008 11:32:38 +1000
>
>  I have applied this patch to the net-next-2.6 branck of lvs-2.6
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-2.6.git
>
> Index: lvs-2.6/net/ipv4/ipvs/ip_vs_core.c
> ===================================================================
> --- lvs-2.6.orig/net/ipv4/ipvs/ip_vs_core.c     2008-09-03 13:39:34.000000000 +1000
> +++ lvs-2.6/net/ipv4/ipvs/ip_vs_core.c  2008-09-03 14:39:17.000000000 +1000
> @@ -654,8 +654,9 @@ void ip_vs_nat_icmp_v6(struct sk_buff *s
>  /* Handle relevant response ICMP messages - forward to the right
>  * destination host. Used for NAT and local client.
>  */
> -static int handle_response_icmp(struct sk_buff *skb, struct iphdr *iph,
> -                               struct iphdr *cih, struct ip_vs_conn *cp,
> +static int handle_response_icmp(int af, struct sk_buff *skb,
> +                               union nf_inet_addr *snet,
> +                               __u8 protocol, struct ip_vs_conn *cp,
>                                struct ip_vs_protocol *pp,
>                                unsigned int offset, unsigned int ihl)
>  {
> @@ -669,18 +670,22 @@ static int handle_response_icmp(struct s
>        /* Ensure the checksum is correct */
>        if (!skb_csum_unnecessary(skb) && ip_vs_checksum_complete(skb, ihl)) {
>                /* Failed checksum! */
> -               IP_VS_DBG(1,
> -                         "Forward ICMP: failed checksum from %d.%d.%d.%d!\n",
> -                         NIPQUAD(iph->saddr));
> +               IP_VS_DBG_BUF(1, "Forward ICMP: failed checksum from %s!\n",
> +                             IP_VS_DBG_ADDR(af, snet));
>                goto out;
>        }
>
> -       if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol)
> +       if (IPPROTO_TCP == protocol || IPPROTO_UDP == protocol)
>                offset += 2 * sizeof(__u16);
>        if (!skb_make_writable(skb, offset))
>                goto out;
>
> -       ip_vs_nat_icmp(skb, pp, cp, 1);
> +#ifdef CONFIG_IP_VS_IPV6
> +       if (af == AF_INET6)
> +               ip_vs_nat_icmp_v6(skb, pp, cp, 1);
> +       else
> +#endif
> +               ip_vs_nat_icmp(skb, pp, cp, 1);
>
>        /* do the statistics and put it back */
>        ip_vs_out_stats(cp, skb);
> @@ -708,6 +713,7 @@ static int ip_vs_out_icmp(struct sk_buff
>        struct ip_vs_conn *cp;
>        struct ip_vs_protocol *pp;
>        unsigned int offset, ihl;
> +       union nf_inet_addr snet;
>
>        *related = 1;
>
> @@ -766,7 +772,9 @@ static int ip_vs_out_icmp(struct sk_buff
>        if (!cp)
>                return NF_ACCEPT;
>
> -       return handle_response_icmp(skb, iph, cih, cp, pp, offset, ihl);
> +       snet.ip = iph->saddr;
> +       return handle_response_icmp(AF_INET, skb, &snet, cih->protocol, cp,
> +                                   pp, offset, ihl);
>  }
>
>  #ifdef CONFIG_IP_VS_IPV6
> @@ -779,7 +787,8 @@ static int ip_vs_out_icmp_v6(struct sk_b
>        struct ip_vs_iphdr ciph;
>        struct ip_vs_conn *cp;
>        struct ip_vs_protocol *pp;
> -       unsigned int offset, verdict;
> +       unsigned int offset;
> +       union nf_inet_addr snet;
>
>        *related = 1;
>
> @@ -838,40 +847,9 @@ static int ip_vs_out_icmp_v6(struct sk_b
>        if (!cp)
>                return NF_ACCEPT;
>
> -       verdict = NF_DROP;
> -
> -       if (IP_VS_FWD_METHOD(cp) != 0) {
> -               IP_VS_ERR("shouldn't reach here, because the box is on the "
> -                         "half connection in the tun/dr module.\n");
> -       }
> -
> -       /* Ensure the checksum is correct */
> -       if (!skb_csum_unnecessary(skb)
> -           && ip_vs_checksum_complete(skb, sizeof(struct ipv6hdr))) {
> -               /* Failed checksum! */
> -               IP_VS_DBG(1, "Forward ICMPv6: failed checksum from "
> -                         NIP6_FMT "!\n",
> -                         NIP6(iph->saddr));
> -               goto out;
> -       }
> -
> -       if (IPPROTO_TCP == cih->nexthdr || IPPROTO_UDP == cih->nexthdr)
> -               offset += 2 * sizeof(__u16);
> -       if (!skb_make_writable(skb, offset))
> -               goto out;
> -
> -       ip_vs_nat_icmp_v6(skb, pp, cp, 1);
> -
> -       /* do the statistics and put it back */
> -       ip_vs_out_stats(cp, skb);
> -
> -       skb->ipvs_property = 1;
> -       verdict = NF_ACCEPT;
> -
> -out:
> -       __ip_vs_conn_put(cp);
> -
> -       return verdict;
> +       snet.in6 = iph->saddr;

I've always been told to use ipv6_addr_copy() for this. I'm not sure
what the problem with the direct struct assignment is though... would
be nice if someone could explain.

> +       return handle_response_icmp(AF_INET6, skb, &snet, cih->nexthdr, cp,
> +                                   pp, offset, sizeof(struct ipv6hdr));
>  }
>  #endif
>
> @@ -1055,7 +1033,7 @@ ip_vs_out(unsigned int hooknum, struct s
>                                                          ICMP_DEST_UNREACH,
>                                                          ICMP_PORT_UNREACH, 0);
>                                        return NF_DROP;
> -                               }
> +                       }

This indents the curly brace incorrectly, probably just deleted a tab
by accident...

>                        }
>                }
>                IP_VS_DBG_PKT(12, pp, skb, 0,
> @@ -1083,6 +1061,7 @@ ip_vs_in_icmp(struct sk_buff *skb, int *
>        struct ip_vs_conn *cp;
>        struct ip_vs_protocol *pp;
>        unsigned int offset, ihl, verdict;
> +       union nf_inet_addr snet;
>
>        *related = 1;
>
> @@ -1142,9 +1121,12 @@ ip_vs_in_icmp(struct sk_buff *skb, int *
>        if (!cp) {
>                /* The packet could also belong to a local client */
>                cp = pp->conn_out_get(AF_INET, skb, pp, &ciph, offset, 1);
> -               if (cp)
> -                       return handle_response_icmp(skb, iph, cih, cp, pp,
> +               if (cp) {
> +                       snet.ip = iph->saddr;
> +                       return handle_response_icmp(AF_INET, skb, &snet,
> +                                                   cih->protocol, cp, pp,
>                                                    offset, ihl);
> +               }
>                return NF_ACCEPT;
>        }
>
> @@ -1183,6 +1165,7 @@ ip_vs_in_icmp_v6(struct sk_buff *skb, in
>        struct ip_vs_conn *cp;
>        struct ip_vs_protocol *pp;
>        unsigned int offset, verdict;
> +       union nf_inet_addr snet;
>
>        *related = 1;
>
> @@ -1240,8 +1223,18 @@ ip_vs_in_icmp_v6(struct sk_buff *skb, in
>        ip_vs_fill_iphdr(AF_INET6, cih, &ciph);
>        /* The embedded headers contain source and dest in reverse order */
>        cp = pp->conn_in_get(AF_INET6, skb, pp, &ciph, offset, 1);
> -       if (!cp)
> +       if (!cp) {
> +               /* The packet could also belong to a local client */
> +               cp = pp->conn_out_get(AF_INET6, skb, pp, &ciph, offset, 1);
> +               if (cp) {
> +                       snet.in6 = iph->saddr;

ipv6_addr_copy() again.

> +                       return handle_response_icmp(AF_INET6, skb, &snet,
> +                                                   cih->nexthdr,
> +                                                   cp, pp, offset,
> +                                                   sizeof(struct ipv6hdr));
> +               }
>                return NF_ACCEPT;
> +       }
>
>        verdict = NF_DROP;
>
> @@ -1281,9 +1274,7 @@ ip_vs_in(unsigned int hooknum, struct sk
>         *      Big tappo: only PACKET_HOST, including loopback for local client
>         *      Don't handle local packets on IPv6 for now
>         */
> -       if (unlikely(skb->pkt_type != PACKET_HOST ||
> -                    (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
> -                                        skb->sk)))) {
> +       if (unlikely(skb->pkt_type != PACKET_HOST)) {
>                IP_VS_DBG_BUF(12, "packet type=%d proto=%d daddr=%s ignored\n",
>                              skb->pkt_type,
>                              iph.protocol,
>


-- 
Julius Volz - Corporate Operations - SysOps

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

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Brian Haley]

Julius Volz wrote:
>> -out:
>> -       __ip_vs_conn_put(cp);
>> -
>> -       return verdict;
>> +       snet.in6 = iph->saddr;
> 
> I've always been told to use ipv6_addr_copy() for this. I'm not sure
> what the problem with the direct struct assignment is though... would
> be nice if someone could explain.

Because an in6_addr is a union of 4 u32's, which won't all be copied in 
a struct assignment.  That's the way I've always understood it.

-Brian

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Julius Volz]

On Fri, Sep 5, 2008 at 5:55 PM, Brian Haley <brian.haley@hp.com> wrote:
> Julius Volz wrote:
>>>
>>> -out:
>>> -       __ip_vs_conn_put(cp);
>>> -
>>> -       return verdict;
>>> +       snet.in6 = iph->saddr;
>>
>> I've always been told to use ipv6_addr_copy() for this. I'm not sure
>> what the problem with the direct struct assignment is though... would
>> be nice if someone could explain.
>
> Because an in6_addr is a union of 4 u32's, which won't all be copied in a
> struct assignment.  That's the way I've always understood it.

No, they _do_ all get copied and the result seems correct. There must
be another reason...

Julius

-- 
Julius Volz - Corporate Operations - SysOps

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

Re: [PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Simon Horman]

On Fri, Sep 05, 2008 at 01:02:44PM +0200, Julius Volz wrote:
> On Fri, Sep 5, 2008 at 3:36 AM, Simon Horman <horms@verge.net.au> wrote:
> > @@ -1244,11 +1278,12 @@ ip_vs_in(unsigned int hooknum, struct sk
> >        ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
> >
> >        /*
> > -        *      Big tappo: only PACKET_HOST (neither loopback nor mcasts)
> > -        *      ... don't know why 1st test DOES NOT include 2nd (?)
> > +        *      Big tappo: only PACKET_HOST, including loopback for local client
> > +        *      Don't handle local packets on IPv6 for now
> >         */
> > -       if (unlikely(skb->pkt_type != PACKET_HOST
> > -                    || skb->dev->flags & IFF_LOOPBACK || skb->sk)) {
> > +       if (unlikely(skb->pkt_type != PACKET_HOST ||
> > +                    (af == AF_INET6 || (skb->dev->flags & IFF_LOOPBACK ||
> > +                                        skb->sk)))) {
> 
> Hm, shouldn't this be (af == AF_INET6 && ...) instead of "||"? The
> current expression just NF_ACCEPTs _any_ incoming IPv6 packets, even
> non-local ones.

Yes, sorry about that. It should have been &&.
This check was removed alltogether by the subsequent patch.

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Simon Horman]

On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
> Hi,
> 
> Just some minor things:

Hi Julius,

thanks for pointing these out. I will send patches to fix these problems ASAP.

More generally speaking, sorry to all parties concerned for applying
these two patches a little prematurely. At this stage I'd rather move
forward rather than roll the tree back. But I am happy to discuss this.

Re: [PATCH 1/2] ipvs: load balance IPv4 connections from a local process

[Simon Horman]

On Fri, Sep 05, 2008 at 08:49:52AM +0300, Siim Põder wrote:
> Hi
> 
> > 	TCP checksum is not optional, why this change appeared?
> 
> The new packets that we handle are on the loopback device and no checksums
> appear to be generated there. I initially changed the condition to check
> for loopback device (which we could do), but checking udp code found that
> it already handled it by checking zero checksum, hence the same
> implementation in tcp code.

I checked with Herbert Xu and its not legal for the checksum to be missing
for TCP. However it is possible for there to be a partial checksum for
loopback traffic. That is, only the pseudo-header is summed.

The patch outlines a fix for this problem using the existing
structure of ip_vs_proto_tcp.c. I believe that a similar fix
is also required for UDP. I am posting it now so people can comment
on its correctness.


Moving forward tcp_partial_csum_update() and tcp_fast_csum_update()
could be implemented in terms of inet_proto_csum_replace* if
inet_proto_csum_replace16 was added. I will work on coding this up.


diff --git a/net/ipv4/ipvs/ip_vs_proto_tcp.c b/net/ipv4/ipvs/ip_vs_proto_tcp.c
index 808e8be..537f616 100644
--- a/net/ipv4/ipvs/ip_vs_proto_tcp.c
+++ b/net/ipv4/ipvs/ip_vs_proto_tcp.c
@@ -134,12 +134,34 @@ tcp_fast_csum_update(int af, struct tcphdr *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);

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Julius Volz]

On Sat, Sep 6, 2008 at 6:14 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
>> Hi,
>>
>> Just some minor things:
>
> Hi Julius,
>
> thanks for pointing these out. I will send patches to fix these problems ASAP.

Thanks, looking good!

> More generally speaking, sorry to all parties concerned for applying
> these two patches a little prematurely. At this stage I'd rather move
> forward rather than roll the tree back. But I am happy to discuss this.

Probably better to just go forward when the tree is already public?

Julius

-- 
Julius Volz - Corporate Operations - SysOps

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

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Simon Horman]

On Sat, Sep 06, 2008 at 11:26:29AM +0200, Julius Volz wrote:
> On Sat, Sep 6, 2008 at 6:14 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
> >> Hi,
> >>
> >> Just some minor things:
> >
> > Hi Julius,
> >
> > thanks for pointing these out. I will send patches to fix these problems ASAP.
> 
> Thanks, looking good!
> 
> > More generally speaking, sorry to all parties concerned for applying
> > these two patches a little prematurely. At this stage I'd rather move
> > forward rather than roll the tree back. But I am happy to discuss this.
> 
> Probably better to just go forward when the tree is already public?

Yes, thats what I was thinking to.

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Simon Horman]

On Sat, Sep 06, 2008 at 11:26:29AM +0200, Julius Volz wrote:
> On Sat, Sep 6, 2008 at 6:14 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
> >> Hi,
> >>
> >> Just some minor things:
> >
> > Hi Julius,
> >
> > thanks for pointing these out. I will send patches to fix these problems ASAP.
> 
> Thanks, looking good!

I took the liberty of taking that response as an Acked-by.
I've applied these minor changes now.

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Julius Volz]

On Mon, Sep 8, 2008 at 3:48 AM, Simon Horman <horms@verge.net.au> wrote:
> On Sat, Sep 06, 2008 at 11:26:29AM +0200, Julius Volz wrote:
>> On Sat, Sep 6, 2008 at 6:14 AM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
>> >> Hi,
>> >>
>> >> Just some minor things:
>> >
>> > Hi Julius,
>> >
>> > thanks for pointing these out. I will send patches to fix these problems ASAP.
>>
>> Thanks, looking good!
>
> I took the liberty of taking that response as an Acked-by.
> I've applied these minor changes now.
>

Thanks! I'll remember to add an explicit Acked-by in the future.

I'll try out your new checksumming patches now and see if everything
works for me (for both remote connections and ones from the
director)...

Julius

-- 
Julius Volz - Corporate Operations - SysOps

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

Re: [PATCH 2/2] ipvs: load balance ipv6 connections from a local process

[Simon Horman]

On Mon, Sep 08, 2008 at 11:30:08AM +0200, Julius Volz wrote:
> On Mon, Sep 8, 2008 at 3:48 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Sat, Sep 06, 2008 at 11:26:29AM +0200, Julius Volz wrote:
> >> On Sat, Sep 6, 2008 at 6:14 AM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Fri, Sep 05, 2008 at 01:40:31PM +0200, Julius Volz wrote:
> >> >> Hi,
> >> >>
> >> >> Just some minor things:
> >> >
> >> > Hi Julius,
> >> >
> >> > thanks for pointing these out. I will send patches to fix these problems ASAP.
> >>
> >> Thanks, looking good!
> >
> > I took the liberty of taking that response as an Acked-by.
> > I've applied these minor changes now.
> >
> 
> Thanks! I'll remember to add an explicit Acked-by in the future.
> 
> I'll try out your new checksumming patches now and see if everything
> works for me (for both remote connections and ones from the
> director)...

Thanks, much appericated.

I'm also a little concerned about the handling of IPv6 ICMP in
this patch. In particular, we need to ensure that the pseudo-header
is included in the checksum. Which it may not be, as I based
the code on the IPv4 version.

http://rfc-ref.org/RFC-TEXTS/1883/chapter8.html#d4e447886