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

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.



> @@ -442,13 +442,17 @@ connections over the same network SHOULD use the previous connection's final
 smoothed RTT value as the resumed connection's initial RTT.  If no previous RTT
 is available, or if the network changes, the initial RTT SHOULD be set to 500ms,
 resulting in a 1 second initial handshake timeout as recommended in
-{{?RFC6298}}. When an acknowledgement is received, a new RTT is computed and the
-timer SHOULD be set for twice the newly computed smoothed RTT.
+{{?RFC6298}}. When the first acknowledgement is received, an RTT is computed and
+the timer SHOULD be set for twice the newly computed RTT.

How about "When the first acknowledgement is received, the timer SHOULD be set for twice the value of the resulting RTT sample"? Now that I see further below, why is this necessary at all? I would remove this last sentence. The beginning of the next para says exactly this, since the smoothed_rtt is set to the RTT sample on the first ack.

>  
-When a crypto packet is sent, the sender MUST set a timer for the crypto
-timeout period.  This timer MUST be updated when a new crypto packet is sent.
-Upon timeout, the sender MUST retransmit all unacknowledged CRYPTO data if
-possible.
+When a crypto packet is sent, the sender MUST set a timer for twice the smoothed
+RTT.  This timer MUST be updated when a new crypto packet is sent and when
+an acknowledgement is received which computes a new RTT sample. Upon timeout,
+the sender MUST retransmit all unacknowledged CRYPTO data if possible.

Why "if possible"?

>  
-When a crypto packet is sent, the sender MUST set a timer for the crypto
-timeout period.  This timer MUST be updated when a new crypto packet is sent.
-Upon timeout, the sender MUST retransmit all unacknowledged CRYPTO data if
-possible.
+When a crypto packet is sent, the sender MUST set a timer for twice the smoothed
+RTT.  This timer MUST be updated when a new crypto packet is sent and when
+an acknowledgement is received which computes a new RTT sample. Upon timeout,
+the sender MUST retransmit all unacknowledged CRYPTO data if possible.
+
+On each consecutive expiration of the crypto timer without receiving an
+acknowledgement for a new packet, the sender SHOULD double the crypto
+retransmission timeout and set a timer for this period.

I'd make this a MUST.

> @@ -458,17 +462,16 @@ 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.
-
-On each consecutive expiration of the crypto timer without receiving an
-acknowledgement for a new packet, the sender SHOULD double the crypto
-retransmission timeout and set a timer for this period.
+MUST ensure the crypto retransmission timer is set if there is unacknowledged
+crypto data and MUST ensure the timer is set until it has 1-RTT keys.

This needs rephrasing. How about "MUST ensure that the crypto retransmission timer is set if there is unacknowledged crypto data or if the client does not yet have 1-RTT keys."

> @@ -458,17 +462,16 @@ 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.
-
-On each consecutive expiration of the crypto timer without receiving an
-acknowledgement for a new packet, the sender SHOULD double the crypto
-retransmission timeout and set a timer for this period.
+MUST ensure the crypto retransmission timer is set if there is unacknowledged
+crypto data and MUST ensure the timer is set until it has 1-RTT keys.
+If the timer expires and the client has no CRYPTO data to retransmit and does
+not have Handshake keys, it MUST send an Initial packet in a UDP datagram of
+at least 1200 bytes.  If the client has Handshake keys, it MUST send a
+Handshake packet.

Suggested reword: "If the crypto retransmission timer expires before the client has 1-RTT keys, it is possible that the client may not have any crypto data to retransmit. The client MUST still send a new packet to allow the server to continue sending data (see Section XX in {{QUIC-TRANSPORT}}).  If Handshake keys are available to the client, it MUST send a Handshake packet, and otherwise, an Initial packet in a UDP datagram of at least 1200 bytes."

> @@ -1114,7 +1117,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):

what does this predicate mean? That there's new data to send?

-- 
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-224097674