Re: TCP_USER_TIMEOUT, SYN-SENT and tcp_syn_retries

[Eric Dumazet <eric.dumazet@gmail.com>]

On 9/26/19 9:46 AM, Eric Dumazet wrote:
> 
> 
> On 9/26/19 8:05 AM, Eric Dumazet wrote:
>>
>>
>> On 9/25/19 1:46 AM, Marek Majkowski wrote:
>>> Hello my favorite mailing list!
>>>
>>> Recently I've been looking into TCP_USER_TIMEOUT and noticed some
>>> strange behaviour on fresh sockets in SYN-SENT state. Full writeup:
>>> https://blog.cloudflare.com/when-tcp-sockets-refuse-to-die/
>>>
>>> Here's a reproducer. It does a simple thing: sets TCP_USER_TIMEOUT and
>>> does connect() to a blackholed IP:
>>>
>>> $ wget https://gist.githubusercontent.com/majek/b4ad53c5795b226d62fad1fa4a87151a/raw/cbb928cb99cd6c5aa9f73ba2d3bc0aef22fbc2bf/user-timeout-and-syn.py
>>>
>>> $ sudo python3 user-timeout-and-syn.py
>>> 00:00.000000 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:01.007053 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:03.023051 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:05.007096 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:05.015037 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:05.023020 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>> 00:05.034983 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>
>>> The connect() times out with ETIMEDOUT after 5 seconds - as intended.
>>> But Linux (5.3.0-rc3) does something weird on the network - it sends
>>> remaining tcp_syn_retries packets aligned to the 5s mark.
>>>
>>> In other words: with TCP_USER_TIMEOUT we are sending spurious SYN
>>> packets on a timeout.
>>>
>>> For the record, the man page doesn't define what TCP_USER_TIMEOUT does
>>> on SYN-SENT state.
>>>
>>
>> Exactly, so far this option has only be used on established flows.
>>
>> Feel free to send patches if you need to override the stack behavior
>> for connection establishment (Same remark for passive side...)
> 
> Also please take a look at TCP_SYNCNT,  which predates TCP_USER_TIMEOUT
> 
> 

I will test the following :

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index dbd9d2d0ee63aa46ad2dda417da6ec9409442b77..1182e51a6b794d75beb8c130354d7804fc83a307 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -220,7 +220,6 @@ static int tcp_write_timeout(struct sock *sk)
                        sk_rethink_txhash(sk);
                }
                retry_until = icsk->icsk_syn_retries ? : net->ipv4.sysctl_tcp_syn_retries;
-               expired = icsk->icsk_retransmits >= retry_until;
        } else {
                if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1, 0)) {
                        /* Black hole detection */
@@ -242,9 +241,9 @@ static int tcp_write_timeout(struct sock *sk)
                        if (tcp_out_of_resources(sk, do_reset))
                                return 1;
                }
-               expired = retransmits_timed_out(sk, retry_until,
-                                               icsk->icsk_user_timeout);
        }
+       expired = retransmits_timed_out(sk, retry_until,
+                                       icsk->icsk_user_timeout);
        tcp_fastopen_active_detect_blackhole(sk, expired);
 
        if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))