Re: [quicwg/base-drafts] Include handshake anti-deadlock logic in pseudocode (#2281)

[ianswett <[e-mail]>]

ianswett commented on this pull request.

Thanks for fixing this.  Because of the deadlock prevention, this is definitely a trick to get right.

It'd be nice to use the "handshake complete" concept in recovery, but defining it may be more work than it's worth.

> @@ -1078,6 +1078,19 @@ GetEarliestLossTime():
   return time, space
 
 SetLossDetectionTimer():
+  if (crypto packets are in flight

Two suggestions: 
1) If loss_time is non-zero, it will be earlier than the crypto retransmission timer, so it's important to check that first.

2) The check for ack-eliciting packets can be moved right in front of the PTO timer.

>      // Crypto retransmission timeout.
     RetransmitUnackedCryptoData()
     crypto_count++
+  else if (endpoint is client without 1-RTT keys):
+    if (will send Initial packet):
+       SendOnePaddedPacket()
+     else:
+       SendOnePacket()
+    crypto_count++
+  else if (loss_time != 0):

Time threshold loss detection should still be before the crypto case.

>      // Crypto retransmission timeout.
     RetransmitUnackedCryptoData()
     crypto_count++
+  else if (endpoint is client without 1-RTT keys):
+    if (will send Initial packet):

I'd say
"if (has Handshake keys):
  SendOneHandshakePacket()
 else
  SendOnePaddedInitialPacket()" 

-- 
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/2281#pullrequestreview-221761780