Re: [quicwg/base-drafts] Proposal for adding ECN support to QUIC. (#1372)

[ianswett <[e-mail]>]

ianswett commented on this pull request.

Some small comments, but I think this looks good

>  (ECN-CE) SHOULD be acknowledged immediately to quicker react to
-congesiton events. Additional ECN-CE marks received during the same
+congestion events. Additional ECN-CE marks received during the same

I don't think you need to say "during the same recovery period" since we're not proscribing any receiver side behavior corresponding to whether the marks are received within the same recovery period.

> @@ -904,9 +904,9 @@ experimentation in using other response functions as discussed in
 
 The ACK_ECN frame defined in {{QUIC-TRANSPORT}} does not provide
 information on which of the newly acknowledged packets that

nit: remove "that"

>  
 ~~~
-   OnPacketsMarked(ce_counter):
-     if (ce_counter > ack_ce_cntr):
-       // update ack_ce_cntr
-       ack_ce_cntr = ce_counter
-     if (!InRecovery(largest_acked_packet)):
+   CongestionEvent(packet_number):

I'd prefer OnCongestionEvent to be consistent with the other names.

> @@ -1442,16 +1441,15 @@ endpoint, an ACK_ECN frame is transmitted back. This ACK_ECN frame indicates how
 many packets that are marked ECT(0), ECT(1) or ECN-CE. Thus the endpoint can
 determine if the endpoints and path is ECN capable.
 
-The ACK_ECN frame will, when received, confirm that the path direction supports
-ECN if the counters show a correct amount of packets received for a valid and
-expected counter combination. ECT marked packets can become remarked as CE along
-the path between the peers. The ACK_ECN counters are added together and compared
-to total number of ACKed packets, if the sum of the counters are equal too or
-larger than the number of ACKed packets then the path is ECN capable. This
-capability check thus has verified that one direction of the path between the
-peers is free from issues with ECN bleaching and that the application does not
-experience problems with access to the ECN field in the IP header. In this case
-the marking of the generated packets with ECT continues.
+When ACK_ECN is received, the counter values can be used to confirm the expected
+number of marks were received. ECT marked packets can become remarked as CE
+along the path between the peers. The ACK_ECN counters are added together and
+compared to total number of ACKed packets, if the sum of the counters are equal
+too or larger than the number of ACKed packets then the path is ECN capable.

too -> to, or just remove the too entirely

-- 
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/1372#pullrequestreview-127977693