Re: [quicwg/base-drafts] Editorial rework (#2080)

[ianswett <[e-mail]>]

ianswett commented on this pull request.

Thanks Jana, I think this is a nice improvement on my PR

> -fraction of an RTT, but implementations MAY experiment with absolute
-thresholds. The RECOMMENDED time threshold, expressed as a fraction
-of the round-trip time (kTimeThreshold), is 1/8.
-
-An endpoint SHOULD declare packets lost no earlier than
-(1 + kTimeThreshold) * max(SRTT, latest_RTT) after when they were
-sent.  If packets sent prior to the largest acknowledged packet cannot yet
-be declared lost, then a timer SHOULD be set for the remaining time.
+{{?RFC6675}}, and RACK {{?RACK=I-D.ietf-tcpm-rack}}. This section provides an
+overview of how these algorithms are implemented in QUIC.
+
+When an ACK frame is received that newly acknowledges a packet, a QUIC sender
+uses two thresholds to determine if prior packets should be declared lost.  A
+packet is declared lost under the following conditions:
+
+* it has a smaller packet number than the newly acknowledged packet,

I would prefer merging the first two bullets because I think it might flow better.
ie: "The packet is retransmittable, unacknowledged, and was sent prior to an acknowledged packet."

The removal of the mention of packet number was to capture the concept a bit more and we talk about packet number in the next bullet.

Also, we typically capitalize the first letter after the *, which I think looks better.

> @@ -596,13 +570,14 @@ kMaxTLPs:
   The RECOMMENDED value is 2.
 
 kPacketThreshold:
-: Maximum reordering in packet number space before FACK style loss detection
+: Maximum reordering in packets  before packet threshold loss detection

```suggestion
: Maximum reordering in packets before packet threshold loss detection
```

> -#### Time Threshold
-
-Time threshold loss detection uses a time threshold to determine how much
-reordering to tolerate.  In this document, the threshold is expressed as a
-fraction of an RTT, but implementations MAY experiment with absolute
-thresholds. The RECOMMENDED time threshold, expressed as a fraction
-of the round-trip time (kTimeThreshold), is 1/8.
-
-An endpoint SHOULD declare packets lost no earlier than
-(1 + kTimeThreshold) * max(SRTT, latest_RTT) after when they were
-sent.  If packets sent prior to the largest acknowledged packet cannot yet
-be declared lost, then a timer SHOULD be set for the remaining time.
+{{?RFC6675}}, and RACK {{?RACK=I-D.ietf-tcpm-rack}}. This section provides an
+overview of how these algorithms are implemented in QUIC.
+
+When an ACK frame is received that newly acknowledges a packet, a QUIC sender

I'm now thinking we should drop newly because we also trigger loss on a timer.  If there's something subtle we're trying to say there, we should probably make it more explicit?

At some point, we discussed basing loss detection on the largest recently acknowledged packet(ie: in the last ACK), but I don't think that's what the pseudocode does and I don't think that's what the current gQUIC code does.  If we want to discuss that, maybe add a paragraph that the algorithm may be enhanced by doing so and it may reduce spurious retransmits in some cases?

> +upon detecting loss.  Implementations that detect spurious retransmissions and
+increase the reordering threshold in packets or time MAY choose to start with
+smaller initial reordering thresholds to minimize recovery latency.
+
+### Packet Threshold
+
+The RECOMMENDED initial value for the packet reordering threshold
+(kPacketThreshold) is 3, based on TCP loss detection {{?RFC5681}} {{?RFC6675}}.
+
+Some networks may exhibit higher degrees of reordering, causing a sender to
+detect spurious losses.  Implementers MAY use algorithms developed for TCP, such
+as TCP-NCR {{?RFC4653}}, to improve QUIC's reordering resilience.
+
+### Time Threshold {#time-threshold}
+
+Ack-based loss detection uses a time threshold to determine how much reordering

I find changing this sentence to start with "Ack-based" is confusing, because there's also the packet number threshold.

How about dropping this first sentence entirely, since I think it's said better above?

-- 
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/2080#pullrequestreview-180523967