Commit fc68e171 authored by Yuchung Cheng's avatar Yuchung Cheng Committed by David S. Miller
Browse files

tcp: revert F-RTO extension to detect more spurious timeouts 



This reverts commit 89fe18e4.

While the patch could detect more spurious timeouts, it could cause
poor TCP performance on broken middle-boxes that modifies TCP packets
(e.g. receive window, SACK options). Since the performance gain is
much smaller compared to the potential loss. The best solution is
to fully revert the change.

Fixes: 89fe18e4 ("tcp: extend F-RTO to catch more spurious timeouts")
Reported-by: default avatarTeodor Milkov <tm@del.bg>
Signed-off-by: default avatarYuchung Cheng <ycheng@google.com>
Signed-off-by: default avatarNeal Cardwell <ncardwell@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent d4131f09

net/ipv4/tcp_input.c

+12 −18
Original line number Diff line number Diff line
@@ -1909,6 +1909,7 @@ void tcp_enter_loss(struct sock *sk) 
	struct tcp_sock *tp = tcp_sk(sk);

	struct net *net = sock_net(sk);

	struct sk_buff *skb;

	bool new_recovery = icsk->icsk_ca_state < TCP_CA_Recovery;

	bool is_reneg;			/* is receiver reneging on SACKs? */

	bool mark_lost;
@@ -1967,15 +1968,12 @@ void tcp_enter_loss(struct sock *sk) 
	tp->high_seq = tp->snd_nxt;

	tcp_ecn_queue_cwr(tp);



	/* F-RTO RFC5682 sec 3.1 step 1 mandates to disable F-RTO

	 * if a previous recovery is underway, otherwise it may incorrectly

	 * call a timeout spurious if some previously retransmitted packets

	 * are s/acked (sec 3.2). We do not apply that retriction since

	 * retransmitted skbs are permanently tagged with TCPCB_EVER_RETRANS

	 * so FLAG_ORIG_SACK_ACKED is always correct. But we do disable F-RTO

	 * on PTMU discovery to avoid sending new data.

	/* F-RTO RFC5682 sec 3.1 step 1: retransmit SND.UNA if no previous

	 * loss recovery is underway except recurring timeout(s) on

	 * the same SND.UNA (sec 3.2). Disable F-RTO on path MTU probing

	 */

	tp->frto = net->ipv4.sysctl_tcp_frto &&

		   (new_recovery || icsk->icsk_retransmits) &&

		   !inet_csk(sk)->icsk_mtup.probe_size;

}
@@ -2628,18 +2626,14 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack, 
	    tcp_try_undo_loss(sk, false))

		return;



	/* The ACK (s)acks some never-retransmitted data meaning not all

	 * the data packets before the timeout were lost. Therefore we

	 * undo the congestion window and state. This is essentially

	 * the operation in F-RTO (RFC5682 section 3.1 step 3.b). Since

	 * a retransmitted skb is permantly marked, we can apply such an

	 * operation even if F-RTO was not used.

	if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */

		/* Step 3.b. A timeout is spurious if not all data are

		 * lost, i.e., never-retransmitted data are (s)acked.

		 */

		if ((flag & FLAG_ORIG_SACK_ACKED) &&

	    tcp_try_undo_loss(sk, tp->undo_marker))

		    tcp_try_undo_loss(sk, true))

			return;



	if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */

		if (after(tp->snd_nxt, tp->high_seq)) {

			if (flag & FLAG_DATA_SACKED || is_dupack)

				tp->frto = 0; /* Step 3.a. loss was real */

CRA Git | Maintained and supported by SUSTech CRA and CCSE