Subject: [quicwg/base-drafts] ack_delay in the first Handshake and 1RTT packet (#3981)

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 15:12:41 +0000 (UTC)

Clarifies that ack_delay includes anytime the packets were buffered and undecryptable, as well as allowing senders to better use those first ACKs if they choose.

Fixes #3980 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * ack_delay in the first Handshake and 1RTT packet

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3981.patch
https://github.com/quicwg/base-drafts/pull/3981.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/3981

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 08:30:24 -0700

@ianswett pushed 1 commit.

311d22d3653542e2742c279822eddf3f4efe1616  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3981/files/9090e42c4942461e6529f3603f5f8e7f5465e662..311d22d3653542e2742c279822eddf3f4efe1616

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 08:34:51 -0700

9ed9536d298e87d76293802588a1f01d533c3c6f  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3981/files/311d22d3653542e2742c279822eddf3f4efe1616..9ed9536d298e87d76293802588a1f01d533c3c6f

---

From: Marten Seemann <[email-masked]>
Date: Mon, 03 Aug 2020 08:51:22 -0700

In #3970 we clarified that the ack_delay is only used for intentional delays introduced by an endpoint. Specifically, the time it takes to decrypt the packet and to parse the frames doesn’t count towards the ack_delay.

Counting the queuing delay for undecryptable packets seems to contract 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/3981#issuecomment-668098532

---

From: ianswett <[email-masked]>
Date: Mon, 03 Aug 2020 08:55:43 -0700

Fair point.  As I mention above, there are other ways this could be dealt with, including advice on processing buffered packets.

-- 
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/3981#issuecomment-668100800

---

From: Martin Thomson <[email-masked]>
Date: Mon, 03 Aug 2020 18:22:29 -0700

@martinthomson commented on this pull request.

I'm not sure about this at all.

This is about delays that are added by a *sender* due to being unable to process a packet (primarily due to a lack of the necessary keys).  So why not be clearer about this and talk about buffering and allow ACK Delay to exceed max_ack_delay by the time that the sender holds packets. 

Or, put another way, when processing an ACK frame, you allow subtract the time that you held the packet in a buffer from the current time in order to calculate the RTT sample.  Then you absorb any costs you imposed.  That means you don't need to concern yourself with the increase to ACK Delay.

As it relates to buffering, and there might be multiple buffered packets, I don't think that you can make a rule for the first packet in a packet number space.

> @@ -348,14 +348,22 @@ latency at the peer or loss of previous ACK frames.  Any delays beyond the
 peer's max_ack_delay are therefore considered effectively part of path delay and
 incorporated into the smoothed_rtt estimate.
 
+The first ACK frame in the Handshake and ApplicationData packet number spaces
+can be greatly delayed due to a lack of decryption keys at the time they're
+received. For these two ACK frames, the max_ack_delay MAY be ignored if the
+sender chooses. If the delayed ACK frame is the first RTT sample, ignoring

The last sentence reads as imperative, but it's more of a "Note that if the delayed"

> @@ -348,14 +348,22 @@ latency at the peer or loss of previous ACK frames.  Any delays beyond the
 peer's max_ack_delay are therefore considered effectively part of path delay and
 incorporated into the smoothed_rtt estimate.
 
+The first ACK frame in the Handshake and ApplicationData packet number spaces
+can be greatly delayed due to a lack of decryption keys at the time they're
+received. For these two ACK frames, the max_ack_delay MAY be ignored if the

It's not really the first, but any packets that are buffered, right?

>  - MUST ignore the ACK Delay field of the ACK frame for packets sent in the
-  Initial and Handshake packet number space.
+  Initial and Handshake packet number space, except for the first ACK frame
+  received in Handshake.

I think that we are again talking about packets that might have been buffered, not just the first.

-- 
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/3981#pullrequestreview-460431969

---

From: Martin Thomson <[email-masked]>
Date: Mon, 03 Aug 2020 22:08:36 -0700

My previous review forgot that there were two cases:

1. a packet is received and buffered and then (much) later processed and (even) later acknowledged

2. a packet containing an ACK is received and then (much) later processed

The first can be handled by reporting a large ACK Delay value.  For packets sent before handshake confirmation, keeping max_ack_delay unbounded is the only sensible way to deal with this.

The second is what this attempts to address.  But I don't think that it needs to.

-- 
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/3981#pullrequestreview-460498954

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 06:46:38 -0700

@ianswett commented on this pull request.

I was attempting to deal with the first issue where a receiver can't process an acknowledge a buffered packet until they keys are available.

Ideally there would be at most one of these inflated RTT samples, though I don't think we make any suggestions on how buffered packets should be processed when the keys are available.

The second issue hadn't occurred to me, because I don't think we've observed it yet in production.  That would only happen if the server received an ACK in 1RTT before completing the handshake.  As you said, an implementation could subtract the time the packet was buffered from the RTT.

Sure, though hopefully you're processing all buffered packets before sending an ACK.

-- 
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/3981#pullrequestreview-460818882

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 07:00:19 -0700

2226fe74aff7ff4bcfc2dbc8d8f837633da969ee  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3981/files/9ed9536d298e87d76293802588a1f01d533c3c6f..2226fe74aff7ff4bcfc2dbc8d8f837633da969ee

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 08:18:35 -0700

cb6f4c7c93a3b481d9794dcb76c83cf941624203  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3981/files/2226fe74aff7ff4bcfc2dbc8d8f837633da969ee..cb6f4c7c93a3b481d9794dcb76c83cf941624203

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 08:19:00 -0700

I'm still not sure I love this PR, but hopefully the text is now clearer.

-- 
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/3981#issuecomment-668659100

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 08:27:01 -0700

73ec359ef76275aeb222af46b48696a4677146eb  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3981/files/cb6f4c7c93a3b481d9794dcb76c83cf941624203..73ec359ef76275aeb222af46b48696a4677146eb

---

From: ianswett <[email-masked]>
Date: Tue, 04 Aug 2020 14:24:48 -0700

I want to add some text about processing all buffered packets at once when new keys are available, but I'd like to merge PR #3979 first.

-- 
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/3981#issuecomment-668832801

---

From: Martin Thomson <[email-masked]>
Date: Tue, 04 Aug 2020 18:10:26 -0700

Yes, this needs updates based on the resolution of #3821, but it is generally good.

> +The server can receive 1-RTT packets containing ACK frames before the decryption
+keys are available. If the packet is buffered and later processed, the server
+MAY reduce the measured RTT by the amount of time the packet was buffered.

It's the client that has this issue most often, which happens with 0-RTT.  But I would switch this to just use 'endpoint'.

-- 
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/3981#pullrequestreview-461273814

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 15:10:23 -0700

Closed #3981.

-- 
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/3981#event-3645216394

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 11 Aug 2020 15:10:39 -0700

Necessary changes taken into #3821.

-- 
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/3981#issuecomment-672309286