Subject: [quicwg/base-drafts] Clarify the Timestamps section (#360)

From: ianswett <[email-masked]>
Date: Mon, 06 Mar 2017 14:00:38 -0800

Intends to resolve #308 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Clarify the Timestamps section

-- File Changes --

    M draft-ietf-quic-transport.md (11)

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Mar 2017 15:58:23 -0800

martinthomson commented on this pull request.

I'd like to see more clarification of the "Delta Last Acknowledged 1..N" section.  It's still not clear that this is relative to the last acknowledged.

> @@ -1221,6 +1221,13 @@ Unlike TCP SACKs, QUIC ACK blocks are cumulative and therefore irrevocable.
 Once a packet has been acknowledged, even if it does not appear in a future ACK
 frame, it is assumed to be acknowledged.
 
+QUIC ack frames contain a timestamp section with up to 255 timestamps.

ACK frames.

-- 
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/360#pullrequestreview-25398854

---

From: Martin Thomson <[email-masked]>
Date: Mon, 06 Mar 2017 19:56:17 -0800

@martinthomson pushed 1 commit.

1eadc0d  upcase "ack" and remove some trailing whitespace

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/360/files/41da8a6db3264fb78f36d7ba71b0cbadf22631be..1eadc0d20be2fb0c3ba132e2a2fd655c2a3285ad

---

From: ianswett <[email-masked]>
Date: Tue, 07 Mar 2017 06:54:05 -0800

@ianswett pushed 1 commit.

a6894e4  Clarify timestamp ordering

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/360/files/1eadc0d20be2fb0c3ba132e2a2fd655c2a3285ad..a6894e43b5c9ce781eff38f30dd2675640fa4395

---

From: ianswett <[email-masked]>
Date: Tue, 07 Mar 2017 07:00:33 -0800

d1114ce  Trailing whitespace.

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

---

From: ianswett <[email-masked]>
Date: Tue, 07 Mar 2017 07:05:03 -0800

Added a section to further clarify that Delta Last Acknowledged 1..N" is relative to the last acknowledged

-- 
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/360#issuecomment-284747011

---

From: janaiyengar <[email-masked]>
Date: Tue, 07 Mar 2017 13:48:02 -0800

janaiyengar commented on this pull request.

> @@ -1352,6 +1358,10 @@ receive times relative to the beginning of the connection.
 ~~~
 {: #timestamp-format title="Timestamp Section"}
 
+The timestamp section is time-ordered, so the 1 byte Delta Largest
+Acknowledged is always relative to the Largest Acknowleged, but the Time
+Since Previous Timestamp fields are relative.

I would suggest moving this to below the descriptions of the fields. Suggested rewording: "The timestamp section lists packet receipt timestamps ordered by timestamp." I think you can drop the rest of this paragraph since it's redundant with text in the descriptions.

-- 
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/360#pullrequestreview-25640138

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Mar 2017 23:07:33 -0800

Rebased and merged manually.

-- 
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/360#issuecomment-284964656

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Mar 2017 23:07:33 -0800

Closed #360.

-- 
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/360#event-990883333

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Mar 2017 23:08:43 -0800

@ianswett, can you try to remember to name your branches in future.  These branch names are going to collide with old patches (as this one already did, causing me some confusion).

-- 
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/360#issuecomment-284964825

---

From: ianswett <[email-masked]>
Date: Wed, 08 Mar 2017 06:41:23 -0800

Sure, I'm happy to name them.  I didn't realize it mattered.

-- 
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/360#issuecomment-285058383