Messages in this thread | | | From | Yuchung Cheng <> | Date | Tue, 12 Jan 2021 14:48:01 -0800 | Subject | Re: [PATCH] tcp: keepalive fixes |
| |
On Tue, Jan 12, 2021 at 2:31 PM Enke Chen <enkechen2020@gmail.com> wrote: > > From: Enke Chen <enchen@paloaltonetworks.com> > > In this patch two issues with TCP keepalives are fixed: > > 1) TCP keepalive does not timeout when there are data waiting to be > delivered and then the connection got broken. The TCP keepalive > timeout is not evaluated in that condition. hi enke Do you have an example to demonstrate this issue -- in theory when there is data inflight, an RTO timer should be pending (which considers user-timeout setting). based on the user-timeout description (man tcp), the user timeout should abort the socket per the specified time after data commences. some data would help to understand the issue.
> > The fix is to remove the code that prevents TCP keepalive from > being evaluated for timeout. > > 2) With the fix for #1, TCP keepalive can erroneously timeout after > the 0-window probe kicks in. The 0-window probe counter is wrongly > applied to TCP keepalives. > > The fix is to use the elapsed time instead of the 0-window probe > counter in evaluating TCP keepalive timeout. > > Cc: stable@vger.kernel.org > Signed-off-by: Enke Chen <enchen@paloaltonetworks.com> > --- > net/ipv4/tcp_timer.c | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) > > diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c > index 6c62b9ea1320..40953aa40d53 100644 > --- a/net/ipv4/tcp_timer.c > +++ b/net/ipv4/tcp_timer.c > @@ -696,12 +696,6 @@ static void tcp_keepalive_timer (struct timer_list *t) > ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_SYN_SENT))) > goto out; > > - elapsed = keepalive_time_when(tp); > - > - /* It is alive without keepalive 8) */ > - if (tp->packets_out || !tcp_write_queue_empty(sk)) > - goto resched; > - > elapsed = keepalive_time_elapsed(tp); > > if (elapsed >= keepalive_time_when(tp)) { > @@ -709,16 +703,15 @@ static void tcp_keepalive_timer (struct timer_list *t) > * to determine when to timeout instead. > */ > if ((icsk->icsk_user_timeout != 0 && > - elapsed >= msecs_to_jiffies(icsk->icsk_user_timeout) && > - icsk->icsk_probes_out > 0) || > + elapsed >= msecs_to_jiffies(icsk->icsk_user_timeout)) || > (icsk->icsk_user_timeout == 0 && > - icsk->icsk_probes_out >= keepalive_probes(tp))) { > + (elapsed >= keepalive_time_when(tp) + > + keepalive_intvl_when(tp) * keepalive_probes(tp)))) { > tcp_send_active_reset(sk, GFP_ATOMIC); > tcp_write_err(sk); > goto out; > } > if (tcp_write_wakeup(sk, LINUX_MIB_TCPKEEPALIVE) <= 0) { > - icsk->icsk_probes_out++; > elapsed = keepalive_intvl_when(tp); > } else { > /* If keepalive was lost due to local congestion, > @@ -732,8 +725,6 @@ static void tcp_keepalive_timer (struct timer_list *t) > } > > sk_mem_reclaim(sk); > - > -resched: > inet_csk_reset_keepalive_timer (sk, elapsed); > goto out; > > -- > 2.29.2 >
| |