Subject: [quicwg/base-drafts] Exception in ECN detection when ACK loss happens (#1555)

From: Magnus Westerlund <[email-masked]>
Date: Thu, 12 Jul 2018 12:47:09 -0700

Adding an exception to the ECN detection to handle ACK loss if that occurs. 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Merge tag 'draft-ietf-quic-transport-13' of https://github.com/quicwg/base-drafts into ecn-ack-loss
  * Adding exception due to ack loss.

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Thu, 12 Jul 2018 12:51:23 -0700

ianswett commented on this pull request.

> @@ -1738,6 +1738,14 @@ network, an endpoint verifies the following when an ACK_ECN frame is received:
 
 * The increase in ECT(0) and ECT(1) counters MUST be no greater than the number
   of packets newly acknowledged that were sent with the corresponding codepoint.
+  
+* If the increase in the ECT(0), ECT(1) and CE counters are greater than the 
+  number of newly ACKed packets, and received ACK's smallest acknowledged or 

I would just say "ACK's smallest acknowledged" and drop "or gap indicated packet number"

> @@ -1738,6 +1738,14 @@ network, an endpoint verifies the following when an ACK_ECN frame is received:
 
 * The increase in ECT(0) and ECT(1) counters MUST be no greater than the number
   of packets newly acknowledged that were sent with the corresponding codepoint.
+  
+* If the increase in the ECT(0), ECT(1) and CE counters are greater than the 
+  number of newly ACKed packets, and received ACK's smallest acknowledged or 
+  gap indicated packet number is larger than previously largest acknowledged 

How about: "...larger than the previous largest acknowledged, then it's likely acknowledgements were missed, and the above comparison MUST NOT be performed."

-- 
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/1555#pullrequestreview-136802602

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 12 Jul 2018 13:44:27 -0700

@gloinul pushed 2 commits.

1d37a4c  Fixing trailing spaces.
261c0e7  Implementing Ian's proposals

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1555/files/d5729ff903cbd11af97b10be1d2877538dff3db4..261c0e7d0a7b11f5f4bd07db253bdbe589f88f0e

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 12 Jul 2018 13:44:55 -0700

gloinul commented on this pull request.

Implemented

-- 
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/1555#discussion_r202173085

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 12 Jul 2018 13:45:02 -0700

Implemented.

-- 
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/1555#discussion_r202173122

---

From: ianswett <[email-masked]>
Date: Thu, 12 Jul 2018 17:39:57 -0700

> @@ -1739,6 +1739,13 @@ network, an endpoint verifies the following when an ACK_ECN frame is received:
 
+* If the increase in the ECT(0), ECT(1) and CE counters are greater than the
+  number of newly ACKed packets, and received ACK's smallest acknowledged is

ACKed -> acknowledged
received -> the received

-- 
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/1555#pullrequestreview-136875401

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 13 Jul 2018 05:57:22 -0700

@gloinul pushed 1 commit.

03ddb3e  Addressing editorial from Ian

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1555/files/261c0e7d0a7b11f5f4bd07db253bdbe589f88f0e..03ddb3ecc0a84c4c16d379262363b00f72d7335e

---

From: ianswett <[email-masked]>
Date: Fri, 13 Jul 2018 06:13:55 -0700

> @@ -1739,12 +1739,14 @@ network, an endpoint verifies the following when an ACK_ECN frame is received:
 
-* If the increase in the ECT(0), ECT(1) and CE counters are greater than the
-  number of newly ACKed packets, and received ACK's smallest acknowledged is
-  larger than the previous largest acknowledged, then it's likely
-  acknowledgements were missed, and the above comparison MUST NOT be
-  performed. Instead a new comparision point is stored by the sender so that
-  only changes after this point will be used in the future comaparisions.
+* If the increase in the ECT(0), ECT(1) and CE counters are greater than
+  the number of newly acknowledged packets, and the received ACK's
+  smallest acknowledged is larger than the previous largest acknowledged,
+  then it's likely acknowledgements were missed, and the above comparison
+  MUST NOT be performed. Instead a new comparision point is stored by the
+  sender so that only changes after this point will be used in the future
+  comaparisions.

Mikkel's right, comaparisons -> comparisons

-- 
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/1555#pullrequestreview-137029588

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 13 Jul 2018 06:24:55 -0700

7687a00  Fixing spelling of comparisons

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1555/files/03ddb3ecc0a84c4c16d379262363b00f72d7335e..7687a0017fb479bd3db344883a7de1e8d81968e0

---

From: ianswett <[email-masked]>
Date: Fri, 13 Jul 2018 06:27:15 -0700

ianswett 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/1555#pullrequestreview-137034175

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 13 Jul 2018 06:43:51 -0700

2a32722  Removed extra empty line.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1555/files/7687a0017fb479bd3db344883a7de1e8d81968e0..2a327224f25af7502350ff042aff340a3a72c7f6

---

From: Martin Thomson <[email-masked]>
Date: Sat, 14 Jul 2018 07:01:06 -0700

martinthomson approved this pull request.

> @@ -1739,6 +1739,14 @@ network, an endpoint verifies the following when an ACK_ECN frame is received:

As I pointed out, this isn't ideal and will lead to ECN being disabled in some cases.  I don't know how much we are going to want to worry about that though.

-- 
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/1555#pullrequestreview-137238926

---

From: janaiyengar <[email-masked]>
Date: Sat, 14 Jul 2018 14:02:47 -0700

janaiyengar 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/1555#pullrequestreview-137248516

---

From: janaiyengar <[email-masked]>
Date: Sat, 14 Jul 2018 14:02:54 -0700

Merged #1555.

-- 
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/1555#event-1733998924