Re: [quicwg/base-drafts] Add a loss_time per packet number space (#2451)

[Marten Seemann <[e-mail]>]

marten-seemann commented on this pull request.



> @@ -1067,14 +1066,18 @@ Pseudocode for OnLossDetectionTimeout follows:
 
 ~~~
 OnLossDetectionTimeout():
-  if (crypto packets are in flight):
+  packets_lost = false
+  for pn_space in [ Initial, Handshake, ApplicatonData ]:
+    if (loss_times[pn_space] != 0):
+      // Time threshold loss Detection
+      DetectLostPackets(pn_space)

It seems a bit wasteful to run this for every packet number space that has a non-zero `loss_time`, since we'll only detect lost packets in the space that has the earliest `loss_time`.

> @@ -1067,14 +1066,18 @@ Pseudocode for OnLossDetectionTimeout follows:
 
 ~~~
 OnLossDetectionTimeout():
-  if (crypto packets are in flight):
+  packets_lost = false
+  for pn_space in [ Initial, Handshake, ApplicatonData ]:
+    if (loss_times[pn_space] != 0):
+      // Time threshold loss Detection
+      DetectLostPackets(pn_space)
+      packets_lost = true
+  // Don't retransmit all crypto data if a packet was just lost.
+  if (!packets_lost &&

If `packets_lost = true`, we'd send PTO packets. I don't think this is the intention. We should return early in that 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/2451#pullrequestreview-202187885