|
On Sat, Nov 05, 2005 at 01:56:43PM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Fri, 4 Nov 2005, Horms wrote:
>
> > I notices that if persistance timeout is less than 2 min
> > (the FIN_WAIT timeout) then it will expire before its
> > controlled connections. This causes ip_vs_conn_expire to
> > set the template's timeout to 2min. Which caues somewhat unexpected
> > results if the persistance timeout, like say 10s.
> >
> > The patch below is a completely untested first cut at fix,
> > it tries to resolve the problem by invalidating expired
> > templates. So while they will stick around in the table for
> > a bit longer, they won't take effect any more.
>
> This patch looks ok but may be it is better this
> timeout restriction to be controlled with sysctl var. This
> new behaviour is dangerous for sessions that include many
> connections from same client. You broke the affinity without
> considering the other connections, may be the user wants all
> connections to go to same RS, even if the cost is extended
> template lifetime.
I'm a bit worried about that too. Although the template has to have
actually timed out for this to occur. In any case, perhaps a patch that
doesn't change the timeout would be better. Something like the
following, though I'm not sure why cp->timeout is set at the top
of ip_vs_conn_expire(), it would seem cleaner if it was set below
expire_later. Actually, I'm not really sure why the timeout is altered
at all.
diff --git a/net/ipv4/ipvs/ip_vs_conn.c b/net/ipv4/ipvs/ip_vs_conn.c
index 015c819..98d0712 100644
--- a/net/ipv4/ipvs/ip_vs_conn.c
+++ b/net/ipv4/ipvs/ip_vs_conn.c
@@ -1220,7 +1220,9 @@ static inline void ip_vs_timeout_detach(
static void ip_vs_conn_expire(unsigned long data)
{
struct ip_vs_conn *cp = (struct ip_vs_conn *)data;
+ unsigned long old_timeout;
+ old_timeout = cp->timeout;
if (cp->timeout_table)
cp->timeout = cp->timeout_table->timeout[IP_VS_S_TIME_WAIT];
else
@@ -1234,8 +1236,10 @@ static void ip_vs_conn_expire(unsigned l
/*
* do I control anybody?
*/
- if (atomic_read(&cp->n_control))
+ if (atomic_read(&cp->n_control)) {
+ cp->timeout = old_timeout;
goto expire_later;
+ }
/*
* unhash it if it is hashed in the conn table
> What is strange to me is that timeout of 10 seconds is
> used for TCP connections. What kind of setups use such low value?
Yes, I would like to know that too. Its a value that a customer is
using. I intend to find out just what they ware trying to do, I suspect
they are confused. The but this shows up is a bug, but its a
corner case.
> So, there can be a sysctl var "extra_persistence",
> "persistence_extent" or another more suitable name. Its default
> value can be "60" seconds, as now. The dangerous value of 0 will
> invalidate the template as you propose (but still use some timeout
> to check for removal). Any other value will be used as before in
> ip_vs_conn_expire, just for templates.
I'm pretty reluctant to add yet another sysctl value, people
are already very confused about how persistance works.
--
Horms
|
