[PATCH net] tcp: fix undo spurious SYNACK in passive Fast Open

[PATCH net] tcp: fix undo spurious SYNACK in passive Fast Open

[Yuchung Cheng]

Commit 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK
retransmit") may cause tcp_fastretrans_alert() to warn about pending
retransmission in Open state. This is triggered when the Fast Open
server both sends data and has spurious SYNACK retransmission during
the handshake, and the data packets were lost or reordered.

The root cause is a bit complicated:

(1) Upon receiving SYN-data: a full socket is created with
    snd_una = ISN + 1 by tcp_create_openreq_child()

(2) On SYNACK timeout the server/sender enters CA_Loss state.

(3) Upon receiving the final ACK to complete the handshake, sender
    does not mark FLAG_SND_UNA_ADVANCED since (1)

    Sender then calls tcp_process_loss since state is CA_loss by (2)

(4) tcp_process_loss() does not invoke undo operations but instead
    mark REXMIT_LOST to force retransmission

(5) tcp_rcv_synrecv_state_fastopen() calls tcp_try_undo_loss(). It
    changes state to CA_Open but has positive tp->retrans_out

(6) Next ACK triggers the WARN_ON in tcp_fastretrans_alert()

The step that goes wrong is (4) where the undo operation should
have been invoked because the ACK successfully acknowledged the
SYN sequence. This fixes that by specifically checking undo
when the SYN-ACK sequence is acknowledged. Then after
tcp_process_loss() the state would be further adjusted based
in tcp_fastretrans_alert() to avoid triggering the warning in (6).

Fixes: 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK retransmit")

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 08a477e74cf3..38dfc308c0fb 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2648,7 +2648,7 @@ static void tcp_process_loss(struct sock *sk, int flag, int num_dupack,
 	struct tcp_sock *tp = tcp_sk(sk);
 	bool recovered = !before(tp->snd_una, tp->high_seq);
 
-	if ((flag & FLAG_SND_UNA_ADVANCED) &&
+	if ((flag & FLAG_SND_UNA_ADVANCED || tp->fastopen_rsk) &&
 	    tcp_try_undo_loss(sk, false))
 		return;
 
-- 
2.22.0.rc2.383.gf4fbbf30c2-goog

Re: [PATCH net] tcp: fix undo spurious SYNACK in passive Fast Open

[David Miller]

From: Yuchung Cheng <ycheng@google.com>
Date: Fri,  7 Jun 2019 18:26:33 -0700

> Commit 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK
> retransmit") may cause tcp_fastretrans_alert() to warn about pending
> retransmission in Open state. This is triggered when the Fast Open
> server both sends data and has spurious SYNACK retransmission during
> the handshake, and the data packets were lost or reordered.
> 
> The root cause is a bit complicated:
> 
> (1) Upon receiving SYN-data: a full socket is created with
>     snd_una = ISN + 1 by tcp_create_openreq_child()
> 
> (2) On SYNACK timeout the server/sender enters CA_Loss state.
> 
> (3) Upon receiving the final ACK to complete the handshake, sender
>     does not mark FLAG_SND_UNA_ADVANCED since (1)
> 
>     Sender then calls tcp_process_loss since state is CA_loss by (2)
> 
> (4) tcp_process_loss() does not invoke undo operations but instead
>     mark REXMIT_LOST to force retransmission
> 
> (5) tcp_rcv_synrecv_state_fastopen() calls tcp_try_undo_loss(). It
>     changes state to CA_Open but has positive tp->retrans_out
> 
> (6) Next ACK triggers the WARN_ON in tcp_fastretrans_alert()
> 
> The step that goes wrong is (4) where the undo operation should
> have been invoked because the ACK successfully acknowledged the
> SYN sequence. This fixes that by specifically checking undo
> when the SYN-ACK sequence is acknowledged. Then after
> tcp_process_loss() the state would be further adjusted based
> in tcp_fastretrans_alert() to avoid triggering the warning in (6).
> 
> Fixes: 794200d66273 ("tcp: undo cwnd on Fast Open spurious SYNACK retransmit")
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>

Applied, thank you.