Re: [quicwg/base-drafts] Persistent congestion pseudocode to match text (#4010)

[Jana Iyengar <[e-mail]>]

@janaiyengar commented on this pull request.



> @@ -1605,26 +1619,36 @@ ProcessECN(ack, pn_space):
 Invoked when DetectAndRemoveLostPackets deems packets lost.
 
 ~~~
-InPersistentCongestion(largest_lost):
-  // Persistent congestion cannot be declared on the
-  // first RTT sample.
-  if (is first RTT sample):
-    return false
+InPersistentCongestion(lost_packets):
+  // Consider lost packets across all packet number spaces.

Perhaps suggest that pc_lost could be time-indexed instead of pn-indexed

> +  // Find the largest contiguous set of lost packets that
+  // starts and ends with an ack-eliciting packet.
+  (first, last) = FindLargestContiguousLoss(pc_lost)

You want the (first, last) around the largest _period_ of loss, not the largest set of lost packets. Perhaps `FindLargestLossPeriod` ?

> -  // first RTT sample.
-  if (is first RTT sample):
-    return false
+InPersistentCongestion(lost_packets):
+  // Consider lost packets across all packet number spaces.
+  pc_lost.append(lost_packets)
+
+  // Disregard packets sent prior to getting an RTT sample.
+  assert(first_rtt_sample != 0)
+  for lost in pc_lost:
+    if lost.time_sent <= first_rtt_sample:
+      pc_lost.remove(lost)
+
+  // Find the largest contiguous set of lost packets that
+  // starts and ends with an ack-eliciting packet.
+  (first, last) = FindLargestContiguousLoss(pc_lost)

@kazu-yamamoto : you can use time indexing for this purpose.

-- 
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/4010#pullrequestreview-474037338