Re: [PATCH] ipvs: Make the synchronization interval controllable

2008-02-08 Thread Andi Kleen
Sven Wegener [EMAIL PROTECTED] writes:

 The default synchronization interval of 1000 milliseconds is too high for a
 heavily loaded director. Collecting the connection information from one second
 and then sending it out in a burst will overflow the socket buffer and lead to
 synchronization information being dropped. Make the interval controllable by a
 sysctl variable so that users can tune it.

It would be better if the defaults just worked under all circumstances.
So why not just lower the default?

Or the code could detect overflowing socket buffers and lower the 
value dynamically.

-Andi
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ipvs: Make the synchronization interval controllable

2008-02-08 Thread Krzysztof Oledzki



On Fri, 8 Feb 2008, Andi Kleen wrote:


Sven Wegener [EMAIL PROTECTED] writes:


The default synchronization interval of 1000 milliseconds is too high for a
heavily loaded director. Collecting the connection information from one second
and then sending it out in a burst will overflow the socket buffer and lead to
synchronization information being dropped. Make the interval controllable by a
sysctl variable so that users can tune it.


It would be better if the defaults just worked under all circumstances.
So why not just lower the default?

Or the code could detect overflowing socket buffers and lower the
value dynamically.


We can also start sending when amount of data reaches defined level.

Best regards,

Krzysztof Olędzki

[PATCH] ipvs: Make the synchronization interval controllable

2008-02-06 Thread Sven Wegener
The default synchronization interval of 1000 milliseconds is too high for a
heavily loaded director. Collecting the connection information from one second
and then sending it out in a burst will overflow the socket buffer and lead to
synchronization information being dropped. Make the interval controllable by a
sysctl variable so that users can tune it.

Signed-off-by: Sven Wegener [EMAIL PROTECTED]
---
 Documentation/networking/ipvs-sysctl.txt |9 +
 net/ipv4/ipvs/ip_vs_ctl.c|9 -
 net/ipv4/ipvs/ip_vs_sync.c   |6 --
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ipvs-sysctl.txt 
b/Documentation/networking/ipvs-sysctl.txt
index 4ccdbca..1389e2f 100644
--- a/Documentation/networking/ipvs-sysctl.txt
+++ b/Documentation/networking/ipvs-sysctl.txt
@@ -141,3 +141,12 @@ sync_threshold - INTEGER
 synchronized, every time the number of its incoming packets
 modulus 50 equals the threshold. The range of the threshold is
 from 0 to 49.
+
+sync_interval - INTEGER
+   default 1000
+
+   The information from synchronization is buffered and sent out at
+   regular intervals by a kernel thread. The interval (in ms) is
+   controlled by this value. The default is too high for a heavily loaded
+   director. If you get a lot of ip_vs_send_async error messages from
+   your kernel, then you should lower this value.
diff --git a/net/ipv4/ipvs/ip_vs_ctl.c b/net/ipv4/ipvs/ip_vs_ctl.c
index 94c5767..2781505 100644
--- a/net/ipv4/ipvs/ip_vs_ctl.c
+++ b/net/ipv4/ipvs/ip_vs_ctl.c
@@ -81,7 +81,7 @@ int sysctl_ip_vs_expire_nodest_conn = 0;
 int sysctl_ip_vs_expire_quiescent_template = 0;
 int sysctl_ip_vs_sync_threshold[2] = { 3, 50 };
 int sysctl_ip_vs_nat_icmp_send = 0;
-
+extern int sysctl_ip_vs_sync_interval;
 
 #ifdef CONFIG_IP_VS_DEBUG
 static int sysctl_ip_vs_debug_level = 0;
@@ -1582,6 +1582,13 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_do_sync_threshold,
},
{
+   .procname   = sync_interval,
+   .data   = sysctl_ip_vs_sync_interval,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec,
+   },
+   {
.procname   = nat_icmp_send,
.data   = sysctl_ip_vs_nat_icmp_send,
.maxlen = sizeof(int),
diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
index 948378d..9b57ad3 100644
--- a/net/ipv4/ipvs/ip_vs_sync.c
+++ b/net/ipv4/ipvs/ip_vs_sync.c
@@ -143,6 +143,8 @@ char ip_vs_backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
 /* multicast addr */
 static struct sockaddr_in mcast_addr;
 
+/* milliseconds between synchronization runs */
+int sysctl_ip_vs_sync_interval = 1000;
 
 static inline void sb_queue_tail(struct ip_vs_sync_buff *sb)
 {
@@ -701,7 +703,7 @@ static void sync_master_loop(void)
if (stop_master_sync)
break;
 
-   msleep_interruptible(1000);
+   msleep_interruptible(sysctl_ip_vs_sync_interval);
}
 
/* clean up the sync_buff queue */
@@ -758,7 +760,7 @@ static void sync_backup_loop(void)
if (stop_backup_sync)
break;
 
-   msleep_interruptible(1000);
+   msleep_interruptible(sysctl_ip_vs_sync_interval);
}
 
