Re: [quicwg/base-drafts] Define AreAllPacketsLost() (#3290)

[ianswett <[e-mail]>]

ianswett commented on this pull request.

Almost there I think.

> @@ -1407,15 +1409,23 @@ Invoked when an ACK frame with an ECN section is received from the peer.
 Invoked from DetectLostPackets when packets are deemed lost.
 
 ~~~
-   InPersistentCongestion(largest_lost_packet):
+   InPersistentCongestion(lost_packets):

I'd add an early return at the very beginning to check if lost_packets is false to avoid questions about whether first() and last() are properly defined.

>       // Determine if all packets in the time period before the
      // newest lost packet, including the edges, are marked
      // lost
-     return AreAllPacketsLost(largest_lost_packet,
-                              congestion_period)
+     all_packets_lost =
+       lost_packets.length() == (lost_packets.last().packet_number -
+                                 lost_packets.first().packet_number - 1)

My mistake on the example, this should be:

```suggestion
                                 lost_packets.first().packet_number + 1)
```

-- 
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/3290#pullrequestreview-328360642