diff mbox

When TCP keepalives tuned shorter than retransmission timeouts

Message ID 4b6029b3-55da-441a-9550-0fed3b49506a@default
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Venkat Venkatsubra Nov. 26, 2013, 3:51 p.m. UTC 
Some of our customers have tcp socket level options set to:
TCP_KEEPIDLE 60
TCP_KEEPINTVL 6 
TCP_KEEPCNT 10

And when the peer is dead they expect the connection to timeout in 2 minutes instead of the
15 minutes from retransmission timeouts.
(We know the tunables are set very low.)

As this code in tcp_keepalive_timer() indicates we skip keepalive probes if there are packets in flight
Or we have more data to send:
/* It is alive without keepalive 8) */
        if (tp->packets_out || tcp_send_head(sk))
                goto resched;

The reason I guess is why burden the network with keepalive packets when
somebody else (retransmissions) is doing it for you.

The change we tried was to not actually send the keepalive probes in this situation but keep counting them as sent. 
To not do this when the receiver window is closed we check tp->snd_wnd. Maybe there are other (more correct ?) ways to do that. 
By the way, we didn't try to address yet the similar issue when the communication with peer dies
after the receiver closes the window.

This is the code change we tried.

We seek your opinion.

Thanks.

Venkat
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eric Dumazet Nov. 26, 2013, 4:20 p.m. UTC | #1 
On Tue, 2013-11-26 at 07:51 -0800, Venkat Venkatsubra wrote:
> Some of our customers have tcp socket level options set to:
> TCP_KEEPIDLE 60
> TCP_KEEPINTVL 6 
> TCP_KEEPCNT 10
> 
> And when the peer is dead they expect the connection to timeout in 2 minutes instead of the
> 15 minutes from retransmission timeouts.
> (We know the tunables are set very low.)

Then change max number of retransmits : tcp_retries2 ?

Keepalive timer is not a way to defeat TCP exponential backoff.

Its really there to send probes when a session is idle.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Venkat Venkatsubra Dec. 4, 2013, 9:50 p.m. UTC | #2 
> 
> Then change max number of retransmits : tcp_retries2 ?
> 
> Keepalive timer is not a way to defeat TCP exponential backoff.
> 
> Its really there to send probes when a session is idle.
> 

Thanks. We plan to try out TCP_USER_TIMEOUT option since that is settable per-socket.

Venkat
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- tcp_timer.c.orig    2013-11-25 07:09:18.328112851 -0800
+++ tcp_timer.c 2013-11-25 08:06:47.339666980 -0800
@@ -588,18 +588,13 @@ 
                        }
                }
                tcp_send_active_reset(sk, GFP_ATOMIC);
-               goto death;
+               tcp_done(sk);
+               goto out;
        }

        if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
                goto out;

-       elapsed = keepalive_time_when(tp);
-
-       /* It is alive without keepalive 8) */
-       if (tp->packets_out || tcp_send_head(sk))
-               goto resched;
-
        elapsed = keepalive_time_elapsed(tp);

        if (elapsed >= keepalive_time_when(tp)) {
@@ -615,8 +610,9 @@ 
                        tcp_write_err(sk);
                        goto out;
                }
-               if (tcp_write_wakeup(sk) <= 0) {
-                       icsk->icsk_probes_out++;
+               if (tp->packets_out || tcp_send_head(sk) || (tcp_write_wakeup(sk) <= 0)) {
+                       if (tp->snd_wnd)
+                               icsk->icsk_probes_out++;
                        elapsed = keepalive_intvl_when(tp);
                } else {
                        /* If keepalive was lost due to local congestion,
@@ -631,12 +627,7 @@ 

        sk_mem_reclaim(sk);

-resched:
        inet_csk_reset_keepalive_timer (sk, elapsed);
-       goto out;
-
-death:
-       tcp_done(sk);

out:
        bh_unlock_sock(sk);