ack_delay in the first Handshake and 1RTT packet #3981
Conversation
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.
|
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. |
|
Fair point. As I mention above, there are other ways this could be dealt with, including advice on processing buffered packets. |
There was a problem hiding this comment.
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.
draft-ietf-quic-recovery.md
Outdated
| 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 |
There was a problem hiding this comment.
The last sentence reads as imperative, but it's more of a "Note that if the delayed"
draft-ietf-quic-recovery.md
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
It's not really the first, but any packets that are buffered, right?
There was a problem hiding this comment.
Sure, though hopefully you're processing all buffered packets before sending an ACK.
draft-ietf-quic-recovery.md
Outdated
| - 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. |
There was a problem hiding this comment.
I think that we are again talking about packets that might have been buffered, not just the first.
There was a problem hiding this comment.
My previous review forgot that there were two cases:
-
a packet is received and buffered and then (much) later processed and (even) later acknowledged
-
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.
There was a problem hiding this comment.
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.
draft-ietf-quic-recovery.md
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
Sure, though hopefully you're processing all buffered packets before sending an ACK.
|
I'm still not sure I love this PR, but hopefully the text is now clearer. |
|
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. |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
It's the client that has this issue most often, which happens with 0-RTT. But I would switch this to just use 'endpoint'.
|
Necessary changes taken into #3821. |
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