Re: [quicwg/base-drafts] Replace end of recovery with recovery_start_time (#1930)

[Masakazu Kitajo <[e-mail]>]

maskit commented on this pull request.



> @@ -1130,8 +1130,9 @@ Invoked when an ACK frame with an ECN section is received from the peer.
      if (ack.ce_counter > ecn_ce_counter):
        ecn_ce_counter = ack.ce_counter
        // Start a new congestion event if the last acknowledged
-       // packet is past the end of the previous recovery epoch.
-       CongestionEvent(ack.largest_acked_packet)
+       // packet was sent after the start of the previous
+       // recovery epoch.
+       CongestionEvent(sent_packets[ack.largest_acked].time)

> I'm happy to have congestion control depend upon loss detection, but it likely would be cleaner to pass in the time directly.

OK, I just thought other places might avoid it intentionally.

> Actually, we're using the ce counter as well.

If you mean `ack.ce_counter` that one is fine because it is in `ack`. I meant `ProcessECN(ack, largest_acked_sent_time)`

> I think I'll leave it as is unless you think there's a correctness issue?

I'd implement it differently but I'm ok with as-is.

-- 
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/1930#discussion_r229154830