Re: [quicwg/base-drafts] Include ack_delay when deciding with PN space to arm PTO for (#3666)

[martinduke <[e-mail]>]

@martinduke requested changes on this pull request.



> -  space = Initial
-  for pn_space in [ Handshake, ApplicationData ]:
-    if (times[pn_space] != 0 &&
-        (time == 0 || times[pn_space] < time) &&
-        # Skip ApplicationData until handshake completion.
-        (pn_space != ApplicationData ||
-          IsHandshakeComplete()):
-      time = times[pn_space];
-      space = pn_space
+GetEarliestTimeAndSpace(times, ack_delay):
+  time, space = times[Initial], Initial
+  if (times[Handshake] != 0 &&
+      (time == 0 || times[Handshake] < time)):
+    time, space = times[Handshake], Handshake
+  # Skip ApplicationData until handshake completion.
+  if (pn_space == ApplicationData &&

I think we need to delete this line.

> @@ -1270,7 +1273,7 @@ PeerCompletedAddressValidation():
        has received HANDSHAKE_DONE
 
 SetLossDetectionTimer():
-  earliest_loss_time, _ = GetEarliestTimeAndSpace(loss_time)
+  earliest_loss_time, _ = GetEarliestTimeAndSpace(loss_time, 0)
   if (earliest_loss_time != 0):
     // Time threshold loss detection.

I confess to not really understanding this bit of code at all. If any packet has been sent in any PN space, we don't actually  go through the motions of setting the timer? Before we check for anti-amplification limit, etc?

-- 
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/3666#pullrequestreview-413750535