Re: [quicwg/base-drafts] lost_pn can no longer be negative (#2621)

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.

This also doesn't solve the problem where:
```lost_pn = largest_acked_packet[pn_space] - kPacketThreshold```
results in a negative `lost_pn`.  My suggestions would be:
1. Initialize lost_pn to infinite
2. When setting `lost_pn`, use the following code:
```
if largest_acked_packet[pn_space] >= kPacketThreshold:
    lost_pn_threshold = largest_acked_packet[pn_space] - kPacketThreshold
```
3. Change the condition below on line 1279 to:
```
(lost_pn != infinite && unacked.packet_number <= lost_pn)
```

> @@ -1273,7 +1276,8 @@ DetectLostPackets(pn_space):
 
     // Mark packet as lost, or set time when it should be marked.
     if (unacked.time_sent <= lost_send_time ||
-        unacked.packet_number <= lost_pn):
+        (largest_acked_packet[pn_space] != infinite &&

DetectLostPackets is only ever called after at least one ACK has been received for this pn_space, so instead of checking for this here, I think this should be an assert() up at the top of the function.

-- 
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/2621#pullrequestreview-226874189