/* release the sending multicast socket */
-- 
1.5.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ipvs: Make the synchronization interval controllable

2008-02-06 Thread David Rientjes
On Wed, 6 Feb 2008, Sven Wegener wrote:

 diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
 index 948378d..9b57ad3 100644
 --- a/net/ipv4/ipvs/ip_vs_sync.c
 +++ b/net/ipv4/ipvs/ip_vs_sync.c
 @@ -143,6 +143,8 @@ char ip_vs_backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
  /* multicast addr */
  static struct sockaddr_in mcast_addr;
  
 +/* milliseconds between synchronization runs */
 +int sysctl_ip_vs_sync_interval = 1000;
  
  static inline void sb_queue_tail(struct ip_vs_sync_buff *sb)
  {

How useful is a negative ip_vs_sync_interval?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ipvs: Make the synchronization interval controllable

2008-02-06 Thread Sven Wegener

On Wed, 6 Feb 2008, David Rientjes wrote:


On Wed, 6 Feb 2008, Sven Wegener wrote:


diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
index 948378d..9b57ad3 100644
--- a/net/ipv4/ipvs/ip_vs_sync.c
+++ b/net/ipv4/ipvs/ip_vs_sync.c
@@ -143,6 +143,8 @@ char ip_vs_backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
 /* multicast addr */
 static struct sockaddr_in mcast_addr;

+/* milliseconds between synchronization runs */
+int sysctl_ip_vs_sync_interval = 1000;

 static inline void sb_queue_tail(struct ip_vs_sync_buff *sb)
 {


How useful is a negative ip_vs_sync_interval?


Negative values will be converted to MAX_JIFFY_OFFSET by msecs_to_jiffies 
and result in a very long interval. A too long interval will be a good way 
to get your system OOM. We could use an unsigned int or even restrict the 
value with proc_dointvec_minmax. I'd prefer the latter, that's what I 
already had in my mind and it also protects from unintentionally choosing 
a too long interval.


Sven
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ipvs: Make the synchronization interval controllable

2008-02-06 Thread Sven Wegener
The default synchronization interval of 1000 milliseconds is too high for a
heavily loaded director. Collecting the connection information from one second
and then sending it out in a burst will overflow the socket buffer and lead to
synchronization information being dropped. Make the interval controllable by a
sysctl variable so that users can tune it. We enforce a lower limit of 0 and an
upper limit of 2000 ms on the interval. A too large interval can make the
synchronization buffer consume too much memory and will also delay the exit of
the kernel threads.

Signed-off-by: Sven Wegener [EMAIL PROTECTED]
---

Changes from the last version include the addition of the range enforcement.
Also place the definitions of the variables where all other ipvs sysctl
variables are.

Documentation/networking/ipvs-sysctl.txt |   10 ++
 include/net/ip_vs.h  |1 +
 net/ipv4/ipvs/ip_vs_ctl.c|   12 
 net/ipv4/ipvs/ip_vs_sync.c   |4 ++--
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/ipvs-sysctl.txt 
b/Documentation/networking/ipvs-sysctl.txt
index 4ccdbca..bb4eb9a 100644
--- a/Documentation/networking/ipvs-sysctl.txt
+++ b/Documentation/networking/ipvs-sysctl.txt
@@ -141,3 +141,13 @@ sync_threshold - INTEGER
 synchronized, every time the number of its incoming packets
 modulus 50 equals the threshold. The range of the threshold is
 from 0 to 49.
+
+sync_interval - INTEGER
+   default 1000
+
+   The information from synchronization is buffered and sent out at a
+   regular interval by a kernel thread. The interval (in ms) is
+   controlled by this value. The default is too high for a heavily loaded
+   director. If you get a lot of ip_vs_send_async error messages from
+   your kernel, then you should lower this value. The value of the
+   interval can be chosen from the range from 0 to 2000.
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 56f3c94..9c4498b 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -854,6 +854,7 @@ extern int sysctl_ip_vs_cache_bypass;
 extern int sysctl_ip_vs_expire_nodest_conn;
 extern int sysctl_ip_vs_expire_quiescent_template;
 extern int sysctl_ip_vs_sync_threshold[2];
+extern int sysctl_ip_vs_sync_interval;
 extern int sysctl_ip_vs_nat_icmp_send;
 extern struct ip_vs_stats ip_vs_stats;
 extern struct ctl_path net_vs_ctl_path[];
diff --git a/net/ipv4/ipvs/ip_vs_ctl.c b/net/ipv4/ipvs/ip_vs_ctl.c
index 94c5767..c6322f7 100644
--- a/net/ipv4/ipvs/ip_vs_ctl.c
+++ b/net/ipv4/ipvs/ip_vs_ctl.c
@@ -80,8 +80,11 @@ int sysctl_ip_vs_cache_bypass = 0;
 int sysctl_ip_vs_expire_nodest_conn = 0;
 int sysctl_ip_vs_expire_quiescent_template = 0;
 int sysctl_ip_vs_sync_threshold[2] = { 3, 50 };
+int sysctl_ip_vs_sync_interval = 1000;
 int sysctl_ip_vs_nat_icmp_send = 0;
 
+static int ip_vs_sync_interval_min = 0;
+static int ip_vs_sync_interval_max = 2000;
 
 #ifdef CONFIG_IP_VS_DEBUG
 static int sysctl_ip_vs_debug_level = 0;
@@ -1582,6 +1585,15 @@ static struct ctl_table vs_vars[] = {
.proc_handler   = proc_do_sync_threshold,
},
{
+   .procname   = sync_interval,
+   .data   = sysctl_ip_vs_sync_interval,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec_minmax,
+   .extra1 = ip_vs_sync_interval_min,
+   .extra2 = ip_vs_sync_interval_max,
+   },
+   {
.procname   = nat_icmp_send,
.data   = sysctl_ip_vs_nat_icmp_send,
.maxlen = sizeof(int),
diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
index 948378d..10ab1b7 100644
--- a/net/ipv4/ipvs/ip_vs_sync.c
+++ b/net/ipv4/ipvs/ip_vs_sync.c
@@ -701,7 +701,7 @@ static void sync_master_loop(void)
if (stop_master_sync)
break;
 
-   msleep_interruptible(1000);
+   msleep_interruptible(sysctl_ip_vs_sync_interval);
}
 
/* clean up the sync_buff queue */
@@ -758,7 +758,7 @@ static void sync_backup_loop(void)
if (stop_backup_sync)
break;
 
-   msleep_interruptible(1000);
+   msleep_interruptible(sysctl_ip_vs_sync_interval);
}
 
/* release the sending multicast socket */
-- 
1.5.4

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ipvs: Make the synchronization interval controllable

2008-02-06 Thread Simon Horman
On Thu, Feb 07, 2008 at 12:57:43AM +0100, Sven Wegener wrote:
 The default synchronization interval of 1000 milliseconds is too high for a
 heavily loaded director. Collecting the connection information from one second
 and then sending it out in a burst will overflow the socket buffer and lead to
 synchronization information being dropped. Make the interval controllable by a
 sysctl variable so that users can tune it. We enforce a lower limit of 0 and 
 an
 upper limit of 2000 ms on the interval. A too large interval can make the
 synchronization buffer consume too much memory and will also delay the exit of
 the kernel threads.
 
 Signed-off-by: Sven Wegener [EMAIL PROTECTED]

Hi Sven,

I have no problems with the spirit of this patch, and the range checking
seems good to me.  Though I wonder if 2000 might not be an exessively
low maximum.  I can't think of a usage case off-hand, but I'm not sure I
understand why a user shouldn't be able to set it to much (rediculously)
higher values.

In any case, thats not a big deal.

Acked-by: Simon Horman [EMAIL PROTECTED]

 ---
 
 Changes from the last version include the addition of the range enforcement.
 Also place the definitions of the variables where all other ipvs sysctl
 variables are.

-- 
Horms

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html