|
Hello,
On Tue, 6 Dec 2005, Horms wrote:
> Yes, I agree with eveyrhing you say above. However, the
> case that we are looking at is that the persistance timeout has
> actually expired. But for some reason there are still controlled
> connections. I'm happy to just leave the timeout as is, because this
> case only really occurs if the persistance timeout is really really
> short, shorter than the FIN_WAIT timout.
>
> In any case, I agree with Ratz that FIN_WAIT should probably be
> reduced. Which would aleviate the problem.
The FIN WAIT and TIME WAIT timeouts help large uploads and
data retransmits. I hope Ratz can change it with ipvsadm if we
implement per-state timeout. ip_vs_random_dropentry can know about
these state too.
> I believe that the idea of reducing the timeout is primarily
> to reduce memory usage. This makes sense for non-templates.
The only non-template that can restart timer is the FTP
control connection. For it 10 seconds can be too low causing next
command/transfer to fail. So, i recommend to assign 10 secs only to
templates. 60 was almost fine for FTP.
> Although 60*HZ is probably a bit high for them.
>
> However, I think that reinstating the timeout, as below, might meet the
> goals of reducing memory preasure, by giving expired entries a low
> timeout, and honouring the user-defined timeout of templates, even if
> they expired between use. I'd like to work with that idea a little more.
For memory presure there is drop_entry, we can extend it
with other states and to modify it to fire when memory for
connections goes after some user-defined threshold causing aggresive
expiration after seconds or tens of seconds depending on presure.
States like FIN WAIT and TIME WAIT can lose first.
> > This is a good idea, in this case setting 10 seconds in
> > ip_vs_conn_expire for templates is not so bad. Also, we should
> > take into account that the ip_vs_control_* mechanism is used from
> > FTP, may be just to extend the TCP EST period for the control conn
> > while there are DATA transfers.
>
> Yes, I was wondering what to do about that. I took a quick look into
> that, and it seems to me that, if the control connection needs
> to handle a new data connection, then it must be TCP EST,
> else it wouldn't recieve the PASV command. And in the case were it
> is just handling existing data connections, this is really
> the same as a persistance template who has unexpired children.
> So I don't think any extra handling of ip_vs_control_* is required.
> Am I mistaken?
Nothing is needed, but we should be careful not to invalidate it.
> Also, do you think the access to ct->timeout below is racy?
Not worse than before :)
> Yesterdays' patch + changing timeout to 10*HZ instead of 60*HZ,
> which I am in favour of.
OK. It works for short connections but not for long. One
can require the persistence timeout to work after the last connection
is expired, not from its start. It can be solved only if we can
control this 10-second assignment. There can be 2 options: set to
10secs or preserve timeout. But this patch is a good start, we can
modify it later.
> diff --git a/net/ipv4/ipvs/ip_vs_conn.c b/net/ipv4/ipvs/ip_vs_conn.c
> index f828fa2..19bd6a9 100644
> --- a/net/ipv4/ipvs/ip_vs_conn.c
> +++ b/net/ipv4/ipvs/ip_vs_conn.c
> @@ -525,7 +525,7 @@ static void ip_vs_conn_expire(unsigned l
> {
> struct ip_vs_conn *cp = (struct ip_vs_conn *)data;
>
> - cp->timeout = 60*HZ;
> + cp->timeout = 10*HZ;
>
> /*
> * hey, I'm using it
> diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
> index 1a0843c..7e011e3 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,11 @@ ip_vs_sched_persist(struct ip_vs_service
> dport = ports[1];
> }
>
> + /*
> + * Make sure template has user-defined timeout
> + */
> + ct->timeout = svc->timeout;
> +
> /*
> * Create a new connection according to the template
> */
Regards
--
Julian Anastasov <ja@xxxxxx>
|
