On Mon, Dec 05, 2005 at 11:12:21AM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 5 Dec 2005, Horms wrote:
>
> > Ok, I've had a bit more of a think about this problem.
> > It seems to me that the real desire is that once a persistance
> > template expires, it shouldn't be used any more. It might
> > have to hang around for a bit because of controlled connections,
> > but it shouldn't be used for new connections.
>
> Very strange, for what app they want this behavior?
I think that you are confused about what I am trying to do,
or perhaps I am confused about what I am trying to do.
The idea of my patch is not to change the persistance sementics.
It is mearly to fix the case where the template expires and
still has controlled connections. The current code will change
the persistance timeout if this occurs. And that persistance template,
whith the new timeout, can continued to be used for new connections
and existing. Even though its timout is not the user-specified value.
Which is just plain weird.
> I think,
> we return to your initial offer and my first answer, we can not
> invalidate template for all users.
>
> 3 seconds is very low value, it can create load.
I don't really mind what value it is, perhaps 10 seconds would
be more appropriate. The current 60 value seems a little high to me.
What do you recommend?
> Better one to call ip_vs_conn_expire_now for ct
> when its n_control reaches 0, eg. in ip_vs_control_del.
I don't really think that is desirable at all. Won't that
cause the persistance template to expire regardless of if
its timeout has expired?
> As for
> IP_VS_CONN_F_EXPIRED I think ip_vs_invalidate_hashed_template is
> more suitable. You can call it depending on some flag for the users
> that want it, for example:
You have stated on several occasions that invalidating
the hash template using that method will break connections.
So surely its a bad idea.
The idea of IP_VS_CONN_F_EXPIRED is just to make sure that the
template will not be used for any new connections. Existing
connections will continue on as normal. Although they will
almost certainly be in the FIN_WAIT state. Although I guess
this semantic is broken if they aren't in that state.
Perhaps a better idea, which I was thinking about this morning,
is to change the timeout value, to say 60*HZ in ip_vs_conn_expire(),
as per the existing code. But always reset it to svc->timeout
when it is used to create a new connection, so if it had timeout
out, but ends up being used again, its timeout will be the
user-defined value when it is re-used.
I think this patch has racy access to ct->timeout,
but it gives you an idea of what I am thinking about.
diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
index 1a0843c..18b5e22 100644
--- a/net/ipv4/ipvs/ip_vs_core.c
+++ b/net/ipv4/ipvs/ip_vs_core.c
@@ -283,8 +283,6 @@ ip_vs_sched_persist(struct ip_vs_service
dest);
if (ct == NULL)
return NULL;
-
- ct->timeout = svc->timeout;
} else {
/* set destination with the found template */
dest = ct->dest;
@@ -337,8 +335,6 @@ ip_vs_sched_persist(struct ip_vs_service
dest);
if (ct == NULL)
return NULL;
-
- ct->timeout = svc->timeout;
} else {
/* set destination with the found template */
dest = ct->dest;
@@ -346,6 +342,7 @@ ip_vs_sched_persist(struct ip_vs_service
dport = ports[1];
}
+ ct->timeout = svc->timeout;
/*
* Create a new connection according to the template
*/
|