Subject: [quicwg/base-drafts] Reset min_rtt on persistent congestion (#3927)

From: Jana Iyengar <[email-masked]>
Date: Tue, 21 Jul 2020 13:24:17 -0700

@pravb wrote the following in his WGLC review (#3919):
```
Section 5.2 "If the path's actual RTT increases, the min_rtt will not adapt to it, allowing future RTT samples that are smaller than the new RTT be included in smoothed_rtt."

Would it safer to reset min_rtt after persistent congestion is declared. This is important so we can adapt to a consistent spike in RTT as a result of a path change.
```

Splitting this off from #3919 as a potential design 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/issues/3927

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 21 Jul 2020 13:25:03 -0700

This seems like a reasonable thing to do.

-- 
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/issues/3927#issuecomment-662087679

---

From: ianswett <[email-masked]>
Date: Tue, 21 Jul 2020 14:05:05 -0700

I'd support a can or a MAY, as an example when you might reset min_rtt, but I don't think we need a SHOULD here.

-- 
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/issues/3927#issuecomment-662105699

---

From: Praveen Balasubramanian <[email-masked]>
Date: Tue, 21 Jul 2020 18:04:00 -0700

min_rtt is currently only used for filtering unexpectedly large ack_delay's reported by the peer. If the path RTT truly inflates then the peer will be able to report much larger ack_delay values and get a much lower RTT sample accepted. But I don't think this is a security problem in practice. The only other consideration is that in future QUIC implementations may reuse this computed value for other purposes like delay based congestion control. It's hard to make a judgement call on how important it is to reset min_rtt. I guess I'd be happy with a SHOULD but I can live with a MAY.

-- 
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/issues/3927#issuecomment-662182651

---

From: ianswett <[email-masked]>
Date: Tue, 21 Jul 2020 18:09:18 -0700

Jana made a good argument that persistent congestion could be declared due to an overly small RTT, so resetting min_rtt could avoid an endpoint repeatedly declaring persistent congestion.  That being said, the RTT samples also need to be too low to spuriously declare persistent congestion, so this is increasingly sounding like an attack scenario, rather than having a poor estimate of min_rtt.

I think a SHOULD is Ok now, but I'd like to state the best rationale we have for why it's a SHOULD and neither a MUST or a MAY.

-- 
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/issues/3927#issuecomment-662184049

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jul 2020 16:26:12 -0700

I think there's one tricky point in resetting the min_rtt after handshake confirmation. That is that all the packets can be acked with delays.

IIRC, one of the reasons we were happy with using min_rtt for capping latest_rtt is that Initial and Handshake packets do not have ack_delays.

If we are to suggest that an endpoint MAY reset min_rtt when persistent congestion is declared, I think we should consider the risk of not having good estimate of min_rtt, as we would have during the handshake.

-- 
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/issues/3927#issuecomment-662746698

---

From: Martin Thomson <[email-masked]>
Date: Wed, 22 Jul 2020 21:44:51 -0700

I don't see that as a risk here.  min_rtt doesn't consider any discretionary delays.

-- 
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/issues/3927#issuecomment-662815868

---

From: Marten Seemann <[email-masked]>
Date: Wed, 22 Jul 2020 21:47:03 -0700

@martinthomson That's exactly the problem. `min_rtt` will be inflated by `max_ack_delay`, unless and until the peer sends an immediate acknowledgement. And until that happens, `smoothed_rtt` will as well.

-- 
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/issues/3927#issuecomment-662816333

---

From: Martin Thomson <[email-masked]>
Date: Wed, 22 Jul 2020 21:50:48 -0700

I think that's OK.  In practice, immediate ACKs do happen, and even at low rates, this should resolve to a "true" minimum.  And if it doesn't, then you have worse performance than you might had otherwise.  You have to have a pretty poor network (or a terrible ACK generation regime) for min_rtt not to eventually resolve to a useful minimum.

Explaining this risk is fine, but the endpoint that does this made the choice to throw out min_rtt and that's just a consequence of 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/issues/3927#issuecomment-662817091

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jul 2020 22:00:26 -0700

On one hand I agree that we can expect to see immediate ACKs, especially when HTTP/3 is used.

At the same time, I am not sure if resetting min_rtt when observing persistent congestion is better than other cases where we might reset min_rtt, but that we do not currently mention.

Change of IP_TTL could be one of the examples in this regard (IIRC @larseggert pointed out that we do not use that value for detecting path changes).

To summarize, I think talking about resetting min_rtt might be fine, though I am not sure if that discussion should be tied to persistent 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/issues/3927#issuecomment-662818940

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 28 Jul 2020 20:36:33 -0700

> To summarize, I think talking about resetting min_rtt might be fine, though I am not sure if that discussion should be tied to persistent congestion.

To give a specific example, one problem that I know with the current definition of min_rtt is that it cannot detect and correct RTT estimate of a congested path.

The sole purpose of min_rtt is to discard excessive ack-delays being reported by the receiver. This works by capping the lower bound of adjusted_rtt to min_rtt. The problem here is that min_rtt is a value that is typically collected when the path is almost idle. Once the path becomes congested, the true RTT becomes much greater than min_rtt.

For instance, if RTT-on-idle is 10ms and if RTT-on-congestion is 50ms, min_rtt would be 10ms, and therefore cannot be used to correct RTT estimates when the peer reports an excessive ack_delay of, say 25ms. The RTT estimate would become 25ms in such case.

I tend to think that this might be a bigger problem of the original issue, and therefore that we should state that an endpoint can periodically re-collect min_rtt estimates, if we are to make a change to the current definition of min_rtt.

-- 
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/issues/3927#issuecomment-665411499