Subject: [quicwg/base-drafts] Ack delays are adjusted by ack_delay_exponent (#3362)

From: ianswett <[email-masked]>
Date: Fri, 17 Jan 2020 13:43:13 -0800

Notes that ack delays are right shifted by ack_delay_exponent when transmitted in ACK frames.

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

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

-- Commit Summary --

  * Ack delays are adjusted by ack_delay_exponent

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 19 Jan 2020 22:03:50 -0800

martinthomson commented on this pull request.

> @@ -315,14 +315,16 @@ samples, and rttvar is the variation in the RTT samples, estimated using a
 mean variation.
 
 The calculation of smoothed_rtt uses path latency after adjusting RTT samples
-for ACK delays.  For packets sent in the ApplicationData packet number space,
-a peer limits any delay in sending an acknowledgement for an ack-eliciting
-packet to no greater than the value it advertised in the max_ack_delay transport
-parameter.  Consequently, when a peer reports an Ack Delay that is greater than
-its max_ack_delay, the delay is attributed to reasons out of the peer's control,
-such as scheduler 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.
+for ACK delays. ACK delays are communicated in the ACK frame as an ack_delay
+field right shiften by the ack_delay_exponent transport parameter.  For packets

```suggestion
field left shifted by the ack_delay_exponent transport parameter.  For packets
```
I know that shifting and endianness are often contended, but I think that this is a left shift.

-- 
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/3362#pullrequestreview-345092840

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 20 Jan 2020 01:08:14 -0800

kazuho commented on this pull request.

 mean variation.

It seems confusing to me regardless.

Can we paraphrase the sentence to something like: _ACK delays are communicated using the ack_delay field of the ACK frame. The field contains ack delay in milliseconds, right shifted by the ack_delay_exponent 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/pull/3362#discussion_r368433398

---

From: Martin Thomson <[email-masked]>
Date: Mon, 20 Jan 2020 02:00:40 -0800

mean variation.

That seems right, apart from the bit where it should say "left".  Maybe we can instead say multiplied by 2 to the power of the ack_delay_exponent transport parameter value.

-- 
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/3362#discussion_r368459195

---

From: ianswett <[email-masked]>
Date: Mon, 20 Jan 2020 11:06:43 -0800

ianswett commented on this pull request.

 mean variation.

I think I'd rather not talk about shifting, since we describe it differently in transport, how about 'adjusted'?

-- 
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/3362#discussion_r368692253

---

From: ianswett <[email-masked]>
Date: Mon, 20 Jan 2020 11:07:24 -0800

@ianswett pushed 1 commit.

b215bd3ad1d3138504559c58f54a3da895ccc679  adjusted?

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3362/files/960c980f8e64b4eb9d63b51078024f1e81f6ca5a..b215bd3ad1d3138504559c58f54a3da895ccc679

---

From: ianswett <[email-masked]>
Date: Mon, 20 Jan 2020 11:11:51 -0800

df34dc8c55d83997e9eeb942eb489ddf12a5e32e  Reference 19.3 of transport

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3362/files/b215bd3ad1d3138504559c58f54a3da895ccc679..df34dc8c55d83997e9eeb942eb489ddf12a5e32e

---

From: ianswett <[email-masked]>
Date: Mon, 20 Jan 2020 11:12:14 -0800

mean variation.

And I added a reference to 19.3 of transport, see what you think.

-- 
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/3362#discussion_r368693677

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 20 Jan 2020 15:54:37 -0800

kazuho 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/3362#pullrequestreview-345575662

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 20 Jan 2020 16:54:16 -0800

janaiyengar commented on this pull request.

> @@ -315,14 +315,17 @@ samples, and rttvar is the variation in the RTT samples, estimated using a
 mean variation.
 
+for ACK delays. ACK delays are communicated in the ack_delay field of the

We shouldn't be using "ACK delays" as a term, only as a field name. I suggest the following text instead:
``` ... after adjusting RTT samples for acknowledgement delays. These delays are computed using the ACK Delay field of the ACK frame as described in Section 19.3 of {{QUIC-TRANSPORT}}.```

-- 
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/3362#pullrequestreview-345586531