* [PATCH v2 0/4] tcp: fix problems when tcp_fin_timeout is greater than 60 @ 2013-03-18 12:36 Toshiaki Makita 2013-03-18 12:39 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita 0 siblings, 1 reply; 7+ messages in thread From: Toshiaki Makita @ 2013-03-18 12:36 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, netdev; +Cc: Toshiaki Makita This is resubmission of the patch set that fixes problems when tcp_fin_timeout is greater than 60. - fix too short FIN_WAIT2 timer when tcp_fin_timeout is between 60 and 120. - fix FIN_WAIT2 timer expression in /proc/net/tcp and inet_diag when tcp_fin_timeout is greater than 60. v2: - Separated changes of indentation to another patch. - Revised the comment of the patch modifying the expression of /proc/net/tcp to explain the reason for change. - Modified inet_diag. Toshiaki Makita (4): tcp: fix too short FIN_WAIT2 time out tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp tcp: fix the indentation in timer calculation of /proc/net/tcp inet_diag: fix FIN_WAIT2 timer expression in inet_diag include/linux/inet_diag.h | 2 ++ net/dccp/diag.c | 15 +++++++++++++++ net/ipv4/inet_diag.c | 20 -------------------- net/ipv4/tcp_diag.c | 27 +++++++++++++++++++++++++++ net/ipv4/tcp_ipv4.c | 23 +++++++++++++++-------- net/ipv4/tcp_timer.c | 8 ++------ net/ipv6/tcp_ipv6.c | 23 +++++++++++++++-------- 7 files changed, 76 insertions(+), 42 deletions(-) -- 1.7.10.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out 2013-03-18 12:36 [PATCH v2 0/4] tcp: fix problems when tcp_fin_timeout is greater than 60 Toshiaki Makita @ 2013-03-18 12:39 ` Toshiaki Makita 2013-03-18 12:43 ` [PATCH v2 2/4] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita 2013-03-19 13:32 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out David Miller 0 siblings, 2 replies; 7+ messages in thread From: Toshiaki Makita @ 2013-03-18 12:39 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, netdev; +Cc: Toshiaki Makita When tcp_fin_timeout is between 60 and 120, a 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 file 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.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp 2013-03-18 12:39 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita @ 2013-03-18 12:43 ` Toshiaki Makita 2013-03-18 12:46 ` [PATCH v2 3/4] tcp: fix the indentation in timer calculation of /proc/net/tcp Toshiaki Makita 2013-03-19 13:32 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out David Miller 1 sibling, 1 reply; 7+ messages in thread From: Toshiaki Makita @ 2013-03-18 12:43 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, netdev; +Cc: Toshiaki Makita When tcp_fin_timeout is greater than 60 and a socket is half-closed, /proc/net/tcp shows a FIN_WAIT2 keepalive timer. This is confusing because keepalive probes will never be sent in this case. Keepalive probes will really be sent with a FIN_WAIT2 keepalive timer shown when a socket is not closed but shutdown and SO_KEEPALIVE is set. Even without this change, a keepalive timer for 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 no keepalive timer for orphaned FIN_WAIT2 sockets. Therefore, this change will hardly affect a bad influence to userspace. This patch changes the expression to show only timewait timer for orphaned FIN_WAIT2 sockets. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- net/ipv4/tcp_ipv4.c | 11 +++++++++-- net/ipv6/tcp_ipv6.c | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 4a8ec45..86a0685 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -2666,8 +2666,15 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len) 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; diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 9b64600..b74b133 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1811,8 +1811,15 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i) 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; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/4] tcp: fix the indentation in timer calculation of /proc/net/tcp 2013-03-18 12:43 ` [PATCH v2 2/4] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita @ 2013-03-18 12:46 ` Toshiaki Makita 2013-03-18 12:50 ` [PATCH v2 4/4] inet_diag: fix FIN_WAIT2 timer expression in inet_diag Toshiaki Makita 0 siblings, 1 reply; 7+ messages in thread From: Toshiaki Makita @ 2013-03-18 12:46 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, netdev; +Cc: Toshiaki Makita Fix the indentation in get_tcp4_sock and get_tcp6_sock. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- net/ipv4/tcp_ipv4.c | 12 ++++++------ net/ipv6/tcp_ipv6.c | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 86a0685..6f0ae53 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -2660,11 +2660,11 @@ 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)) { if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) { timer_active = 3; @@ -2676,8 +2676,8 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len) 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 b74b133..78cdb81 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1805,11 +1805,11 @@ 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)) { if (sp->sk_state == TCP_FIN_WAIT2 && sock_flag(sp, SOCK_DEAD)) { timer_active = 3; @@ -1821,8 +1821,8 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i) timer_expires = sp->sk_timer.expires; } } else { - timer_active = 0; - timer_expires = jiffies; + timer_active = 0; + timer_expires = jiffies; } seq_printf(seq, -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] inet_diag: fix FIN_WAIT2 timer expression in inet_diag 2013-03-18 12:46 ` [PATCH v2 3/4] tcp: fix the indentation in timer calculation of /proc/net/tcp Toshiaki Makita @ 2013-03-18 12:50 ` Toshiaki Makita 0 siblings, 0 replies; 7+ messages in thread From: Toshiaki Makita @ 2013-03-18 12:50 UTC (permalink / raw) To: David S. Miller, Eric Dumazet, netdev; +Cc: Toshiaki Makita This is a change similar to the patch for /proc/net/tcp. FIN_WAIT2 timers for orphaned sockets are also shown in inet_diag. There will be no keepalive timer for orphand FIN_WAIT2 sockets by this change. Besides, timer calculation of an inet_connection_sock is splitted into two pieces because few common codes remain between timer calculation of a tcp_sock and a dccp_sock: Special handling with FIN_WAIT2 is needed for a tcp_sock, and consideration of ICSK_TIME_PROBE0 is unnecessary for a dccp_sock. Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- include/linux/inet_diag.h | 2 ++ net/dccp/diag.c | 15 +++++++++++++++ net/ipv4/inet_diag.c | 20 -------------------- net/ipv4/tcp_diag.c | 27 +++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h index 46da024..f7f359e 100644 --- a/include/linux/inet_diag.h +++ b/include/linux/inet_diag.h @@ -1,6 +1,7 @@ #ifndef _INET_DIAG_H_ #define _INET_DIAG_H_ 1 +#include <linux/kernel.h> #include <uapi/linux/inet_diag.h> struct sock; @@ -27,6 +28,7 @@ struct inet_diag_handler { }; struct inet_connection_sock; +#define EXPIRES_IN_MS(tmo) DIV_ROUND_UP((tmo - jiffies) * 1000, HZ) int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk, struct sk_buff *skb, struct inet_diag_req_v2 *req, struct user_namespace *user_ns, diff --git a/net/dccp/diag.c b/net/dccp/diag.c index 028fc43..b979a5b 100644 --- a/net/dccp/diag.c +++ b/net/dccp/diag.c @@ -42,6 +42,21 @@ static void dccp_get_info(struct sock *sk, struct tcp_info *info) static void dccp_diag_get_info(struct sock *sk, struct inet_diag_msg *r, void *_info) { + const struct inet_connection_sock *icsk = inet_csk(sk); + + if (icsk->icsk_pending == ICSK_TIME_RETRANS) { + r->idiag_timer = 1; + r->idiag_retrans = icsk->icsk_retransmits; + r->idiag_expires = EXPIRES_IN_MS(icsk->icsk_timeout); + } else if (timer_pending(&sk->sk_timer)) { + r->idiag_timer = 2; + r->idiag_retrans = icsk->icsk_probes_out; + r->idiag_expires = EXPIRES_IN_MS(sk->sk_timer.expires); + } else { + r->idiag_timer = 0; + r->idiag_expires = 0; + } + r->idiag_rqueue = r->idiag_wqueue = 0; if (_info != NULL) diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c index 7afa2c3..32c22a7 100644 --- a/net/ipv4/inet_diag.c +++ b/net/ipv4/inet_diag.c @@ -156,26 +156,6 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk, goto out; } -#define EXPIRES_IN_MS(tmo) DIV_ROUND_UP((tmo - jiffies) * 1000, HZ) - - if (icsk->icsk_pending == ICSK_TIME_RETRANS) { - r->idiag_timer = 1; - r->idiag_retrans = icsk->icsk_retransmits; - r->idiag_expires = EXPIRES_IN_MS(icsk->icsk_timeout); - } else if (icsk->icsk_pending == ICSK_TIME_PROBE0) { - r->idiag_timer = 4; - r->idiag_retrans = icsk->icsk_probes_out; - r->idiag_expires = EXPIRES_IN_MS(icsk->icsk_timeout); - } else if (timer_pending(&sk->sk_timer)) { - r->idiag_timer = 2; - r->idiag_retrans = icsk->icsk_probes_out; - r->idiag_expires = EXPIRES_IN_MS(sk->sk_timer.expires); - } else { - r->idiag_timer = 0; - r->idiag_expires = 0; - } -#undef EXPIRES_IN_MS - if (ext & (1 << (INET_DIAG_INFO - 1))) { attr = nla_reserve(skb, INET_DIAG_INFO, sizeof(struct tcp_info)); diff --git a/net/ipv4/tcp_diag.c b/net/ipv4/tcp_diag.c index ed3f2ad..120a4250 100644 --- a/net/ipv4/tcp_diag.c +++ b/net/ipv4/tcp_diag.c @@ -20,9 +20,36 @@ static void tcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r, void *_info) { + const struct inet_connection_sock *icsk = inet_csk(sk); const struct tcp_sock *tp = tcp_sk(sk); struct tcp_info *info = _info; + if (icsk->icsk_pending == ICSK_TIME_RETRANS) { + r->idiag_timer = 1; + r->idiag_retrans = icsk->icsk_retransmits; + r->idiag_expires = EXPIRES_IN_MS(icsk->icsk_timeout); + } else if (icsk->icsk_pending == ICSK_TIME_PROBE0) { + r->idiag_timer = 4; + r->idiag_retrans = icsk->icsk_probes_out; + r->idiag_expires = EXPIRES_IN_MS(icsk->icsk_timeout); + } else if (timer_pending(&sk->sk_timer)) { + if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) { + const unsigned long timer_expires = + sk->sk_timer.expires + + (tp->linger2 >= 0 ? TCP_TIMEWAIT_LEN : 0); + + r->idiag_timer = 3; + r->idiag_expires = EXPIRES_IN_MS(timer_expires); + } else { + r->idiag_timer = 2; + r->idiag_retrans = icsk->icsk_probes_out; + r->idiag_expires = EXPIRES_IN_MS(sk->sk_timer.expires); + } + } else { + r->idiag_timer = 0; + r->idiag_expires = 0; + } + if (sk->sk_state == TCP_LISTEN) { r->idiag_rqueue = sk->sk_ack_backlog; r->idiag_wqueue = sk->sk_max_ack_backlog; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out 2013-03-18 12:39 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita 2013-03-18 12:43 ` [PATCH v2 2/4] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita @ 2013-03-19 13:32 ` David Miller 2013-03-21 11:45 ` Toshiaki Makita 1 sibling, 1 reply; 7+ messages in thread From: David Miller @ 2013-03-19 13:32 UTC (permalink / raw) To: makita.toshiaki; +Cc: eric.dumazet, netdev From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Date: Mon, 18 Mar 2013 21:39:04 +0900 > 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; > } Well, now you're completely ignoring the user's linger setting. I really can't take these patches seriously, and will not apply them, sorry. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out 2013-03-19 13:32 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out David Miller @ 2013-03-21 11:45 ` Toshiaki Makita 0 siblings, 0 replies; 7+ messages in thread From: Toshiaki Makita @ 2013-03-21 11:45 UTC (permalink / raw) To: David Miller; +Cc: eric.dumazet, netdev, makita.toshiaki On Tue, 2013-03-19 at 09:32 -0400, David Miller wrote: > From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > Date: Mon, 18 Mar 2013 21:39:04 +0900 > > > 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; > > } > > Well, now you're completely ignoring the user's linger setting. If you mention TCP_LINGER2, I don't think I'm ignoring it. It is taken into account in tcp_rcv_state_process() or tcp_close(). If I'm misunderstanding, I'd be glad if you could point out it. > > I really can't take these patches seriously, and will not apply them, > sorry. I think, at least, too short timeout is harmful. If tcp_fin_timeout is set to 61, it will expire in 2 seconds, which cause peer to receive unexpected reset by sending fin. Don't you think there is a problem? Toshiaki Makita ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-03-21 11:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-18 12:36 [PATCH v2 0/4] tcp: fix problems when tcp_fin_timeout is greater than 60 Toshiaki Makita 2013-03-18 12:39 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out Toshiaki Makita 2013-03-18 12:43 ` [PATCH v2 2/4] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp Toshiaki Makita 2013-03-18 12:46 ` [PATCH v2 3/4] tcp: fix the indentation in timer calculation of /proc/net/tcp Toshiaki Makita 2013-03-18 12:50 ` [PATCH v2 4/4] inet_diag: fix FIN_WAIT2 timer expression in inet_diag Toshiaki Makita 2013-03-19 13:32 ` [PATCH v2 1/4] tcp: fix too short FIN_WAIT2 time out David Miller 2013-03-21 11:45 ` Toshiaki Makita
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).