Subject: [quicwg/base-drafts] It's unclear if persistent congestion is a per-PN-space property (#3939)

From: Kazuho Oku <[email-masked]>
Date: Tue, 21 Jul 2020 21:49:05 -0700

My understanding is that it is *not*, but it's unclear.

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

---

From: Marten Seemann <[email-masked]>
Date: Tue, 21 Jul 2020 22:24:24 -0700

It can't be, since the congestion window is a global property, and declaring persistent congestion results in a collapse of the congestion window.

-- 
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/3939#issuecomment-662248264

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 22 Jul 2020 20:56:51 -0700

It shouldn't be, because congestion is a network/connection-level property. That said, @marten-seemann and I were discussing this offline, and I'm now coming to terms with the fact that it's hard to do this globally across packet number spaces.  

Basically we have no current model to track packet transmissions across packet number spaces, and implementations do not typically do this either. I think this is hard to do across PN spaces... or at least, I've not been able to figure out a reliable way to do it. 

I'm now thinking that we might want to disable persistent congestion until the handshake is confirmed. That is a departure from TCP, but I might argue that is still ok. In practice, we still have a congestion window reduction that occurs on losses in this period. Also, in practice, since the handshake will involve only a few packets in flight, losses should cause the cwnd to reduce to a small enough value.

Thoughts?

-- 
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/3939#issuecomment-662806831

---

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

@janaiyengar 
> Basically we have no current model to track packet transmissions across packet number spaces, and implementations do not typically do this either. I think this is hard to do across PN spaces... or…

I think that it is actually easy; see https://github.com/quicwg/base-drafts/pull/3889#discussion_r459160599.

-- 
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/3939#issuecomment-662807227

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 22 Jul 2020 21:01:38 -0700

(@kazuho: Could you move the discussion on that from the PR to this 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/3939#issuecomment-662807663

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jul 2020 21:05:02 -0700

@janaiyengar Sure. The following is what was written in https://github.com/quicwg/base-drafts/pull/3889#discussion_r459160599:

One trivial way of implementing `InPersistentCongestion` is as follows:
* Retain the entries in sentmap (the list of packets being sent) for at least 3 PTO.
* When receiving an acknowledgement, mark a hole in the sentmap for the packet being acked.
* When a loss is detected in a packet number space, check if that loss spans across more than 3 PTO, by traversing the sentmap. Then, if did, check the sentmap of other packet number spaces (by traversing through those sentmaps) to see if any ACK-eliciting packet sent in that period has been acked.

This approach might sound too trivial, as it is O(N) where N is the number of packets being lost (or in case there are multiple PN spaces in action, the number of inflight packets in other PN spaces). But in practice, I think this approach is sufficient in terms of performance, because N would be like 3 for the current packet number space, and because we would not be sending that many packets on Initial and Handshake packet number spaces.

-- 
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/3939#issuecomment-662808337

---

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

In #3889, @marten-seemann states:
>> When receiving an acknowledgement, mark a hole in the sentmap for the packet being acked.
> 
> I don't think that's trivial. Creating a "hole" is extra state that has to be cleaned up at some point.

It does not require additional state.

In QUIC, there is an expectation that packet numbers would be used sequentially, starting from zero. Therefore, a "gap" can be detected when iterating through the sentmap and if the packet number increases (or decreases) by more than one.

Note that endpoints are recommended to _treat receipt of an acknowledgment for a packet it did not send as a connection error of type PROTOCOL_VIOLATION, if it is able to detect the condition_ ([transport draft; section 13.1](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#section-13.1-3)). Therefore, when intentionally skipping a packet number, it is a good idea to have that recorded in the sentmap, and close the connection when the peer acks that skipped packet number.

-- 
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/3939#issuecomment-662811465

---

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

> * When receiving an acknowledgement, mark a hole in the sentmap for the packet being acked.

I don't think this is trivial, quite the opposite actually. A "hole" would be extra state (note that just the absence of a packet at a certain PN does not constitute a hole, since you could've skipped that PN to mitigate the optimistic ACK attack), and that additional state has to be cleaned up.

In my implementation, when a packet is acknowledged, I know I can delete it and get rid of all state associated with it. Deviating from this, just to be able to do do persistent congestion detection across packet number spaces, seems like a lot of hassle to me.
As @janaiyengar described, any packet loss during the handshake will half the congestion window, and with #3943, it will even reduce the congestion window to half the bytes in flight (which will commonly be less than the cwnd during the handshake). Simple Reno-style congestion control will therefore lead to behavior that is very similar to actually doing persistent congestion detection across packet number spaces, without all the additional complexity.

-- 
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/3939#issuecomment-662811684

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 22 Jul 2020 21:53:40 -0700

There was a comment reorder.

@marten-seemann I think your question has been answered in my previous comment.

@janaiyengar Going back to the spec discussion, even if it is the case that _some_ implementations might have difficulty in implementing persistent congestion detection covering multiple packet number spaces, that alone does not mean that our recommendation should deviate from what TCP does.

I would very much prefer phrasing the discussion around _if_ we should have a MAY allowing endpoints skip persistent congestion detection 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/3939#issuecomment-662817609

---

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

> @marten-seemann I think your question has been answered in my previous comment.

There are different ways to implement packet number skipping for optimistic ACK defense, which don't involve creating fake packets in the sentmap. Those ways would make it impossible to use the "hole method" without creating additional state for already acknowledged packets.

> @janaiyengar Going back to the spec discussion, even if it is the case that some implementations might have difficulty in implementing persistent congestion detection covering multiple packet number spaces, that alone does not mean that our recommendation should deviate from what TCP does.

No matter what we do, our recommendation will deviate from what TCP does. Going back to the example you posted in https://github.com/quicwg/base-drafts/pull/3889#discussion_r458552186, the acknowledgement of the Handshake packet would already trigger an RTO in TCP. In QUIC, we can't do that, because the packets live in different packet number spaces, and an ACK can only ever declare loss in its own packet number space.

So the question cannot be if we want to deviate from what TCP does. We already do, and there's no way around it.
Instead, we need to ask how much additional complexity we want to introduce to handle this corner case. I'd argue that the benefit is extremely limited, and we should therefore pick the simplest solution: only run persistent congestion detection for packets sent after handshake confirmation.

-- 
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/3939#issuecomment-662819300

---

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

> So the question cannot be if we want to deviate from what TCP does. We already do, and there's no way around it.

For some of the things, we have to deviate from QUIC, because there is no way around it.

But for this particular case, it is easy for some if not most of the QUIC stacks. Then, there's no ground to "recommend" something that deviates from TCP.

-- 
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/3939#issuecomment-662820926

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 28 Jul 2020 11:21:34 -0700

#3961 fixes this by clarifying that persistent congestion is across PN spaces. Since the resolution in #3961 introduces normative language, I would mark this as design.

-- 
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/3939#issuecomment-665199879