Subject: [quicwg/base-drafts] Is ACK delay zero for Initial and Handshake ACK frames? (#3459)

From: Lars Eggert <[email-masked]>
Date: Thu, 13 Feb 2020 23:31:47 -0800

Section 5.2.1 of -recovery-25 says:

>   When the PTO is armed for Initial or Handshake packet number spaces,
>   the max_ack_delay is 0, as specified in 13.2.5 of [QUIC-TRANSPORT].

However, Section 13.2.5 of -transport-25 doesn't actually say that (anymore?) Section 13.2.1 says

> For Initial and Handshake packets, a max_ack_delay of 0 is used.

so I guess at least the reference is incorrect.

But since Section 19.3 only has a SHOULD for encoding zero

>   Because the receiver
>      doesn't use the ACK Delay for Initial and Handshake packets, a
>      sender SHOULD send a value of 0.

and the pseudo code simply uses the encoded ACK delay, if the ACK sender doesn't encode zero, the ACK receiver will compute its RTT based on the encoded ACK delay.

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

---

From: ianswett <[email-masked]>
Date: Fri, 14 Feb 2020 07:32:35 -0800

The intent is to use 0 as a peer max_ack_delay for the Initial and Handshake packet number spaces.

The pseudocode says:
 ack_delay = min(Ack Delay in ACK Frame, max_ack_delay)

So for Initial and Handshake, min(Ack Delay, 0) = 0 

The text in the "Estimating smoothed RTT and RTTVar" section is fairly clear I think?
"- MUST ignore the Ack Delay field of the ACK frame for packets sent in the
   Initial and Handshake packet number space."

I fixed the section reference in #3460 

-- 
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/3459#issuecomment-586338131

---

From: Lars Eggert <[email-masked]>
Date: Sun, 16 Feb 2020 23:44:07 -0800

I understood the `max_ack_delay` in the `MIN` to be the value of the transport parameter?

-- 
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/3459#issuecomment-586857442

---

From: ianswett <[email-masked]>
Date: Mon, 17 Feb 2020 08:10:54 -0800

Not for Initial and Handshake PN spaces.  Would it be helpful to have a comment above the min() in the pseudocode?

-- 
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/3459#issuecomment-587064851

---

From: Lars Eggert <[email-masked]>
Date: Mon, 17 Feb 2020 23:41:52 -0800

That would at least help me (pointing out it is NOT the value of the TP, although the name is the same.)

-- 
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/3459#issuecomment-587323385

---

From: ianswett <[email-masked]>
Date: Tue, 18 Feb 2020 07:50:43 -0800

I noticed the pseudocode before UpdateRtt() already makes this fairly clear.  I can fold the pn_space check into UpdateRtt if you think it would read more clearly?

>     ack_delay = 0
>     if (pn_space == ApplicationData):
>       ack_delay = ack.ack_delay
>     UpdateRtt(ack_delay)

-- 
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/3459#issuecomment-587529337

---

From: Lars Eggert <[email-masked]>
Date: Wed, 19 Feb 2020 00:36:50 -0800

Ah, so we are not limiting `max_ack_delay`, we are overwriting `ack_delay`.

-- 
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/3459#issuecomment-588099777

---

From: Lars Eggert <[email-masked]>
Date: Wed, 19 Feb 2020 01:46:30 -0800

Closed #3459.

-- 
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/3459#event-3050977351