Re: [quicwg/base-drafts] Don't arm the handshake timer if there's no data (#2590)

[martinduke <[e-mail]>]

martinduke commented on this pull request.

My comments are a little broader than the issue, but it's all related.

> @@ -1151,7 +1152,8 @@ OnLossDetectionTimeout():
     DetectLostPackets(pn_space)
   // Retransmit crypto data if no packets were lost
   // and there are still crypto packets in flight.
-  else if (crypto packets are in flight):
+  else if (crypto packets are in flight &&
+           crypto data to send):
     // Crypto retransmission timeout.
     RetransmitUnackedCryptoData()
     crypto_count++

else if (client && (no 1-RTT keys)
    send a 1200B packet
or something to that effect

> @@ -1114,7 +1114,8 @@ SetLossDetectionTimer():
     loss_detection_timer.update(loss_time)
     return
 
-  if (crypto packets are in flight):
+  if (crypto packets are in flight &&
+      crypto data to send):

|| (client && (no 1-RTT keys yet))

> @@ -458,11 +458,11 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
+MUST start the crypto retransmission timer until it has 1RTT keys, even if there

MUST start the crypto retransmission timer upon timeout....

In line 448: ack-eliciting crypto packet?

LIne 451: "if possible, and restart the crypto retransmission timer if there is unacknowledged data"

> @@ -458,11 +458,11 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
+MUST start the crypto retransmission timer until it has 1RTT keys, even if there
+is no unacknowledged CRYPTO data.  If the timer expires and the client has no
+CRYPTO data to retransmit and does not have Handshake keys, it SHOULD send an

SHOULD -> MUST? If we don't do this, the connection could hang in a fairly common case.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590#pullrequestreview-223296742