* [PATCH 0/2] tcp: fix problems when tcp_fin_timeout is greater than 60
@ 2013-02-12 12:49 Toshiaki Makita
2013-02-12 12:52 ` [PATCH 1/2] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita
0 siblings, 1 reply; 6+ messages in thread
From: Toshiaki Makita @ 2013-02-12 12:49 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Toshiaki Makita
Hi all,
I found a strange behavior of FIN_WAIT2 timer when tcp_fin_timeout is greater
than 60.
When it is between 60 and 120, if the socket is closed, FIN_WAIT2 keepalive
timer of (tcp_fin_timeout - 60) seconds long starts. After it expires, timewait
timer of (tcp_fin_timeout - 60) seconds long starts again. This takes total time
of (tcp_fin_timeout - 60) * 2 seconds to disappear the FIN_WAIT2 socket, which
is shorter than tcp_fin_timeout.
# sysctl -w net.ipv4.tcp_fin_timeout=63
net.ipv4.tcp_fin_timeout = 63
# while :; do netstat -anot | grep 54321 | grep FIN_WAIT2; sleep 1; done
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 keepalive (2.62/0/0)
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 keepalive (1.59/0/0)
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 keepalive (0.56/0/0)
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 timewait (2.61/0/0)
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 timewait (1.59/0/0)
tcp 0 0 127.0.0.1:43034 127.0.0.1:54321 FIN_WAIT2 timewait (0.56/0/0)
When it is greater than 120, although timewait timer appears to start from
(tcp_fin_timeout - 60), it expires after 60 seconds elapse in practice.
# sysctl -w net.ipv4.tcp_fin_timeout=150
net.ipv4.tcp_fin_timeout = 150
# while :; do netstat -anot | grep 54321 | grep FIN_WAIT2; sleep 1; done
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (90.00/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (88.97/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (87.95/0/0)
...
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (2.76/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (1.73/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 keepalive (0.70/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (89.68/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (88.66/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (87.63/0/0)
...
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (32.21/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (31.18/0/0)
tcp 0 0 127.0.0.1:43036 127.0.0.1:54321 FIN_WAIT2 timewait (30.16/0/0)
(no more messages)
This seems to have been so for many years, but I think this behavior is not
desirable because it is confusing and does not match the documents.
Besides, it is also confusing that netstat shows keepalive timer first, and then
shows timewait timer, even though it is necessary to limit resources for the
orphaned socket.
I made patches that fix these problems.
Toshiaki Makita
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] tcp: fix too short FIN_WAIT2 time out
2013-02-12 12:49 [PATCH 0/2] tcp: fix problems when tcp_fin_timeout is greater than 60 Toshiaki Makita
@ 2013-02-12 12:52 ` Toshiaki Makita
2013-02-12 12:54 ` [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita
0 siblings, 1 reply; 6+ messages in thread
From: Toshiaki Makita @ 2013-02-12 12:52 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Toshiaki Makita
When tcp_fin_timeout is between 60 and 120, FIN_WAIT2 socket
disappears in (tcp_fin_timeout - 60) * 2 sec, which is shorter
than expected.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
net/ipv4/tcp_timer.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index b78aac3..c20e474 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -576,12 +576,8 @@ static void tcp_keepalive_timer (unsigned long data)
if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
if (tp->linger2 >= 0) {
- const int tmo = tcp_fin_time(sk) - TCP_TIMEWAIT_LEN;
-
- if (tmo > 0) {
- tcp_time_wait(sk, TCP_FIN_WAIT2, tmo);
- goto out;
- }
+ tcp_time_wait(sk, TCP_FIN_WAIT2, TCP_TIMEWAIT_LEN);
+ goto out;
}
tcp_send_active_reset(sk, GFP_ATOMIC);
goto death;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp
2013-02-12 12:52 ` [PATCH 1/2] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita
@ 2013-02-12 12:54 ` Toshiaki Makita
2013-02-12 14:57 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Toshiaki Makita @ 2013-02-12 12:54 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Toshiaki Makita
When tcp_fin_timeout is greater than 60, /proc/net/tcp shows
FIN_WAIT2 keepalive timer as expires in (tcp_fin_timeout - 60)
sec. This is confusing because the timer is not for keepalive
and the socket needs another timewait timer to disappear.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
net/ipv4/tcp_ipv4.c | 23 +++++++++++++++--------
net/ipv6/tcp_ipv6.c | 23 +++++++++++++++--------
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 629937d..32bde0e 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2656,17 +2656,24 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
int rx_queue;
if (icsk->icsk_pending == ICSK_TIME_RETRANS) {
- timer_active = 1;
- timer_expires = icsk->icsk_timeout;
+ timer_active = 1;
+ timer_expires = icsk->icsk_timeout;
} else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
- timer_active = 4;
- timer_expires = icsk->icsk_timeout;
+ timer_active = 4;
+ timer_expires = icsk->icsk_timeout;
} else if (timer_pending(&sk->sk_timer)) {
- timer_active = 2;
- timer_expires = sk->sk_timer.expires;
+ if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
+ timer_active = 3;
+ timer_expires = sk->sk_timer.expires +
+ (tp->linger2 >= 0 ?
+ TCP_TIMEWAIT_LEN : 0);
+ } else {
+ timer_active = 2;
+ timer_expires = sk->sk_timer.expires;
+ }
} else {
- timer_active = 0;
- timer_expires = jiffies;
+ timer_active = 0;
+ timer_expires = jiffies;
}
if (sk->sk_state == TCP_LISTEN)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 93825dd..3af4f9d 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1795,17 +1795,24 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
srcp = ntohs(inet->inet_sport);
if (icsk->icsk_pending == ICSK_TIME_RETRANS) {
- timer_active = 1;
- timer_expires = icsk->icsk_timeout;
+ timer_active = 1;
+ timer_expires = icsk->icsk_timeout;
} else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
- timer_active = 4;
- timer_expires = icsk->icsk_timeout;
+ timer_active = 4;
+ timer_expires = icsk->icsk_timeout;
} else if (timer_pending(&sp->sk_timer)) {
- timer_active = 2;
- timer_expires = sp->sk_timer.expires;
+ if (sp->sk_state == TCP_FIN_WAIT2 && sock_flag(sp, SOCK_DEAD)) {
+ timer_active = 3;
+ timer_expires = sp->sk_timer.expires +
+ (tp->linger2 >= 0 ?
+ TCP_TIMEWAIT_LEN : 0);
+ } else {
+ timer_active = 2;
+ timer_expires = sp->sk_timer.expires;
+ }
} else {
- timer_active = 0;
- timer_expires = jiffies;
+ timer_active = 0;
+ timer_expires = jiffies;
}
seq_printf(seq,
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp
2013-02-12 12:54 ` [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita
@ 2013-02-12 14:57 ` Eric Dumazet
2013-02-12 15:13 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-02-12 14:57 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: David S. Miller, netdev
On Tue, 2013-02-12 at 21:54 +0900, Toshiaki Makita wrote:
> When tcp_fin_timeout is greater than 60, /proc/net/tcp shows
> FIN_WAIT2 keepalive timer as expires in (tcp_fin_timeout - 60)
> sec. This is confusing because the timer is not for keepalive
> and the socket needs another timewait timer to disappear.
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> net/ipv4/tcp_ipv4.c | 23 +++++++++++++++--------
> net/ipv6/tcp_ipv6.c | 23 +++++++++++++++--------
> 2 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 629937d..32bde0e 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -2656,17 +2656,24 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
> int rx_queue;
>
> if (icsk->icsk_pending == ICSK_TIME_RETRANS) {
> - timer_active = 1;
> - timer_expires = icsk->icsk_timeout;
> + timer_active = 1;
> + timer_expires = icsk->icsk_timeout;
> } else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
> - timer_active = 4;
> - timer_expires = icsk->icsk_timeout;
> + timer_active = 4;
> + timer_expires = icsk->icsk_timeout;
> } else if (timer_pending(&sk->sk_timer)) {
> - timer_active = 2;
> - timer_expires = sk->sk_timer.expires;
> + if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
> + timer_active = 3;
> + timer_expires = sk->sk_timer.expires +
> + (tp->linger2 >= 0 ?
> + TCP_TIMEWAIT_LEN : 0);
> + } else {
> + timer_active = 2;
> + timer_expires = sk->sk_timer.expires;
> + }
> } else {
> - timer_active = 0;
> - timer_expires = jiffies;
> + timer_active = 0;
> + timer_expires = jiffies;
> }
>
I find this patch confusing :
1) Please don't change the indentation for a bug fix
2) You add a new 'active=3' field, that some user space
reading /proc/net/tcp wont expect.
So the changelog is not matching the changes.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp
2013-02-12 14:57 ` Eric Dumazet
@ 2013-02-12 15:13 ` Eric Dumazet
2013-02-15 8:26 ` Toshiaki Makita
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2013-02-12 15:13 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: David S. Miller, netdev
On Tue, 2013-02-12 at 06:57 -0800, Eric Dumazet wrote:
> I find this patch confusing :
>
> 1) Please don't change the indentation for a bug fix
>
> 2) You add a new 'active=3' field, that some user space
> reading /proc/net/tcp wont expect.
>
> So the changelog is not matching the changes.
Also, net/ipv4/inet_diag.c was not changed
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp
2013-02-12 15:13 ` Eric Dumazet
@ 2013-02-15 8:26 ` Toshiaki Makita
0 siblings, 0 replies; 6+ messages in thread
From: Toshiaki Makita @ 2013-02-15 8:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
On Tue, 2013-02-12 at 07:13 -0800, Eric Dumazet wrote:
> On Tue, 2013-02-12 at 06:57 -0800, Eric Dumazet wrote:
>
> > I find this patch confusing :
> >
> > 1) Please don't change the indentation for a bug fix
OK, I will separate changes of indentation to another patch.
> >
> > 2) You add a new 'active=3' field, that some user space
> > reading /proc/net/tcp wont expect.
I think, at least, it's harmless and beneficial for netstat.
Even without this change, a keepalive timer of an orphaned
FIN_WAIT2 socket will eventually turn to a timewait timer,
and by default, or when tcp_fin_timeout is 60, there will be
only timewait timers for orphaned FIN_WAIT2.
If the socket is not closed and SO_KEEPALIVE is set,
a keepalive timer of FIN_WAIT2 will also be shown in
/proc/net/tcp, whose state isn't equal to above one.
I don't know any application that will be damaged with this
change, and I think the possibility that this change affects
a bad influence to userspace is low.
Please advise.
> >
> > So the changelog is not matching the changes.
>
> Also, net/ipv4/inet_diag.c was not changed
>
thank you for pointing out my mistake.
I will correct it, too.
Toshiaki Makita
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-15 8:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-12 12:49 [PATCH 0/2] tcp: fix problems when tcp_fin_timeout is greater than 60 Toshiaki Makita
2013-02-12 12:52 ` [PATCH 1/2] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita
2013-02-12 12:54 ` [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita
2013-02-12 14:57 ` Eric Dumazet
2013-02-12 15:13 ` Eric Dumazet
2013-02-15 8:26 ` Toshiaki Makita
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.