Subject: [quicwg/base-drafts] Simplify TLP and RTO into Probe Timeout (#2114)

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 15:57:24 +0000 (UTC)

This PR simplifies QUIC's timeout-based loss detection mechanisms. Unfortunately, this PR cuts through text is many places, so I'm summarizing the key changes here.  The compiled version of the draft, [here](https://quicwg.org/base-drafts/pto/draft-ietf-quic-recovery.html), might be easier to read than the diffs.

- TLP and RTO are combined into a single "Probe Timeout" (PTO). Consecutive PTOs are exponentially longer, just like RTO used to be.  The rationale for calling this a "Probe" Timeout is that loss detection in QUIC only happens when an ack is received, and this timer and timeout only cause probe packets to be sent out to elicit acks.

- Min TLP, RTO, crypto timeout values are now removed. We had a minTLP of 10ms (which was used as a min for the crypto timeout as well) and a minRTO of 200ms.  The number of 10ms was a bit arbitrary, and based on a long-expired TCP TLP individual draft. We discussed the minRTO in #1017, and there was general agreement that adding a max_ack_delay should allow us to remove the minRTO. max_ack_delay is now present and included in the PTO computation.

- Clock granularity is added for both crypto and PTO timeouts, ensuring that even when the computed network RTT might be too small and represented as 0, timeouts are set for at least 1 clock tick. This is in keeping with use of granularity in TCP (RFC 6298). A min granularity of 1ms is recommended.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2114

-- Commit Summary --

  * sync
  * first cut of mechanism
  * complete text
  * pseudocode changes

-- File Changes --

    M draft-ietf-quic-recovery.md (251)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2114.patch
https://github.com/quicwg/base-drafts/pull/2114.diff

-- 
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/2114

---

From: Subodh Iyengar <[email-masked]>
Date: Tue, 11 Dec 2018 13:11:20 -0800

siyengar commented on this pull request.

> @@ -726,10 +700,9 @@ Pseudocode for OnAckReceived and UpdateRtt follow:
       // RTO was spurious. Otherwise, inform congestion control.
       if (rto_count > 0 &&
             smallest_newly_acked > largest_sent_before_rto):

largest_sent_before_rto is still used here

 ~~~
-   OnRetransmissionTimeoutVerified(packet_number)
+   OnProbeTimeoutVerified(packet_number)

This looks like we will set the probetimeout verified even if this happens

    1, 2, 3 [lost], 4 [probe], 5 [probe]
    -> [ack 4]
    -> cwnd = mincwnd

in the first probe case it seems like the probe would be aggressive so it would be bad to drop the cwnd down.

even if it not aggressive right now, in the future I think we are going to tune timeouts constant per application (realtime vs interactive). This means that if we tune timeout constants too aggressively, it will result in having this reaction.

-- 
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/2114#pullrequestreview-183900828

---

From: Nick Banks <[email-masked]>
Date: Tue, 11 Dec 2018 13:35:51 -0800

nibanks commented on this pull request.

In general, I like the simplification here. It would be interesting to see the results of an A/B test before and after these changes.

> @@ -361,8 +361,7 @@ increases loss detection delay.
 
 Timeout loss detection recovers from losses that cannot be handled by
 ack-based loss detection.  It uses a single timer which switches between
-a crypto retransmission timer, a Tail Loss Probe timer and Retransmission
-Timeout mechanisms.
+a crypto retransmission timer and a Probe timer.

You don't capitalize `Probe` in other places. It looks out of place here.

As well as `rto_count`

> @@ -812,17 +785,11 @@ Pseudocode for SetLossDetectionTimer follows:
       loss_detection_timer.set(loss_time)
       return
 
-    // RTO or TLP timer
-    // Calculate RTO duration
+    // PTO timer. Calculate PTO duration

Nit: I feel like having either `PTO timer` or `Calculate PTO duration` would be cleaner. Having both seems a bit overdone.

-## Retransmission Timeout
+A PTO expiration is classified as spurious or valid when an ACK frame is
+received that newly acknowledges packets in flight, see {{pto-loss}}.  On a
+valid PTO, the congestion window MUST be reduced to the minimum congestion
+window and slow start is re-entered.

Just to confirm my understanding, this is a difference from TLP before, right? Before this change, if TLP triggered and you sent a probe packet that elicited an ACK for just that packet, you wouldn't go back into slow start, but now an ACK for only the first probe packet would trigger slow start. Correct?

-- 
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/2114#pullrequestreview-183909793

---

From: ianswett <[email-masked]>
Date: Tue, 11 Dec 2018 13:57:00 -0800

ianswett commented on this pull request.

> @@ -328,8 +328,8 @@ 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.
+detect spurious losses.  Implementers MAY increase the packet threshold under
+such conditions.

This change seems unrelated to the rest of the change.

-- 
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/2114#pullrequestreview-183923299

---

From: ianswett <[email-masked]>
Date: Tue, 11 Dec 2018 14:22:59 -0800

I like this change in general, but I think it makes sense to keep existing behavior more similar to the status quo and allow a sender to do a normal loss-style reduction if the first or second PTO are the ones acknowledged.

> @@ -342,7 +342,7 @@ lost, then a timer SHOULD be set for the remaining time.
 The RECOMMENDED time threshold (kTimeThreshold), expressed as a round-trip time
 multiplier, is 9/8.
 
-Using max(SRTT, latest_RTT) protects from the two following cases:
+Using max(smoothed_rtt, latest_rtt) protects from the two following cases:

You still use max(SRTT, latest_RTT) on 338.

-A packet sent at the tail is particularly vulnerable to slow loss detection,
-since acks of subsequent packets are needed to trigger ack-based detection. To
-ameliorate this weakness of tail packets, the sender schedules a timer when the
-last ack-eliciting packet before quiescence is transmitted. Upon timeout,
-a Tail Loss Probe (TLP) packet is sent to evoke an acknowledgement from the
-receiver.
+A Probe Timeout (PTO) triggers a probe packet when ack-eliciting data is in
+flight but an acknowledgement does not seem forthcoming.  A PTO enables a

"does not seem forthcoming" is non-specific.
```suggestion
flight but an acknowledgement is not received within the expected time.  A PTO enables a
```

-* PTO SHOULD be scheduled for max(1.5*SRTT+MaxAckDelay, kMinTLPTimeout)
+When the final ack-eliciting packet before quiescence is transmitted, the sender

Final is an optimization and we don't define quiescence
```suggestion
When an ack-eliciting packet is sent, the sender
```

> @@ -577,21 +560,18 @@ kTimeThreshold:
   considers a packet lost. Specified as an RTT multiplier. The RECOMMENDED
   value is 9/8.
 
-kMinTLPTimeout:
-: Minimum time in the future a tail loss probe timer may be set for.
-  The RECOMMENDED value is 10ms.
+kGranularity:

It feels odd to define this here, but use it three times up above.  I'd suggest discussing the idea that you're using clock granularity above where you first use it.

-A PTO value of at least 1.5*SRTT ensures that the ACK is overdue.  The 1.5 is
-based on {{?TLP}}, but implementations MAY experiment with other constants.
+The PTO period is the amount of time that a sender ought to wait for an

```suggestion
The PTO period is the amount of time that a sender SHOULD wait for an
```

-A PTO value of at least 1.5*SRTT ensures that the ACK is overdue.  The 1.5 is
-based on {{?TLP}}, but implementations MAY experiment with other constants.
+The PTO period is the amount of time that a sender ought to wait for an
+acknowledgement for a sent packet to be received.  This time period includes the

```suggestion
acknowledgement of a sent packet.  This time period includes the
```

-A PTO value of at least 1.5*SRTT ensures that the ACK is overdue.  The 1.5 is
-based on {{?TLP}}, but implementations MAY experiment with other constants.
+The PTO period is the amount of time that a sender ought to wait for an
+acknowledgement for a sent packet to be received.  This time period includes the
+estimated network roundtrip-time (smoothed_rtt), the variance in the estimate

Using variance here is potentially confusing, since it's not true variance and it's 4*rttvar.

I'm having a hard time coming up with suggestions, but I'll think about it.

-To reduce latency, it is RECOMMENDED that the sender set and allow the TLP timer
-to fire twice before setting an RTO timer. In other words, when the TLP timer
-expires the first time, a TLP packet is sent, and it is RECOMMENDED that the TLP
-timer be scheduled for a second time. When the TLP timer expires the second
-time, a second TLP packet is sent, and an RTO timer SHOULD be scheduled {{rto}}.
+There is no requirement on the clock granularity. If the PTO computation results
+in a value of zero, a sender MUST set the PTO value to kGranularity, to avoid

How about "The PTO must be set to at least kGranularity,"

-A TLP packet SHOULD carry new data when possible. If new data is unavailable or
-new data cannot be sent due to flow control, a TLP packet MAY retransmit
-unacknowledged data to potentially reduce recovery time. Since a TLP timer is
-used to send a probe into the network prior to establishing any packet loss,
-prior unacknowledged packets SHOULD NOT be marked as lost when a TLP timer
-expires.
+A PTO timer is set on an ack-eliciting tail packet.  A sender may not know that

Drop this first sentence and move the second sentence up with "When the final ack-eliciting packet"

-Similar to TCP {{?RFC6298}}, the RTO period is set based on the following
-conditions:
+When a PTO timer expires, the sender MUST send one ack-eliciting packet as a
+probe. A sender MAY send up to two ack-eliciting packets, to avoid an expensive
+consecutive PTO expiration due to packet loss.

```suggestion
consecutive PTO expiration due to a single packet loss.
```

-* When an RTO timer expires, the RTO period is doubled.
+Probe packets sent on a PTO MUST be ack-eliciting.  A probe packet SHOULD carry
+new data when possible.  Implementers MAY use alternate strategies for
+determining the content of probe packets.  An implementation could use new data

>From "An implementation could" on, this paragraph seems quite speculative, and the first sentence is redundant to the above setence "A probe packet SHOULD carry new data when possible."

-A packet sent on an RTO timer MUST NOT be blocked by the sender's congestion
-controller. A sender MUST however count these packets as being in flight, since
-this packet adds network load without establishing packet loss.
+If an ACK frame is received that newly acknowledges only those packets that were
+sent after a PTO timer expiration, all unacknowledged packets with lower packet
+numbers MUST be marked as lost.

This will happen naturally due to loss detection I believe.

-Acknowledgement or loss of tail loss probes are treated like any other packet.
+Probe packets MUST NOT be blocked by the congestion controller.  A sender MUST
+however count these packets as being additionally in flight, since these packets
+adds network load without establishing packet loss.  Note that sending probe
+packets might cause the sender's estimated bytes in flight to exceed the

```suggestion
packets might cause the sender's bytes in flight to exceed the
```

-Acknowledgement or loss of tail loss probes are treated like any other packet.
+Probe packets MUST NOT be blocked by the congestion controller.  A sender MUST
+however count these packets as being additionally in flight, since these packets
+adds network load without establishing packet loss.  Note that sending probe
+packets might cause the sender's estimated bytes in flight to exceed the
+sender's congestion window until an acknowledgement is received that establishes

```suggestion
congestion window until an acknowledgement is received that establishes
```

-Acknowledgement or loss of tail loss probes are treated like any other packet.
+Probe packets MUST NOT be blocked by the congestion controller.  A sender MUST
+however count these packets as being additionally in flight, since these packets
+adds network load without establishing packet loss.  Note that sending probe
+packets might cause the sender's estimated bytes in flight to exceed the
+sender's congestion window until an acknowledgement is received that establishes

Also, I think this would read better as "until packets are acknowledged or lost."

Agreed, this is a potentially substantial change.

 ~~~

I'd suggest we keep a "you can not do the CWND reduction on the first or 2nd PTO" to keep behavior more similar to what QUIC has now.

-- 
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/2114#pullrequestreview-183923684

---

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 15:00:15 -0800

@janaiyengar pushed 1 commit.

903b4e59eff7bcd3b7cb7c696221096ec576f449  ian/subodh/nick comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/c4f38d214557d97d746d5f090100b01d2b4c1297..903b4e59eff7bcd3b7cb7c696221096ec576f449

---

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 15:00:35 -0800

janaiyengar commented on this pull request.

This isn't a normative statement. I can use should, but SHOULD isn't warranted here.

I don't understand your suggestion, but I've rewritten this, PTAL

I've rephrased it. The idea is to recommend one strategy but allow experimentation.

This is what we had earlier, I'm just keeping this text. I think we should simplify this response to a single loss-detection response, but I'd like to do that next.

-kMinTLPTimeout:

This is what we used to do with kMinTLP and kMinRTO. There's a forward reference where it's used earlier to this section.

I don't like this use of bytes in flight... it doesn't sound right. If "bytes in flight" is being used as a var name, we should hyphenate and use it. Otherwise, as an English sentence, it doesn't read well. I'll change it because I think we need to make this change more globally in this doc.

I prefer "established as lost" to "packets are lost".  Packets are lost when they're dropped in the network, and the sender establishes loss later.

Only if there was in fact loss previously and if the PTO is valid. If the PTO is spurious, which is equivalent to when TLP is triggered but is spurious, the outcome is the same -- no action.

-- 
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/2114#discussion_r240817198

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 Dec 2018 15:04:58 -0800

martinthomson commented on this pull request.

Haven't reviewed, but these might help

> @@ -297,7 +297,7 @@ If a packet is lost, the QUIC transport needs to recover from that loss, such
 as by retransmitting the data, sending an updated frame, or abandoning the
 frame.  For more information, see Section 13.2 of {{QUIC-TRANSPORT}}.
 
-## Ack-based Detection
+## Ack-based Detection {#ack-loss-detection}

```suggestion
## ACK-based Detection {#ack-loss-detection}
```

-* If RTO ({{rto}}) is earlier, schedule a TLP in its place. That is,
-  PTO SHOULD be scheduled for min(RTO, PTO).
+PTO = max(smoothed_rtt + 4*rttvar + max_ack_delay, kGranularity)

```suggestion
~~~
PTO = max(smoothed_rtt + 4*rttvar + max_ack_delay, kGranularity)
~~~
```

>       else:
-       // RTO.
-       if (rto_count == 0)
-         largest_sent_before_rto = largest_sent_packet
+       // PTO.
+       if (pto_count == 0)

```suggestion
       if (pto_count == 0):
```

-- 
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/2114#pullrequestreview-183946331

---

From: Martin Thomson <[email-masked]>
Date: Tue, 11 Dec 2018 15:17:58 -0800

Does this have an issue?

-- 
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/2114#issuecomment-446399948

---

From: ianswett <[email-masked]>
Date: Tue, 11 Dec 2018 19:07:39 -0800

Looking much better.

-A packet sent at the tail is particularly vulnerable to slow loss detection,
-since acks of subsequent packets are needed to trigger ack-based detection. To
-ameliorate this weakness of tail packets, the sender schedules a timer when the
-last ack-eliciting packet before quiescence is transmitted. Upon timeout,
-a Tail Loss Probe (TLP) packet is sent to evoke an acknowledgement from the
-receiver.
+A Probe Timeout (PTO) triggers a probe packet when ack-eliciting data is in
+flight but an acknowledgement is not received within the expected period of
+time.  A PTO enables a connection to recover from loss of tail packets or acks,
+where acks of subsequent packets are not available to trigger ack-based loss

This sentence is long and the second half is awkward.  I'd either split it into two sentences or just drop the second half.

+There is no requirement on the clock granularity. The PTO value MUST be set to

I find it weird you talk about not having a requirement on clock granularity, but don't discuss what it is more than the name.

Also, is this clock granularity or alarm granularity?  I think you want alarm granularity?

-* When the final TLP packet is sent, the RTO period is set to max(SRTT +
-  4*RTTVAR + MaxAckDelay, kMinRTOTimeout)
+Consecutive PTO periods increase exponentially, and as a result, connection
+recovery latency increases exponentially as packets continue to be dropped in
+the network.  Sending two packets on PTO expiration increases resilience to
+packet drop in both directions, thus reducing the probability of consecutive PTO

How does it help in both directions?  If the two packets are sent at about the same time, wouldn't they be acknowledged in a single ACK?  I guess that depends upon the details of what "immediately" means for ACKs with newly missing packets?

-* When an RTO timer expires, the RTO period is doubled.
+Probe packets sent on a PTO MUST be ack-eliciting.  A probe packet SHOULD carry
+new data when possible.  A probe packet MAY carry retransmitted unacknowledged
+data when new data is unavailable, when flow control does not permit new data to
+be sent, or to opportunistically reduce loss recovery delay.  Implementers MAY
+use alternate strategies for determining the content of probe packets, including
+sending new or retransmitted data based on the application's priorities.

Thanks, this is much better!

-QUIC also diverges from TCP by including MaxAckDelay in the RTO period. Since
-QUIC corrects for this delay in its SRTT and RTTVAR computations, it is
-necessary to add this delay explicitly in the TLP and RTO computation.
+Delivery or loss of packets in flight is established when an acknowledgement is

Should this be "an ACK"?  We typically talk about an acknowledgement as referencing a single packet.

-QUIC also diverges from TCP by including MaxAckDelay in the RTO period. Since
-QUIC corrects for this delay in its SRTT and RTTVAR computations, it is
-necessary to add this delay explicitly in the TLP and RTO computation.
+Delivery or loss of packets in flight is established when an acknowledgement is
+received that newly acknowledges one or more packets.  An endpoint might receive
+an acknowledgement after multiple consecutive PTO periods.

I feel like you're trying to say something more with this last sentence, but I'm not sure what?

> @@ -802,7 +772,7 @@ Pseudocode for SetLossDetectionTimer follows:
         timeout = 2 * kInitialRtt
       else:
         timeout = 2 * smoothed_rtt
-      timeout = max(timeout, kMinTLPTimeout)
+      timeout = max(timeout, granularity)

```suggestion
      timeout = max(timeout, kGranularity)
```

>      timeout =
       smoothed_rtt + 4 * rttvar + max_ack_delay
-    timeout = max(timeout, kMinRTOTimeout)
-    timeout = timeout * (2 ^ rto_count)
-    if (tlp_count < kMaxTLPs):
-      // Tail Loss Probe
-      tlp_timeout = max(1.5 * smoothed_rtt
-                        + max_ack_delay, kMinTLPTimeout)
-      timeout = min(tlp_timeout, timeout)
+    timeout = max(timeout, granularity)

```suggestion
    timeout = max(timeout, kGranularity)
```

-- 
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/2114#pullrequestreview-183994915

---

From: janaiyengar <[email-masked]>
Date: Tue, 11 Dec 2018 20:51:44 -0800

60cb630433a4d8633173e4ed1f6ded7a765b7467  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/903b4e59eff7bcd3b7cb7c696221096ec576f449..60cb630433a4d8633173e4ed1f6ded7a765b7467

---

From: ianswett <[email-masked]>
Date: Wed, 12 Dec 2018 10:35:36 +0000 (UTC)

If you have a lot of tails(ie: HTTP traffic), then it doesn't take too many packet losses to end up hitting this.  TLP was targeted partially at cases like that, and I think you'll find changing it will produce measurable performance regressions in web traffic.  We can do something else in a follow-up, but as I said before, I'd prefer text saying SHOULD consider an acknowledgement of the first PTO as standard packet loss, not extreme congestion, and treat it as such.

-- 
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/2114#discussion_r240960383

---

From: ianswett <[email-masked]>
Date: Wed, 12 Dec 2018 02:39:59 -0800

Specific suggestion in pseudocode to prevent a single packet loss from leading to massive CWND reduction.

> @@ -722,14 +695,13 @@ Pseudocode for OnAckReceived and UpdateRtt follow:
       // Find the smallest newly acknowledged packet
       smallest_newly_acked =
         FindSmallestNewlyAcked(newly_acked_packets)
-      // If any packets sent prior to RTO were acked, then the
-      // RTO was spurious. Otherwise, inform congestion control.
-      if (rto_count > 0 &&
-            smallest_newly_acked > largest_sent_before_rto):
-        OnRetransmissionTimeoutVerified(smallest_newly_acked)
+      // If any packets sent prior to PTO were acked, then the
+      // PTO was spurious. Otherwise, inform congestion control.
+      if (pto_count > 0 &&
+            smallest_newly_acked > largest_sent_before_pto):

You could define a kNumCongestivePtoPackets here and add it here and then define it above to be the number of PTOs you will treat as standard packet loss, and not extreme congestion.

-- 
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/2114#pullrequestreview-184111080

---

From: Martin Thomson <[email-masked]>
Date: Wed, 12 Dec 2018 14:47:17 -0800

What is "extreme congestion" ?

-- 
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/2114#discussion_r241213835

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 21:05:55 +0000 (UTC)

I've changed this, but also added some text at the definition of granularity.

I'll remove "in both directions" -- more trouble than it's worth.

I think this is not required. There's mention of consecutive PTOs later on where it's more relevant.

@martinthomson : @ianswett is trying to capture the difference in congestion response that the current loss recovery spec has for TLP vs. RTO. The idea is to say that for a couple of PTOs, there will be a smaller reduction in cwnd, and after that, the cwnd will be collapsed. I can write that up.

I'm going to simplify this further. I think that having a verification is unnecessary; the behavior ends up being the same whether a PTO is verified or not. I can retain this notion of reducing the cwnd differently for the first 2 PTOs and then collapsing it to min_cwnd after that.

-- 
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/2114#discussion_r241557699

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 13:06:16 -0800

b2c3fd7e2119c3694b27bc53067f181028f84e5b  comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/4b4eb6415fe57a74dc8eeea8594874ff24fd0209..b2c3fd7e2119c3694b27bc53067f181028f84e5b

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 14:05:13 -0800

ef864826a2386b3b6048e9f216a6913886eab78f  Unify post-PTO behavior and allow 2 PTOs before collapsing cwnd

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/b2c3fd7e2119c3694b27bc53067f181028f84e5b..ef864826a2386b3b6048e9f216a6913886eab78f

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 14:09:21 -0800

4f88a7ed9ba50b10d3abc9c00fefaa9877d4ccdd  collapse cwnd after 3 PTOs not 2

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/ef864826a2386b3b6048e9f216a6913886eab78f..4f88a7ed9ba50b10d3abc9c00fefaa9877d4ccdd

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 14:20:05 -0800

I've updated the description text and the PR quite a bit based on the comments. Specifically, I've removed the verification of a timeout, since the behavior with and without it was the same. I've also added a clause that allows for the cwnd to be collapsed to min cwnd only after 2 PTOs, to retain parity with TLP/RTO behavior.

I'll open an issue to discuss the issues this PR addresses. We should have discussion there.

-- 
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/2114#issuecomment-447141247

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 18:23:55 -0800

pravb commented on this pull request.

Worthy change. Really nice that we can get rid of minRTO. Adding some comments.

-A packet sent at the tail is particularly vulnerable to slow loss detection,
-since acks of subsequent packets are needed to trigger ack-based detection. To
-ameliorate this weakness of tail packets, the sender schedules a timer when the
-last ack-eliciting packet before quiescence is transmitted. Upon timeout,
-a Tail Loss Probe (TLP) packet is sent to evoke an acknowledgement from the
-receiver.
+A Probe Timeout (PTO) triggers a probe packet when ack-eliciting data is in
+flight but an acknowledgement is not received within the expected period of
+time.  A PTO enables a connection to recover from loss of tail packets or acks.
+The PTO algorithm used in QUIC implements the reliability functions of Tail Loss
+Probe {{?TLP=I-D.dukkipati-tcpm-tcp-loss-probe}}, RTO {{?RFC5681}} and F-RTO

TLP is now subsumed by RACK draft so we should cite that instead.

-The sender typically has incurred a high latency penalty by the time an RTO
-timer expires, and this penalty increases exponentially in subsequent
-consecutive RTO events. Sending a single packet on an RTO event therefore makes
-the connection very sensitive to single packet loss. Sending two packets instead
-of one significantly increases resilience to packet drop in both directions,
-thus reducing the probability of consecutive RTO events.
+Probe packets sent on a PTO MUST be ack-eliciting.  A probe packet SHOULD carry
+new data when possible.  A probe packet MAY carry retransmitted unacknowledged
+data when new data is unavailable, when flow control does not permit new data to
+be sent, or to opportunistically reduce loss recovery delay.  Implementers MAY

"Implementations MAY" is better

-kMinRTOTimeout:
-:  Minimum time in the future an RTO timer may be set for. The RECOMMENDED
-   value is 200ms.
+: Timer granularity. This is a system-dependent value.  However, implementations
+  SHOULD use a value no smaller than 1ms.
 
 kDelayedAckTimeout:
 : The length of the peer's delayed ack timer. The RECOMMENDED value is 25ms.
 
 kInitialRtt:

I know this value wasn't changed in this PR but 100 ms seems too aggressive for many links on the Internet. Any particular reason to go lower than 300 msec outside of intra-DC scenarios? Should we add text that implementations MAY use 2 * cached path RTT if platform support is available?

-When retransmissions are sent due to a retransmission timeout timer, no change
-is made to the congestion window until the next acknowledgement arrives.  The
-retransmission timeout is considered spurious when this acknowledgement
-acknowledges packets sent prior to the first retransmission timeout.  The
-retransmission timeout is considered valid when this acknowledgement
-acknowledges no packets sent prior to the first retransmission timeout.  In this
-case, the congestion window MUST be reduced to the minimum congestion window and
-slow start is re-entered.
+If two consecutive PTOs have occurred (pto_count is at least 3), the network is

I am not sure why pto_count needs to be 3? Shouldn't it be 2 after two PTOs?

-- 
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/2114#pullrequestreview-184945843

---

From: ianswett <[email-masked]>
Date: Thu, 13 Dec 2018 19:05:49 -0800

Thanks for the update, this looks very close.  I'm concerned about the gap between number of PTOs that have occurred vs the number that were lost, but possibly it doesn't matter much in practice?

> @@ -429,19 +429,20 @@ a subsequent connection attempt to the server.
 
 A Probe Timeout (PTO) triggers a probe packet when ack-eliciting data is in
 flight but an acknowledgement is not received within the expected period of
-time.  A PTO enables a connection to recover from loss of tail packets or acks,
-where acks of subsequent packets are not available to trigger ack-based loss
-detection.  The PTO algorithm used in QUIC implements the reliability functions
-of Tail Loss Probe {{?TLP=I-D.dukkipati-tcpm-tcp-loss-probe}}, RTO {{?RFC5681}}
-and F-RTO algorithms for TCP {{?RFC5682}}, and the computation is based on TCP's
+time.  A PTO enables a connection to recover from loss of tail packets or acks.
+The PTO algorithm used in QUIC implements the reliability functions of Tail Loss
+Probe {{?TLP=I-D.dukkipati-tcpm-tcp-loss-probe}}, RTO {{?RFC5681}} and F-RTO
+algorithms for TCP {{?RFC5682}}, and the computation is based on TCP's

"the computation" is vague.  How about "the timeout computation"?

> @@ -473,8 +474,7 @@ consecutive PTO expiration due to a single packet loss.
 Consecutive PTO periods increase exponentially, and as a result, connection
 recovery latency increases exponentially as packets continue to be dropped in
 the network.  Sending two packets on PTO expiration increases resilience to
-packet drop in both directions, thus reducing the probability of consecutive PTO
-events.
+packet drop, thus reducing the probability of consecutive PTO events.

packet drops

The original implementation and algorithms preceded RACK, so the benefit of referencing TLP is that it's more specific and time-relevant.  But if RACK was an RFC, then we would definitely prefer that.  Either works for me.

If you want to use granularity, then you need to define it above.  Otherwise, use kGranularity as suggested.

+1, I'm confused.  Also, this should be more about the number of PTOs that were lost than the number sent.   ie: I could send 3 PTOs, then the network returns to normal, no packets were lost, and the connection continues.

-- 
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/2114#pullrequestreview-184953216

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 19:32:29 -0800

Thanks for you comments! Responses inline.

I think the function we're talking about is TLP, though I'm happy to add and cite RACK separately somewhere. I'll do that separately.

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

Would you mind opening a separate issue for this? I'd argue for 100ms, but I don't want to do it in this PR. FWIW, I think the cost of an early timeout is not high, so having this be aggressive seems fine to me.

This is based on the number of timeouts that happened, not based on packets sent.  We want cwnd collapse at the equivalent of the first RTO's occurrence. This is the third timeout (2 TLP timeouts and then 1 RTO), which means pto_count is at least 3. Or am I doing this math wrong?

-- 
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/2114#pullrequestreview-184957703

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 19:32:56 -0800

9267dbbdc9ec2d9309923538d3e8adea547f56b8  pravb and ian comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/4f88a7ed9ba50b10d3abc9c00fefaa9877d4ccdd..9267dbbdc9ec2d9309923538d3e8adea547f56b8

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 19:38:57 -0800

a9041b492081fb8401d1f259d06dd14151368a46  kGranularity not granularity

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/9267dbbdc9ec2d9309923538d3e8adea547f56b8..a9041b492081fb8401d1f259d06dd14151368a46

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 19:51:02 -0800

dd05532b6d889ef91ee2e22c6fd1dff03b6b66e7  add RACK reference to TLP

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/a9041b492081fb8401d1f259d06dd14151368a46..dd05532b6d889ef91ee2e22c6fd1dff03b6b66e7

---

From: ianswett <[email-masked]>
Date: Thu, 13 Dec 2018 19:54:49 -0800

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

Agreed, that's a separable issue, which was previously raised and then I believe closed.

-- 
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/2114#discussion_r241638123

---

From: ianswett <[email-masked]>
Date: Thu, 13 Dec 2018 19:56:00 -0800

ianswett approved this pull request.

-- 
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/2114#pullrequestreview-184961594

---

From: ianswett <[email-masked]>
Date: Thu, 13 Dec 2018 20:06:49 -0800

> @@ -1157,6 +1095,9 @@ window.
        congestion_window *= kLossReductionFactor
        congestion_window = max(congestion_window, kMinimumWindow)
        ssthresh = congestion_window
+       // Collapse congestion window if persistent congestion
+       if (pto_count > 2):

2 is a magic number.  Previously we said you SHOULD have 2 TLPs, but you could have fewer.  I think it makes sense to define this as a constant and explain its reason for existing there?

-- 
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/2114#pullrequestreview-184962887

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 13 Dec 2018 20:09:05 -0800

kazuho commented on this pull request.

To me it seems that "pto_count is at least 3" is correct, however "If two consecutive PTOs have occurred" is incorrect and needs to be changed to "If three...".

-- 
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/2114#discussion_r241639404

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 22:36:29 -0800

TLP is an expired draft in 2013 and RACK is standards track and active. Linux, FreeBSD and Windows are all implementing the RACK draft and TLP as specified in the RACK draft. I would just cite the RACK draft. There is a chance it may got into last call within the next two IETFs.

-- 
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/2114#discussion_r241655332

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 22:39:35 -0800

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

Ok I will open a separate issue. This will cause unnecessary timeouts for CI and 0-RTT data on many Internet paths. And pretty much always on satellite links. If we want to pick a magic number then let's pick 200 msec or 300 msec (the historical minrto values for Linux and Windows respectively). 

-- 
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/2114#discussion_r241655813

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 22:47:48 -0800

IIRC Ian was going to get data on whether we need 2 TLPs or just one is sufficient. FWIW we only do one TLP in Windows TCP. Yes at the least please fix as Kazuho says: "if three consecutive PTOs have occurred"

-- 
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/2114#discussion_r241657054

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 22:48:51 -0800

I commented on the thread above this. Yes we should settle on one or two TLPs. I personally prefer one but unfortunately I don't have data to say if two is better since Windows TCP has been doing just one TLP.

-- 
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/2114#discussion_r241657223

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 22:56:08 -0800

pravb approved this pull request.

Added a few more comments. 

-- 
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/2114#pullrequestreview-184985891

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 13 Dec 2018 23:02:34 -0800

kazuho approved this pull request.

LGTM modulo 
https://github.com/quicwg/base-drafts/pull/2114#discussion_r241639404.

-- 
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/2114#pullrequestreview-184987108

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 13 Dec 2018 23:08:20 -0800

> -
-QUIC includes MaxAckDelay in all probe timeouts, because it assumes the ack
-delay may come into play, regardless of the number of ack-eliciting
-packets in flight. TCP's TLP assumes if at least 2 ack-eliciting packets are
-in flight, acks will not be delayed.
-
-A PTO value of at least 1.5*SRTT ensures that the ACK is overdue.  The 1.5 is
-based on {{?TLP}}, but implementations MAY experiment with other constants.
-
-To reduce latency, it is RECOMMENDED that the sender set and allow the TLP timer
-to fire twice before setting an RTO timer. In other words, when the TLP timer
-expires the first time, a TLP packet is sent, and it is RECOMMENDED that the TLP
-timer be scheduled for a second time. When the TLP timer expires the second
-time, a second TLP packet is sent, and an RTO timer SHOULD be scheduled {{rto}}.
+~~~
+PTO = max(smoothed_rtt + 4*rttvar + max_ack_delay, kGranularity)

Sorry just noticed another difference. If peer advertises max_ack_delay as 0 (which seems to be legal), then first PTO becomes Srtt + 4 * RttVar, whereas in TCP the first PTO is 2* Srtt. If variance is low this may be too aggressive? Should we also do PTO = max (PTO, 2 *Srtt) ?

-- 
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/2114#pullrequestreview-184988159

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 23:28:20 -0800

8e9e1785dc8b5a0d793fc986150f88108d587f94  add kPersistentCongestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/dd05532b6d889ef91ee2e22c6fd1dff03b6b66e7..8e9e1785dc8b5a0d793fc986150f88108d587f94

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 23:28:56 -0800

I've added a constant (kPersistentCongestion) and described it. PTAL.

Ah, right. Ok, fixed. I'll leave it at 3 for now, and I've turned it into a constant, kPersistentCongestion. We can discuss the value in a separate issue.

Ok, added constant and explained it.

-- 
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/2114#pullrequestreview-184987461

---

From: Subodh Iyengar <[email-masked]>
Date: Thu, 13 Dec 2018 23:29:18 -0800

i think this condition will bring back the issue that i raised previously of a longer timeout.

Consider this case:

1 [acked], 2, 3[pto], 4[pto], 5 [pto]

-> ack 5

now we get into a state

1 [acked], 2 [lost], 3[outsatding], 4[outstanding], 5 [acked]

since minCwnd = 2

you end up with consuming the cwnd fully still and will have to wait until another pto to get out of this case.

The previous logic intentionally cleared all packets < rto_verified packet to avoid this which I had raised in a previous issue and is intentional

-- 
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/2114#discussion_r241663171

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 23:43:06 -0800

I'll push back against this. I like the simplicity of having a single computation that includes rttvar. What is the rationale in TCP for using 2xSRTT when RTTVAR is available? If variance is low, that still seems like the right thing to do. Why inflate the PTO when you don't need to?

FWIW, the first PTO is akin to the first TLP. The second PTO would be backed off, so you will automatically get 2 x (SRTT + 4xRTTVAR).

-- 
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/2114#discussion_r241665530

---

From: janaiyengar <[email-masked]>
Date: Thu, 13 Dec 2018 23:49:08 -0800

@siyengar : In this case, by the time 5 is acked, 3 and 4 should be marked as lost by the time-threshold loss detection mechanism, since they're going to be > 1.125 x SRTT in the past.

-- 
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/2114#discussion_r241666519

---

From: MikkelFJ <[email-masked]>
Date: Fri, 14 Dec 2018 02:51:33 -0800

mikkelfj commented on this pull request.

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

Here are 10 ping packets from Denmark to China

10 packets transmitted, 10 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 211.218/350.768/593.271/149.383 ms

-- 
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/2114#discussion_r241714981

---

From: MikkelFJ <[email-masked]>
Date: Fri, 14 Dec 2018 02:55:29 -0800

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

And here from Denmark to New York, which is basically a direct sea cable link.

10 packets transmitted, 10 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 103.982/104.505/105.343/0.475 ms

-- 
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/2114#discussion_r241716116

---

From: MikkelFJ <[email-masked]>
Date: Fri, 14 Dec 2018 11:02:34 +0000 (UTC)

-kMinRTOTimeout:
 
 kDelayedAckTimeout:
 
 kInitialRtt:

And Denmark New Zealand

10 packets transmitted, 10 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 324.042/454.764/672.469/122.684 ms

-- 
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/2114#discussion_r241718059

---

From: ianswett <[email-masked]>
Date: Fri, 14 Dec 2018 06:31:42 -0800

> @@ -1065,6 +1004,16 @@ kLossReductionFactor:
 : Reduction in congestion window when a new loss event is detected.
   The RECOMMENDED value is 0.5.
 
+kPersistentCongestion:
+: Number of consecutive PTOs after which network is considered to be
+  experiencing persistent congestion.  The rationale for this threshold is to
+  enable a sender to use initial PTOs for aggressive probing, similar to Tail
+  Loss Probe (TLP) in TCP {{TLP}} {{RACK}}.  Once the number of consecutive PTOs
+  reaches this threshold - that is, persistent congestion is established - the
+  sender responds by collapsing its congestion window, similar to a
+  Retransmission Timeout (RTO) in TCP {{RFC5681}}.  The RECOMMENDED value is 3,

The pseudocode below decreases CWND to kMinimumWindow, which is 2 packets.  I think sticking with that makes sense, at least for now.  It also requires less text to justify.

-- 
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/2114#pullrequestreview-185132070

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 14 Dec 2018 09:07:11 -0800

Quoting RACK draft:

Aiming for a PTO value of 2*SRTT allows a sender to wait long enough
   to know that an ACK is overdue.  Under normal circumstances, i.e. no
   losses, an ACK typically arrives in one SRTT.  But choosing PTO to be
   exactly an SRTT is likely to generate spurious probes given that
   network delay variance and even end-system timings can easily push an
   ACK to be above an SRTT.  We chose PTO to be the next integral
   multiple of SRTT.

   Similarly, network delay variations and end-system processing
   latencies and timer granularities can easily delay ACKs beyond
   2*SRTT, so senders SHOULD add at least 2ms to a computed PTO value
   (and MAY add more if the sending host OS timer granularity is more
   coarse than 1ms).

-- 
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/2114#discussion_r241825088

---

From: Marten Seemann <[email-masked]>
Date: Fri, 14 Dec 2018 09:35:29 -0800

marten-seemann commented on this pull request.

I really like this PR, since it's a big simplification.

The one thing I don't understand is how we can get rid of the (R/P)TO verified. I think I understand @janaiyengar's response to @siyengar's comment in https://github.com/quicwg/base-drafts/pull/2114#discussion_r241663171, i.e. that when receiving the ACK for the PTO, packets sent before the PTO are declared lost because the time-threshold based loss detection kicks in. But shouldn't that also have applied to RTO verified in the first place? Why did we need RTO verified in the first place then?

-- 
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/2114#pullrequestreview-185211888

---

From: Marten Seemann <[email-masked]>
Date: Fri, 14 Dec 2018 09:43:18 -0800

I think I found the answer to my question:
Since in #1974 we make time-threshold based loss recovery mandatory (before that you could choose between packet-threshold and time-threshold), we don't need the RTO verified anymore (see #2168) anyway. This is independent of this PR, this PR just happens to fix multiple issues at the same time.

Sorry for the noise.

-- 
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/2114#issuecomment-447398702

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 11:46:20 -0800

+kPersistentCongestion:

The recommendation is 3 for kPersistentCongestion, not for the value of cwnd. I'll change the text a bit to clarify.

-- 
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/2114#discussion_r241869845

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 12:09:59 -0800

d4f0646c71a86cd25e1d1cbdfc94e384b345318d  rewording

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/8e9e1785dc8b5a0d793fc986150f88108d587f94..d4f0646c71a86cd25e1d1cbdfc94e384b345318d

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 20:23:13 +0000 (UTC)

@pravb: Thanks for the context -- I should have just looked at the RACK draft. So, to be clear, the RACK draft does not use this 2ms when WCDelAckT is used, that is, when it accounts for the worst case delayed ack timer. This is what we expect max_ack_delay to do for us, but as you note, a receiver might send 0 here. 

I think we should put the onus of this onto the receiver.  A receiver should send the worst case ack delay in max_ack_delay. A receiver that doesn't do that shoots itself in the foot by causing spurious retransmissions at the sender. We may want to add some strong wording about how a receiver should choose max ack delay, I've filed #2186 for that.

Sound reasonable?

-- 
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/2114#discussion_r241881343

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 12:26:50 -0800

@marten-seemann : Exactly. This PR builds on #1974, which started the movement in this direction. This PR fixes multiple issues at the same time because the design that comes out at the end is clearer than the one you'd get by fixing the issues piecewise.

-- 
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/2114#issuecomment-447465709

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 12:36:45 -0800

There's exactly one question that's pending to be resolved now in this PR: @pravb's question about using 2SRTT for initial PTO. I'm happy to open a new issue for that specific question if Praveen thinks we should continue that discussion, but I'd like to merge this soon, as (i) I'm hearing pushback on a detail or two but not the overall PR, which we can handle in separate issues, and (ii) this PR is fast approaching github's limit on comments (hence the unicorn tag).

-- 
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/2114#issuecomment-447472441

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 20:52:01 +0000 (UTC)

f77ffa38f3d6353fba9d9ce2660a8df75a6e63bd  minor change to kPersistentCongestionThreshold

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2114/files/d4f0646c71a86cd25e1d1cbdfc94e384b345318d..f77ffa38f3d6353fba9d9ce2660a8df75a6e63bd

---

From: ianswett <[email-masked]>
Date: Fri, 14 Dec 2018 12:54:23 -0800

Thanks Jana, looks even better.

-- 
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/2114#pullrequestreview-185281417

---

From: Nick Banks <[email-masked]>
Date: Fri, 14 Dec 2018 12:58:26 -0800

nibanks approved this pull request.

LGTM

-- 
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/2114#pullrequestreview-185282663

---

From: janaiyengar <[email-masked]>
Date: Fri, 14 Dec 2018 17:59:45 -0800

Merged #2114 into master.

-- 
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/2114#event-2028260158

---

From: Praveen Balasubramanian <[email-masked]>
Date: Sun, 16 Dec 2018 09:40:21 -0800

That's reasonable. So this is how we'd justify the deviation from TCP:
1. Include RttVar component in the computation for in network variability. 
2. Require the receiver to include the perturbation for system processing delays. So 2186 needs to add a SHOULD for adding a perturbation value on top of the actual max ack delay. I'll comment there.

-- 
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/2114#discussion_r241989226