Re: [PATCH][v4] tcp: fix ICMP-RTO war
From: Ilpo Järvinen <hidden>
Date: 2010-02-16 12:45:27
First of all I want to let you know that I've no objection to this fix itself (DaveM applied it already), it certainly does it's jobs in preventing invalid < RTO_MIN state. However, I wonder if we could do further improvements in this area... On Wed, 10 Feb 2010, Damian Lukowski wrote:
quoted hunk ↗ jump to hunk
Make sure, that TCP has a nonzero RTT estimation after three-way handshake. Currently, a listening TCP has a value of 0 for srtt, rttvar and rto right after the three-way handshake is completed with TCP timestamps disabled. This will lead to corrupt RTO recalculation and retransmission flood when RTO is recalculated on backoff reversion as introduced in "Revert RTO on ICMP destination unreachable" (f1ecd5d9e7366609d640ff4040304ea197fbc618). This behaviour can be provoked by connecting to a server which "responds first" (like SMTP) and rejecting every packet after the handshake with dest-unreachable, which will lead to softirq load on the server (up to 30% per socket in some tests). Thanks to Ilpo Jarvinen for providing debug patches and to Denys Fedoryshchenko for reporting and testing. Changes since v3: Removed bad characters in patchfile. Reported-by: Denys Fedoryshchenko <redacted> Signed-off-by: Damian Lukowski <redacted> --- net/ipv4/tcp_input.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 28e0296..da7173b 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c@@ -5783,12 +5783,10 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb, /* tcp_ack considers this ACK as duplicate * and does not calculate rtt. - * Fix it at least with timestamps. + * Force it here. */ - if (tp->rx_opt.saw_tstamp && - tp->rx_opt.rcv_tsecr && !tp->srtt) - tcp_ack_saw_tstamp(sk, 0); - + tcp_ack_update_rtt(sk, 0, 0); +
...Here a zero seq_rtt is given to RTT estimator (it will be effective only in the case w/o timestamps, TS case recalculates it from the stored timestamps). Maybe we could use some field (timestamp related one comes to my mind) in request sock to get a real RTT estimate for non-timestamp case too. ...It seems possible to me, though tricky because the request_sock is no longer that easily available here so some parameter passing would be needed. -- i.