| Message ID | 20190608012633.107118-1-ycheng@google.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] tcp: fix undo spurious SYNACK in passive Fast Open | expand |
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.
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;
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(-)