Subject: Re: [quicwg/base-drafts] Change PTO to be per packet number space (#3066)

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 06:31:27 -0700

@ianswett pushed 1 commit.

355d1e552b4ec41891941f7d1f8993ca3723c3da  Merge branch 'master' into ianswett-pto-per-pn-space

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3066/files/50f670169e4cae5b13065c015674b597515cda2e..355d1e552b4ec41891941f7d1f8993ca3723c3da

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 06:31:54 -0700

ianswett commented on this pull request.

> @@ -1153,8 +1174,9 @@ Pseudocode for OnLossDetectionTimeout follows:
 
 ~~~
 OnLossDetectionTimeout():
-  loss_time, pn_space = GetEarliestLossTime()
-  if (loss_time != 0):
+  earliest_loss_time, pn_space =
+    GetEarliestTimeAndSpace(loss_times)

Thanks, I missed that one.

-- 
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/3066#discussion_r341572362

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 06:32:23 -0700

cff00e01280aabf3916edb64508a5d786d779bb3  1 loss_times -> loss_time

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3066/files/355d1e552b4ec41891941f7d1f8993ca3723c3da..cff00e01280aabf3916edb64508a5d786d779bb3

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 01 Nov 2019 11:02:01 -0700

janaiyengar commented on this pull request.

> @@ -461,21 +464,34 @@ The PTO period is the amount of time that a sender ought to wait for an
 acknowledgement of a sent packet.  This time period includes the estimated
 network roundtrip-time (smoothed_rtt), the variance in the estimate (4*rttvar),
 and max_ack_delay, to account for the maximum time by which a receiver might
-delay sending an acknowledgement.
+delay sending an acknowledgement.  When the PTO is armed for Initial or
+Handshake packet number spaces, the max_ack_delay is 0.

"The endpoint may not have the peer's max_ack_delay when sending Initial or Handshake packets. The endpoint SHOULD set the max_ack_delay to 0 when arming a PTO for Initial or Handshake packet number spaces."

This brings up a question: I don't think transport params are available until the handshake is completed. I think we need tighter text around this to say that max_ack_delay needs to be assumed 0 until transport params are available. I'll file a separate issue for that.

 The PTO value MUST be set to at least kGranularity, to avoid the timer expiring
 immediately.
 
+A sender computes its PTO timer every time an ack-eliciting packet is sent.
+When ack-eliciting packets are in-flight in multiple packet number spaces,
+the timer MUST be set for the packet number space with the earliest timeout,
+except for ApplicationData, which MUST be ignored until the handshake
+completes; see Section 4.1.1 of {{QUIC-TLS}}.  Not arming the PTO for
+ApplicationData prioritizes completing the handshake and prevents the server
+from sending a 1-RTT PTO packet before it has the keys to process a 1-RTT

"from sending a 1-RTT packet in response to a PTO expiration before ..."

 The PTO value MUST be set to at least kGranularity, to avoid the timer expiring
 immediately.
 
+A sender computes its PTO timer every time an ack-eliciting packet is sent.
+When ack-eliciting packets are in-flight in multiple packet number spaces,
+the timer MUST be set for the packet number space with the earliest timeout,
+except for ApplicationData, which MUST be ignored until the handshake
+completes; see Section 4.1.1 of {{QUIC-TLS}}.  Not arming the PTO for
+ApplicationData prioritizes completing the handshake and prevents the server
+from sending a 1-RTT PTO packet before it has the keys to process a 1-RTT
+packet.  When probe packets from two different packet number spaces are sent
+when the PTO fires and both spaces have in-flight packets, this simplifies to
+setting the PTO on the lowest active packet number space.

Can you reword this sentence? It's got two "when"s and it is a mouthful

>  When a PTO timer expires, the PTO period MUST be set to twice its current
 value. This exponential reduction in the sender's rate is important because
 the PTOs might be caused by loss of packets or acknowledgements due to severe
-congestion.  The life of a connection that is experiencing consecutive PTOs is
-limited by the endpoint's idle timeout.
+congestion.  Even when there are ack-eliciting packets in-flight in multiple
+packet number spaces, the exponential increase in probe timeout occurs across
+all spaces to prevent excess load on the network.  For example, a timeout in
+the Initial packet number space doubles the length of the timeout in the
+Handshake packet number space.

Are all timers restarted at this point?

-- 
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/3066#pullrequestreview-310606049

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 11:08:44 -0700

We already fixed this issue, see Issue #2638 

-- 
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/3066#discussion_r341690443

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:31:45 -0700

I add a citation to QUIC-TRANSPORT for this.

-- 
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/3066#discussion_r341825030

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:34:15 -0700

"In response to" sounds like it's from the peer, which I find more confusing.

-- 
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/3066#discussion_r341825130

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:37:51 -0700

7b29c66d562641af795196e9be527196cb51ef55  Reword an awkward sentence

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3066/files/cff00e01280aabf3916edb64508a5d786d779bb3..7b29c66d562641af795196e9be527196cb51ef55

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:38:06 -0700

Agreed, I tried to reword 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/3066#discussion_r341825237

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:40:23 -0700

There's only one timer still.

-- 
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/3066#discussion_r341825301

---

From: Marten Seemann <[email-masked]>
Date: Sat, 02 Nov 2019 23:07:27 -0700

What is the motivation behind not sending a PTO for the application data packet number space until the handshake is completed? If I had to guess, we're trying to not put too much load onto the network (and thereby increase the chance that PTO packets containing Initial / Handshake data make it through) until we know that we're done with the handshake.
However, this only works for the server side. The client completes the handshake after sending its cert and the CFIN, but it doesn't know if those packets were actually received.

Maybe the signal we're actually looking for here is handshake confirmed, and not handshake completed?

-- 
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/3066#issuecomment-549108352

---

From: ianswett <[email-masked]>
Date: Sun, 03 Nov 2019 05:35:03 -0800

@marten-seemann The key is there's no point in arming the PTO for ApplicationData if you don't have keys to process the ACK frame if it was received.  I tried to make that clear in the text, but suggestions are welcome.

-- 
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/3066#issuecomment-549137176

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 18:24:38 -0800

@ianswett Thanks for the explanation. Re-reading the text, I think this makes sense now. I noted that this also prevents the client from PTO-ing on 0-RTT packets, which is nice.

-- 
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/3066#issuecomment-549207457

---

From: Marten Seemann <[email-masked]>
Date: Sun, 03 Nov 2019 18:37:04 -0800

marten-seemann commented on this pull request.

+A sender computes its PTO timer every time an ack-eliciting packet is sent.
+When ack-eliciting packets are in-flight in multiple packet number spaces,
+the timer MUST be set for the packet number space with the earliest timeout,
+except for ApplicationData, which MUST be ignored until the handshake
+completes; see Section 4.1.1 of {{QUIC-TLS}}.  Not arming the PTO for
+ApplicationData prioritizes completing the handshake and prevents the server
+from sending a 1-RTT PTO packet before it has the keys to process a 1-RTT
+packet.  This simplifies to setting the PTO on the lowest active packet number
+space if packets from two different packet number spaces are sent when the PTO
+fires.

I'm not sure if I understand this sentence. Consider the server that has just sent ServerHello and it's certificate, i.e. has outstanding Initial and Handshake data.
Isn't the whole point of this PR to arm the PTO timer based on both packet number spaces?

This is what I'd imagine would happen if no ACKs from the client are received:
t = 0: send Initial, immediately afterwards send Handshake. Set PTO timer for the Initial.
t = 1: PTO timer expires for the Initial packet. Send an Initial probe packet. Now the Handshake packet sent at t = 0 is the earliest outstanding packet, so set a PTO timer (with exponential backoff) for this packet.
t = 2: PTO timer expires for the Handshake packet. Send a Handshake probe packet. Now the Initial probe packet sent at t=1 is the earliest outstanding packet. The PTO timer for this packet would expire at t=5.

Is this 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/3066#pullrequestreview-310863196

---

From: ianswett <[email-masked]>
Date: Mon, 04 Nov 2019 10:09:33 -0800

The Handshake PTO would expire at t=3, because of the backoff, which may have been what you meant.

The second Initial PTO would expire at t=5, as you said.

One note is that the text does recommend you send a packet in other packet number spaces when possible
"In addition to sending data in the packet number space for which the timer
expired, the sender SHOULD send ack-eliciting packets from other packet
number spaces with in-flight data, coalescing packets if possible."

-- 
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/3066#discussion_r342192408

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 04 Nov 2019 17:53:28 -0800

"from sending a 1-RTT packet on a PTO before ..."

We don't have a "PTO packet"

@marten-seemann is right, I missed this too. The text says that the PTO is set on the "lowest active PN space", which is incorrect. I would simply drop this sentence.

-- 
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/3066#discussion_r342352193

---

From: ianswett <[email-masked]>
Date: Tue, 05 Nov 2019 11:21:31 -0800

Done.

-- 
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/3066#discussion_r342750898

---

From: ianswett <[email-masked]>
Date: Tue, 05 Nov 2019 11:21:57 -0800

Removed

-- 
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/3066#discussion_r342751110

---

From: ianswett <[email-masked]>
Date: Tue, 05 Nov 2019 11:22:32 -0800

6e902d3ae04f23269607c914d94e6e86cc6b425e  Jana's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3066/files/7b29c66d562641af795196e9be527196cb51ef55..6e902d3ae04f23269607c914d94e6e86cc6b425e

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 05 Nov 2019 15:14:11 -0800

janaiyengar 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/3066#pullrequestreview-312106650

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 05 Nov 2019 15:14:24 -0800

Merged #3066 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/3066#event-2773351558

---

From: Marten Seemann <[email-masked]>
Date: Tue, 05 Nov 2019 20:02:17 -0800

@ianswett No, I didn't mean t=3. The Handshake packet was sent sent at t=0, and we're doing one exponential backoff, meaning that the timer duration should be 2.
Or am I missing something 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/pull/3066#discussion_r342908364

---

From: ianswett <[email-masked]>
Date: Wed, 06 Nov 2019 03:28:48 -0800

I was thinking a Handshake packet was sent at 1, so it's my mistake.  Your example is correct given you're only going to send a single QUIC packet on PTO.

-- 
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/3066#discussion_r343045355