netdev - When TCP keepalives tuned shorter than retransmission timeouts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <4b6029b3-55da-441a-9550-0fed3b49506a@default>
Date:	Tue, 26 Nov 2013 07:51:55 -0800 (PST)
From:	Venkat Venkatsubra <venkat.x.venkatsubra@...cle.com>
To:	netdev@...r.kernel.org
Cc:	David Miller <davem@...emloft.net>
Subject: When TCP keepalives tuned shorter than retransmission timeouts

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.
--- 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);

We seek your opinion.

Thanks.

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