* [patch 0/2] ipvs: avoid overcommit on the standby, take II @ 2007-11-01 9:28 Simon Horman 2007-11-01 9:28 ` [patch 1/2] ipvs: Bind connections on stanby if the destination exists Simon Horman 2007-11-01 9:28 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections Simon Horman 0 siblings, 2 replies; 8+ messages in thread From: Simon Horman @ 2007-11-01 9:28 UTC (permalink / raw) To: lvs-devel, netdev Cc: Wensong Zhang, Rumen G. Bogdanovski, Julian Anastasov, Graeme Fowler, Joseph Mack NA3T, David S. Miller Two related patches from Rumen G. Bogdanovski to help prevent overcommit on the standby. After sending the last patchset I discovered scripts/checkpatch.pl and found that it didn't like quite a few things. I've fixed those problems and here is the result. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch 1/2] ipvs: Bind connections on stanby if the destination exists 2007-11-01 9:28 [patch 0/2] ipvs: avoid overcommit on the standby, take II Simon Horman @ 2007-11-01 9:28 ` Simon Horman 2007-11-01 9:28 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections Simon Horman 1 sibling, 0 replies; 8+ messages in thread From: Simon Horman @ 2007-11-01 9:28 UTC (permalink / raw) To: lvs-devel, netdev Cc: Wensong Zhang, Rumen G. Bogdanovski, Julian Anastasov, Graeme Fowler, Joseph Mack NA3T, David S. Miller [-- Attachment #1: linux-2.6.23.1-ipvs-rb.patch --] [-- Type: text/plain, Size: 7357 bytes --] From: Rumen G. Bogdanovski <rumen@voicecho.com> This patch fixes the problem with node overload on director fail-over. Given the scenario: 2 nodes each accepting 3 connections at a time and 2 directors, director failover occurs when the nodes are fully loaded (6 connections to the cluster) in this case the new director will assign another 6 connections to the cluster, If the same real servers exist there. The problem turned to be in not binding the inherited connections to the real servers (destinations) on the backup director. Therefore: "ipvsadm -l" reports 0 connections: root@test2:~# ipvsadm -l IP Virtual Server version 1.2.1 (size=4096) Prot LocalAddress:Port Scheduler Flags -> RemoteAddress:Port Forward Weight ActiveConn InActConn TCP test2.local:5999 wlc -> node473.local:5999 Route 1000 0 0 -> node484.local:5999 Route 1000 0 0 while "ipvs -lnc" is right root@test2:~# ipvsadm -lnc IPVS connection entries pro expire state source virtual destination TCP 14:56 ESTABLISHED 192.168.0.10:39164 192.168.0.222:5999 192.168.0.51:5999 TCP 14:59 ESTABLISHED 192.168.0.10:39165 192.168.0.222:5999 192.168.0.52:5999 So the patch I am sending fixes the problem by binding the received connections to the appropriate service on the backup director, if it exists, else the connection will be handled the old way. So if the master and the backup directors are synchronized in terms of real services there will be no problem with server over-committing since new connections will not be created on the nonexistent real services on the backup. However if the service is created later on the backup, the binding will be performed when the next connection update is received. With this patch the inherited connections will show as inactive on the backup: root@test2:~# ipvsadm -l IP Virtual Server version 1.2.1 (size=4096) Prot LocalAddress:Port Scheduler Flags -> RemoteAddress:Port Forward Weight ActiveConn InActConn TCP test2.local:5999 wlc -> node473.local:5999 Route 1000 0 1 -> node484.local:5999 Route 1000 0 1 rumen@test2:~$ cat /proc/net/ip_vs IP Virtual Server version 1.2.1 (size=4096) Prot LocalAddress:Port Scheduler Flags -> RemoteAddress:Port Forward Weight ActiveConn InActConn TCP C0A800DE:176F wlc -> C0A80033:176F Route 1000 0 1 -> C0A80032:176F Route 1000 0 1 Regards, Rumen Bogdanovski Acked-by: Julian Anastasov <ja@ssi.bg> Signed-off-by: Rumen G. Bogdanovski <rumen@voicecho.com> Signed-off-by: Simon Horman <horms@verge.net.au> --- Thu, 01 Nov 2007 18:26:24 +0900, Horms * Various whitespace and indentation changes * Rediffed against net-2.6 * Ran against ./scripts/checkpatch.pl and fixed everything that it complained about Index: net-2.6/include/net/ip_vs.h =================================================================== --- net-2.6.orig/include/net/ip_vs.h 2007-11-01 17:57:30.000000000 +0900 +++ net-2.6/include/net/ip_vs.h 2007-11-01 18:06:56.000000000 +0900 @@ -901,6 +901,10 @@ extern int ip_vs_use_count_inc(void); extern void ip_vs_use_count_dec(void); extern int ip_vs_control_init(void); extern void ip_vs_control_cleanup(void); +extern struct ip_vs_dest * +ip_vs_find_dest(__be32 daddr, __be16 dport, + __be32 vaddr, __be16 vport, __u16 protocol); +extern struct ip_vs_dest *ip_vs_try_bind_dest(struct ip_vs_conn *cp); /* Index: net-2.6/net/ipv4/ipvs/ip_vs_conn.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_conn.c 2007-11-01 17:57:30.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_conn.c 2007-11-01 18:06:47.000000000 +0900 @@ -426,6 +426,25 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, s /* + * Check if there is a destination for the connection, if so + * bind the connection to the destination. + */ +struct ip_vs_dest *ip_vs_try_bind_dest(struct ip_vs_conn *cp) +{ + struct ip_vs_dest *dest; + + if ((cp) && (!cp->dest)) { + dest = ip_vs_find_dest(cp->daddr, cp->dport, + cp->vaddr, cp->vport, cp->protocol); + ip_vs_bind_dest(cp, dest); + return dest; + } else + return NULL; +} +EXPORT_SYMBOL(ip_vs_try_bind_dest); + + +/* * Unbind a connection entry with its VS destination * Called by the ip_vs_conn_expire function. */ Index: net-2.6/net/ipv4/ipvs/ip_vs_ctl.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_ctl.c 2007-11-01 17:57:30.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_ctl.c 2007-11-01 18:06:47.000000000 +0900 @@ -579,6 +579,34 @@ ip_vs_lookup_dest(struct ip_vs_service * return NULL; } +/* + * Find destination by {daddr,dport,vaddr,protocol} + * Cretaed to be used in ip_vs_process_message() in + * the backup synchronization daemon. It finds the + * destination to be bound to the received connection + * on the backup. + * + * ip_vs_lookup_real_service() looked promissing, but + * seems not working as expected. + */ +struct ip_vs_dest *ip_vs_find_dest(__be32 daddr, __be16 dport, + __be32 vaddr, __be16 vport, + __u16 protocol) +{ + struct ip_vs_dest *dest; + struct ip_vs_service *svc; + + svc = ip_vs_service_get(0, protocol, vaddr, vport); + if (!svc) + return NULL; + dest = ip_vs_lookup_dest(svc, daddr, dport); + if (dest) + atomic_inc(&dest->refcnt); + ip_vs_service_put(svc); + + return dest; +} +EXPORT_SYMBOL(ip_vs_find_dest); /* * Lookup dest by {svc,addr,port} in the destination trash. Index: net-2.6/net/ipv4/ipvs/ip_vs_sync.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 17:57:30.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:06:56.000000000 +0900 @@ -284,6 +284,7 @@ static void ip_vs_process_message(const struct ip_vs_sync_conn_options *opt; struct ip_vs_conn *cp; struct ip_vs_protocol *pp; + struct ip_vs_dest *dest; char *p; int i; @@ -317,20 +318,35 @@ static void ip_vs_process_message(const s->caddr, s->cport, s->vaddr, s->vport); if (!cp) { + /* + * Find the appropriate destination for the connection. + * If it is not found the connection will remain unbound + * but still handled. + */ + dest = ip_vs_find_dest(s->daddr, s->dport, + s->vaddr, s->vport, + s->protocol); cp = ip_vs_conn_new(s->protocol, s->caddr, s->cport, s->vaddr, s->vport, s->daddr, s->dport, - flags, NULL); + flags, dest); + if (dest) + atomic_dec(&dest->refcnt); if (!cp) { IP_VS_ERR("ip_vs_conn_new failed\n"); return; } cp->state = ntohs(s->state); } else if (!cp->dest) { - /* it is an entry created by the synchronization */ - cp->state = ntohs(s->state); - cp->flags = flags | IP_VS_CONN_F_HASHED; + dest = ip_vs_try_bind_dest(cp); + if (!dest) { + /* it is an unbound entry created by + * synchronization */ + cp->state = ntohs(s->state); + cp->flags = flags | IP_VS_CONN_F_HASHED; + } else + atomic_dec(&dest->refcnt); } /* Note that we don't touch its state and flags if it is a normal entry. */ -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-01 9:28 [patch 0/2] ipvs: avoid overcommit on the standby, take II Simon Horman 2007-11-01 9:28 ` [patch 1/2] ipvs: Bind connections on stanby if the destination exists Simon Horman @ 2007-11-01 9:28 ` Simon Horman 2007-11-01 23:36 ` Julian Anastasov 1 sibling, 1 reply; 8+ messages in thread From: Simon Horman @ 2007-11-01 9:28 UTC (permalink / raw) To: lvs-devel, netdev Cc: Wensong Zhang, Rumen G. Bogdanovski, Julian Anastasov, Graeme Fowler, Joseph Mack NA3T, David S. Miller [-- Attachment #1: linux-2.6.23.1-ipvs-csync-ag-inc-rb.patch --] [-- Type: text/plain, Size: 5142 bytes --] From: Rumen G. Bogdanovski <rumen@voicecho.com> This patch makes the master daemon to sync the connection when it is about to close. This makes the connections on the backup to close or timeout according their state. Before the sync was performed only if the connection is in ESTABLISHED state which always made the connections to timeout in the hard coded 3 minutes. However the Andy Gospodarek's patch ([IPVS]: use proper timeout instead of fixed value) effectively did nothing more than increasing this to 15 minutes (Established state timeout). So this patch makes use of proper timeout since it syncs the connections on status changes to FIN_WAIT (2min timeout) and CLOSE (10sec timeout). However if the backup misses CLOSE hopefully it did not miss FIN_WAIT. Otherwise we will just have to wait for the ESTABLISHED state timeout. As it is without this patch. This way the number of the hanging connections on the backup is kept to minimum. And very few of them will be left to timeout with a long timeout. This is important if we want to make use of the fix for the real server overcommit on master/backup fail-over. Regards, Rumen Bogdanovski Signed-off-by: Rumen G. Bogdanovski <rumen@voicecho.com> Signed-off-by: Simon Horman <horms@verge.net.au> --- Thu, 01 Nov 2007 18:25:10 +0900, Horms * Redifed for net-2.6 * Ran through scripts/checkpatch.pl and fixed up everything that it complains about except the use of volatile, as its in keeping with other fields in the structure. If its wrong, lets fix them all together. WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #49: FILE: include/net/ip_vs.h:523: + volatile __u16 old_state; /* old state, to be used for Index: net-2.6/include/net/ip_vs.h =================================================================== --- net-2.6.orig/include/net/ip_vs.h 2007-11-01 18:17:55.000000000 +0900 +++ net-2.6/include/net/ip_vs.h 2007-11-01 18:21:02.000000000 +0900 @@ -520,6 +520,10 @@ struct ip_vs_conn { spinlock_t lock; /* lock for state transition */ volatile __u16 flags; /* status flags */ volatile __u16 state; /* state info */ + volatile __u16 old_state; /* old state, to be used for + * state transition triggerd + * synchronization + */ /* Control members */ struct ip_vs_conn *control; /* Master control connection */ Index: net-2.6/net/ipv4/ipvs/ip_vs_core.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_core.c 2007-11-01 18:17:55.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_core.c 2007-11-01 18:18:23.000000000 +0900 @@ -979,15 +979,23 @@ ip_vs_in(unsigned int hooknum, struct sk ret = NF_ACCEPT; } - /* increase its packet counter and check if it is needed - to be synchronized */ + /* Increase its packet counter and check if it is needed + * to be synchronized + * + * Sync connection if it is about to close to + * encorage the standby servers to update the connections timeout + */ atomic_inc(&cp->in_pkts); if ((ip_vs_sync_state & IP_VS_STATE_MASTER) && - (cp->protocol != IPPROTO_TCP || - cp->state == IP_VS_TCP_S_ESTABLISHED) && - (atomic_read(&cp->in_pkts) % sysctl_ip_vs_sync_threshold[1] - == sysctl_ip_vs_sync_threshold[0])) + (((cp->protocol != IPPROTO_TCP || + cp->state == IP_VS_TCP_S_ESTABLISHED) && + (atomic_read(&cp->in_pkts) % sysctl_ip_vs_sync_threshold[1] + == sysctl_ip_vs_sync_threshold[0])) || + ((cp->protocol == IPPROTO_TCP) && (cp->old_state != cp->state) && + ((cp->state == IP_VS_TCP_S_FIN_WAIT) || + (cp->state == IP_VS_TCP_S_CLOSE))))) ip_vs_sync_conn(cp); + cp->old_state = cp->state; ip_vs_conn_put(cp); return ret; Index: net-2.6/net/ipv4/ipvs/ip_vs_sync.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:17:55.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:20:30.000000000 +0900 @@ -332,7 +332,7 @@ static void ip_vs_process_message(const s->daddr, s->dport, flags, dest); if (dest) - atomic_dec(&dest->refcnt); + ip_vs_dest_get(dest); if (!cp) { IP_VS_ERR("ip_vs_conn_new failed\n"); return; @@ -343,10 +343,9 @@ static void ip_vs_process_message(const if (!dest) { /* it is an unbound entry created by * synchronization */ - cp->state = ntohs(s->state); cp->flags = flags | IP_VS_CONN_F_HASHED; } else - atomic_dec(&dest->refcnt); + ip_vs_dest_put(dest); } /* Note that we don't touch its state and flags if it is a normal entry. */ @@ -358,6 +357,7 @@ static void ip_vs_process_message(const p += SIMPLE_CONN_SIZE; atomic_set(&cp->in_pkts, sysctl_ip_vs_sync_threshold[0]); + cp->state = ntohs(s->state); pp = ip_vs_proto_get(s->protocol); cp->timeout = pp->timeout_table[cp->state]; ip_vs_conn_put(cp); -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-01 9:28 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections Simon Horman @ 2007-11-01 23:36 ` Julian Anastasov 2007-11-02 0:53 ` Simon Horman 0 siblings, 1 reply; 8+ messages in thread From: Julian Anastasov @ 2007-11-01 23:36 UTC (permalink / raw) To: Simon Horman Cc: lvs-devel, netdev, Wensong Zhang, Rumen G. Bogdanovski, Graeme Fowler, Joseph Mack NA3T, David S. Miller Hello, On Thu, 1 Nov 2007, Simon Horman wrote: > --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:17:55.000000000 +0900 > +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:20:30.000000000 +0900 > @@ -332,7 +332,7 @@ static void ip_vs_process_message(const > s->daddr, s->dport, > flags, dest); > if (dest) Is that correct? Sorry, I was flooded with different versions of this patch and I'm not sure if it is the final one. > - atomic_dec(&dest->refcnt); > + ip_vs_dest_get(dest); > if (!cp) { > IP_VS_ERR("ip_vs_conn_new failed\n"); > return; Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-01 23:36 ` Julian Anastasov @ 2007-11-02 0:53 ` Simon Horman 2007-11-02 9:47 ` [lvs-devel] " Rumen Bogdanovski 0 siblings, 1 reply; 8+ messages in thread From: Simon Horman @ 2007-11-02 0:53 UTC (permalink / raw) To: Julian Anastasov Cc: lvs-devel, netdev, Wensong Zhang, Rumen G. Bogdanovski, Graeme Fowler, Joseph Mack NA3T, David S. Miller On Fri, Nov 02, 2007 at 01:36:07AM +0200, Julian Anastasov wrote: > > Hello, > > On Thu, 1 Nov 2007, Simon Horman wrote: > > > --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:17:55.000000000 +0900 > > +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:20:30.000000000 +0900 > > @@ -332,7 +332,7 @@ static void ip_vs_process_message(const > > s->daddr, s->dport, > > flags, dest); > > if (dest) > > Is that correct? Sorry, I was flooded with different versions > of this patch and I'm not sure if it is the final one. > > > - atomic_dec(&dest->refcnt); > > + ip_vs_dest_get(dest); > > if (!cp) { > > IP_VS_ERR("ip_vs_conn_new failed\n"); > > return; The ip_vs_dest_get() call shouldn't be there. I'll double check the rest of the patch. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [lvs-devel] [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-02 0:53 ` Simon Horman @ 2007-11-02 9:47 ` Rumen Bogdanovski 0 siblings, 0 replies; 8+ messages in thread From: Rumen Bogdanovski @ 2007-11-02 9:47 UTC (permalink / raw) To: LVS Development mailing list Cc: Julian Anastasov, Wensong Zhang, netdev, Graeme Fowler, Joseph Mack NA3T, David S. Miller I should have been very tired to miss this one when I looked at the patch... I looked again in the version I have tested. For some reason/net/ipv4/ipvs/ip_vs_sync.c is not patched at all, this is what I have missed. Rumen On Fri, 2007-11-02 at 09:53 +0900, Simon Horman wrote: > On Fri, Nov 02, 2007 at 01:36:07AM +0200, Julian Anastasov wrote: > > > > Hello, > > > > On Thu, 1 Nov 2007, Simon Horman wrote: > > > > > --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:17:55.000000000 +0900 > > > +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-01 18:20:30.000000000 +0900 > > > @@ -332,7 +332,7 @@ static void ip_vs_process_message(const > > > s->daddr, s->dport, > > > flags, dest); > > > if (dest) > > > > Is that correct? Sorry, I was flooded with different versions > > of this patch and I'm not sure if it is the final one. > > > > > - atomic_dec(&dest->refcnt); > > > + ip_vs_dest_get(dest); > > > if (!cp) { > > > IP_VS_ERR("ip_vs_conn_new failed\n"); > > > return; > > The ip_vs_dest_get() call shouldn't be there. > I'll double check the rest of the patch. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch 0/2] ipvs: avoid overcommit on the standby, take III @ 2007-11-05 3:08 horms, Simon Horman 2007-11-05 3:08 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections horms, Simon Horman 0 siblings, 1 reply; 8+ messages in thread From: horms, Simon Horman @ 2007-11-05 3:08 UTC (permalink / raw) To: lvs-devel, netdev Cc: Rumen G. Bogdanovski, Graeme Fowler, Joseph Mack NA3T, Julian Anastasov, Wensong Zhang, David S. Miller Two related patches from Rumen G. Bogdanovski to help prevent overcommit on the standby. On the last two attempts I have managed to send somewhat bogus patches. So I started from scratch. I tool the original patches, fixed up what scripts/checkpatch.pl didn't like, then compared the output to my previous attempt, which happily showed the bogus bits that I know about have been fixed. -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-05 3:08 [patch 0/2] ipvs: avoid overcommit on the standby, take III horms, Simon Horman @ 2007-11-05 3:08 ` horms, Simon Horman 2007-11-07 10:37 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: horms, Simon Horman @ 2007-11-05 3:08 UTC (permalink / raw) To: lvs-devel, netdev Cc: Rumen G. Bogdanovski, Graeme Fowler, Joseph Mack NA3T, Julian Anastasov, Wensong Zhang, David S. Miller [-- Attachment #1: linux-2.6.23.1-ipvs-csync-ag-inc-rb.patch --] [-- Type: text/plain, Size: 4770 bytes --] From: Rumen G. Bogdanovski <rumen@voicecho.com> This patch makes the master daemon to sync the connection when it is about to close. This makes the connections on the backup to close or timeout according their state. Before the sync was performed only if the connection is in ESTABLISHED state which always made the connections to timeout in the hard coded 3 minutes. However the Andy Gospodarek's patch ([IPVS]: use proper timeout instead of fixed value) effectively did nothing more than increasing this to 15 minutes (Established state timeout). So this patch makes use of proper timeout since it syncs the connections on status changes to FIN_WAIT (2min timeout) and CLOSE (10sec timeout). However if the backup misses CLOSE hopefully it did not miss FIN_WAIT. Otherwise we will just have to wait for the ESTABLISHED state timeout. As it is without this patch. This way the number of the hanging connections on the backup is kept to minimum. And very few of them will be left to timeout with a long timeout. This is important if we want to make use of the fix for the real server overcommit on master/backup fail-over. Regards, Rumen Bogdanovski Signed-off-by: Rumen G. Bogdanovski <rumen@voicecho.com> Signed-off-by: Simon Horman <horms@verge.net.au> --- Thu, 01 Nov 2007 18:25:10 +0900, Horms * Redifed for net-2.6 * Ran through scripts/checkpatch.pl and fixed up everything that it complains about except the use of volatile, as its in keeping with other fields in the structure. If its wrong, lets fix them all together. WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #49: FILE: include/net/ip_vs.h:523: + volatile __u16 old_state; /* old state, to be used for Index: net-2.6/include/net/ip_vs.h =================================================================== --- net-2.6.orig/include/net/ip_vs.h 2007-11-05 11:37:45.000000000 +0900 +++ net-2.6/include/net/ip_vs.h 2007-11-05 11:37:49.000000000 +0900 @@ -520,6 +520,10 @@ struct ip_vs_conn { spinlock_t lock; /* lock for state transition */ volatile __u16 flags; /* status flags */ volatile __u16 state; /* state info */ + volatile __u16 old_state; /* old state, to be used for + * state transition triggerd + * synchronization + */ /* Control members */ struct ip_vs_conn *control; /* Master control connection */ Index: net-2.6/net/ipv4/ipvs/ip_vs_core.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_core.c 2007-11-05 11:37:45.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_core.c 2007-11-05 11:37:49.000000000 +0900 @@ -979,15 +979,23 @@ ip_vs_in(unsigned int hooknum, struct sk ret = NF_ACCEPT; } - /* increase its packet counter and check if it is needed - to be synchronized */ + /* Increase its packet counter and check if it is needed + * to be synchronized + * + * Sync connection if it is about to close to + * encorage the standby servers to update the connections timeout + */ atomic_inc(&cp->in_pkts); if ((ip_vs_sync_state & IP_VS_STATE_MASTER) && - (cp->protocol != IPPROTO_TCP || - cp->state == IP_VS_TCP_S_ESTABLISHED) && - (atomic_read(&cp->in_pkts) % sysctl_ip_vs_sync_threshold[1] - == sysctl_ip_vs_sync_threshold[0])) + (((cp->protocol != IPPROTO_TCP || + cp->state == IP_VS_TCP_S_ESTABLISHED) && + (atomic_read(&cp->in_pkts) % sysctl_ip_vs_sync_threshold[1] + == sysctl_ip_vs_sync_threshold[0])) || + ((cp->protocol == IPPROTO_TCP) && (cp->old_state != cp->state) && + ((cp->state == IP_VS_TCP_S_FIN_WAIT) || + (cp->state == IP_VS_TCP_S_CLOSE))))) ip_vs_sync_conn(cp); + cp->old_state = cp->state; ip_vs_conn_put(cp); return ret; Index: net-2.6/net/ipv4/ipvs/ip_vs_sync.c =================================================================== --- net-2.6.orig/net/ipv4/ipvs/ip_vs_sync.c 2007-11-05 11:37:45.000000000 +0900 +++ net-2.6/net/ipv4/ipvs/ip_vs_sync.c 2007-11-05 11:37:49.000000000 +0900 @@ -344,7 +344,6 @@ static void ip_vs_process_message(const if (!dest) { /* it is an unbound entry created by * synchronization */ - cp->state = ntohs(s->state); cp->flags = flags | IP_VS_CONN_F_HASHED; } else atomic_dec(&dest->refcnt); @@ -359,6 +358,7 @@ static void ip_vs_process_message(const p += SIMPLE_CONN_SIZE; atomic_set(&cp->in_pkts, sysctl_ip_vs_sync_threshold[0]); + cp->state = ntohs(s->state); pp = ip_vs_proto_get(s->protocol); cp->timeout = pp->timeout_table[cp->state]; ip_vs_conn_put(cp); -- -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch 2/2] ipvs: Syncrhonise Closing of Connections 2007-11-05 3:08 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections horms, Simon Horman @ 2007-11-07 10:37 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2007-11-07 10:37 UTC (permalink / raw) To: horms; +Cc: lvs-devel, netdev, rumen, graeme, jmack, ja, wensong From: horms@vergenet.net Date: Mon, 05 Nov 2007 12:08:53 +0900 > From: Rumen G. Bogdanovski <rumen@voicecho.com> > > This patch makes the master daemon to sync the connection when it is about > to close. This makes the connections on the backup to close or timeout > according their state. Before the sync was performed only if the > connection is in ESTABLISHED state which always made the connections to > timeout in the hard coded 3 minutes. However the Andy Gospodarek's patch > ([IPVS]: use proper timeout instead of fixed value) effectively did nothing > more than increasing this to 15 minutes (Established state timeout). So > this patch makes use of proper timeout since it syncs the connections on > status changes to FIN_WAIT (2min timeout) and CLOSE (10sec timeout). > However if the backup misses CLOSE hopefully it did not miss FIN_WAIT. > Otherwise we will just have to wait for the ESTABLISHED state timeout. As > it is without this patch. This way the number of the hanging connections > on the backup is kept to minimum. And very few of them will be left to > timeout with a long timeout. > > This is important if we want to make use of the fix for the real server > overcommit on master/backup fail-over. > > Signed-off-by: Rumen G. Bogdanovski <rumen@voicecho.com> > Signed-off-by: Simon Horman <horms@verge.net.au> Also applied, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-11-07 10:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-11-01 9:28 [patch 0/2] ipvs: avoid overcommit on the standby, take II Simon Horman 2007-11-01 9:28 ` [patch 1/2] ipvs: Bind connections on stanby if the destination exists Simon Horman 2007-11-01 9:28 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections Simon Horman 2007-11-01 23:36 ` Julian Anastasov 2007-11-02 0:53 ` Simon Horman 2007-11-02 9:47 ` [lvs-devel] " Rumen Bogdanovski -- strict thread matches above, loose matches on Subject: below -- 2007-11-05 3:08 [patch 0/2] ipvs: avoid overcommit on the standby, take III horms, Simon Horman 2007-11-05 3:08 ` [patch 2/2] ipvs: Syncrhonise Closing of Connections horms, Simon Horman 2007-11-07 10:37 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).