Subject: [quicwg/base-drafts] Add note about stale ACK frames breaking ECN (#2879)

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Jul 2019 21:18:10 -0700

Closes #2205.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Add note about stale ACK frames breaking ECN

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 21:28:12 -0700

martinthomson commented on this pull request.

I don't think that the conditions are quite right here.

> @@ -3154,11 +3154,13 @@ to be greater than the number of packets acknowledged in an ACK frame.  When
 this happens, and if verification succeeds, the local reference counts MUST be
 increased to match the counts in the ACK frame.
 
-Processing counts out of order can result in verification failure.  An endpoint
-SHOULD NOT perform this verification if the ACK frame is received in a packet
-with packet number lower than a previously received ACK frame.  Verifying based
-on ACK frames that arrive out of order can result in disabling ECN
-unnecessarily.
+Processing counts out of order can result in verification failure.  This can
+happen when packets containing ACK frames are reordered in the network, or if a
+sender retransmits an ACK frame with stale information.  An endpoint SHOULD NOT
+perform this verification if the ACK frame is received in a packet with packet
+number lower than a previously received ACK frame or if the ACK frame does not
+acknowledge any new packets. ACK frames that arrive out of order can result in

```suggestion
acknowledge any new packets. Validating ACK frames in those conditions can result in
```

> @@ -3154,11 +3154,13 @@ to be greater than the number of packets acknowledged in an ACK frame.  When
 this happens, and if verification succeeds, the local reference counts MUST be
 increased to match the counts in the ACK frame.
 
-Processing counts out of order can result in verification failure.  An endpoint
-SHOULD NOT perform this verification if the ACK frame is received in a packet
-with packet number lower than a previously received ACK frame.  Verifying based
-on ACK frames that arrive out of order can result in disabling ECN
-unnecessarily.
+Processing counts out of order can result in verification failure.  This can
+happen when packets containing ACK frames are reordered in the network, or if a
+sender retransmits an ACK frame with stale information.  An endpoint SHOULD NOT
+perform this verification if the ACK frame is received in a packet with packet
+number lower than a previously received ACK frame or if the ACK frame does not
+acknowledge any new packets. ACK frames that arrive out of order can result in

Question: is it possible to skip validation based on just one of these conditions?  I think that only the packet number condition is really necessary.

Rationale: Say I ACK 1-4 then later drop that range (for any reason) and ACK 5-9 in another packet.  If those packets get reordered, then when the first eventually arrives, it will have new acknowledgments, but it will still have bad ECN counts.  So you can't say that new acknowledgments is necessary.

The packet number is clearly the thing to use - and sufficient on its own - but would it be enough to look for increases in largest acknowledged?  ACK processors should not be expected to have access to packet numbers.  Allowing implementations to use largest acknowledged would depend on us not allowing that to regress, but that seems like it follows from existing advice.

-- 
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/2879#pullrequestreview-259262482

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Jul 2019 23:01:09 -0700

janaiyengar commented on this pull request.

I'm not sure that we need to collapse this. I think it's reasonable to say that the largest acked should increase, though it's going to be too broad. Any scenario that has reordering of received packets will have ACKs being sent where the largest acked is not increasing.

If the packet number of the received ACK is increasing, it should be adequate, with this one exception (which ought to be rare) where the sender retransmits stale ACKs. I'd rather cover the common case of looking at packet number to determine whether the count is valid, and handling this exceptional case additionally.

-- 
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/2879#discussion_r301407837

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 23:24:35 -0700

I would prefer instead to say that you MUST NOT retransmit stale ACKs AND to require that new ACKs always increase the largest acknowledged (or the ACK range with the largest packet number cannot be discarded).  Then we only need to say that endpoints skip ECN validation when the largest acknowledged does not increase.

That's strictly less good than using packet number, because newly acknowledged packets don't always increase the largest acknowledged, so we could mention that checking for an increasing packet number is even better, but that only enables validation more often.

-- 
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/2879#discussion_r301413488

---

From: MikkelFJ <[email-masked]>
Date: Tue, 09 Jul 2019 02:00:44 -0700

mikkelfj commented on this pull request.

Remember that asynchronous senders cannot always coordinate counters exactly. So process A uses some packet numbers, process B user some others, interleaved. you cannot be sure acks are exactly in sync, only approximately so. If the receiver looks at largest acked rather than packet numbers I think it should be ok.

-- 
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/2879#discussion_r301473963

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Jul 2019 16:37:23 -0700

Summarizing where I think @martinthomson and I are currently. An endpoint only validates ECN counts for ACK frames received where the largest acknowledged increases. This guarantees that the ACK is newer, meaning that ECN counts should be higher... This is only true if we require that the peer SHOULD increase ECN counts only in ACK frames where largest acknowledged is not decreasing. If the peer doesn't, then it risks ECN validation failing.

-- 
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/2879#discussion_r301835287

---

From: ianswett <[email-masked]>
Date: Tue, 09 Jul 2019 16:50:28 -0700

ianswett commented on this pull request.

That SG.  We should also strongly recommend NOT retransmitting old ACKs if we don't already.

-- 
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/2879#discussion_r301837919

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Jul 2019 00:14:41 -0700

Note that we're not strictly requiring that endpoints never regress largest acknowledged, but any implementation that does that probably won't benefit from ECN for very long.

-- 
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/2879#discussion_r301915509

---

From: Magnus Westerlund <[email-masked]>
Date: Tue, 23 Jul 2019 08:53:53 -0700

gloinul commented on this pull request.

> Rationale: Say I ACK 1-4 then later drop that range (for any reason) and ACK 5-9 in another packet. If those packets get reordered, then when the first eventually arrives, it will have new acknowle…

But, in this situation when the ECN counters increase more than expected, the sender getting the ACK will detect that ECN counter increase more and re-synchronize its state. Per this text: 

   An endpoint could miss acknowledgements for a packet when ACK frames
   are lost.  It is therefore possible for the total increase in ECT(0),
   ECT(1), and CE counts to be greater than the number of packets
   acknowledged in an ACK frame.  When this happens, and if verification
   succeeds, the local reference counts MUST be increased to match the
   counts in the ACK frame.

Thus, if ACK 1-4 arrive then it will not increase the largest acknowledged, it may however ACK new packets if there are no overlapp. But, according to the rules here it will not result in a validation. 
> The packet number is clearly the thing to use - and sufficient on its own - but would it be enough to look for increases in largest acknowledged? ACK processors should not be expected to have acces…

I think this also will ensure that @janaiyengar  rule about not increasing ECN counter is also not needed, or do I fail to understand the concern? 

-- 
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/2879#discussion_r306398981