Subject: Re: [quicwg/base-drafts] Gorry's ECN rewrite (#4059)

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 03 Sep 2020 00:27:58 -0700

@gorryfair commented on this pull request.

>  with an ECT(0) marking.  Similarly, ECN validation fails if the sum of the
 increases to ECT(1) and ECN-CE counts is less than the number of newly
 acknowledged packets sent with an ECT(1) marking.  These checks can detect
-removal of ECN markings in the network.
+remarking of ECN CE-markings by the network.

I'm OK with a change of word, but the checks only detect some changes to the ECN-Field. They can't detect rewriting of a CE-Marking back to ECT(0) (for instance when ECT(0) was sent), so we should be careful to avoid giving that impression..

-- 
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/4059#discussion_r482764338

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 03 Sep 2020 00:38:03 -0700

> +frame.  This is why ECN counts are permitted to be larger than the value
+corresponding to the largest acknowledged packet number.

I think the original text was this - I wouldn't object to using these words, but I think they could be clearer: "This is why counts are permitted to be larger than might be accounted for by newly acknowledged packets."

-- 
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/4059#discussion_r482770155

---

From: Lars Eggert <[email-masked]>
Date: Thu, 03 Sep 2020 13:29:08 +0000 (UTC)

@larseggert requested changes on this pull request.

> -rate in response, as described in {{QUIC-RECOVERY}}.
-
-To use ECN, QUIC endpoints first determine whether a path supports ECN marking
-and the peer is able to access the ECN codepoint in the IP header.  A network
-path does not support ECN if ECN marked packets get dropped or ECN markings are
-rewritten on the path. An endpoint validates the use of ECN on the path, both
-during connection establishment and when migrating to a new path
-({{migration}}).
+detect and respond to network congestion.  ECN allows an endpoint to set an ECT
+codepoint in the ECN field of an IP packet. A network node can then indicate
+congestion by setting the CE codepoint in the ECN field instead of dropping the
+packet {{?RFC8087}}.  Endpoints react to reported congestion by reducing their
+sending rate in response, as described in {{QUIC-RECOVERY}}.
+
+To enable ECN, a sending QUIC endpoint first determines whether a path supports
+ECN marking and whether reports the ECN value in received IP headers; see

```suggestion
ECN marking and whether the peer reports the ECN values in received IP headers; see
```

-It is possible for faulty network devices to corrupt or erroneously drop packets
-with ECN markings.  To provide robust connectivity in the presence of such
-devices, each endpoint independently validates ECN counts and disables ECN if
-errors are detected.
+For example, if one each of an Initial, 0-RTT, Handshake, and 1-RTT QUIC packet
+are coalesced, the corresponding counts for the Initial and Handshake packet
+number space will be incremented by one and the counts for the application data

```suggestion
number spaces will be incremented by one each and the counts for the application data
```

-#### Sending ECN Markings
+It is possible for faulty network devices to corrupt or erroneously drop packets
+that set an ECN codepoint.  To provide robust connectivity in the presence of

```suggestion
that carry an ECN codepoint.  To provide robust connectivity in the presence of
```

-* Set the ECT(0) codepoint in the IP header of early outgoing packets sent on a
-  new path to the peer ({{!RFC8311}}).
+* The endpoint SHOULD set the ECT(0) codepoint in the IP header of early

```suggestion
* The endpoint MUST set the ECT(0) codepoint in the IP header of early
```
Can't validate if you don't set it.

-* If all packets that were sent with the ECT(0) codepoint are eventually deemed
-  lost (Section 6 of {{QUIC-RECOVERY}}), validation is deemed to have failed.
+* The endpoint SHOULD monitor whether all packets sent with an ECT codepoint are

```suggestion
* The endpoint MUST monitor whether all packets sent with an ECT codepoint are
```
Can't validate if you don't monitor.

> +  eventually deemed lost (Section 6 of {{QUIC-RECOVERY}}), deeming this as an
+  indication that ECN validation has failed.

```suggestion
  eventually deemed lost (Section 6 of {{QUIC-RECOVERY}}), indicating
  that ECN validation has failed.
```

> +To reduce the chances of misinterpreting loss of packets dropped by a faulty
+network element, an endpoint could set an ECT codepoint for only the first ten
+outgoing packets on a path, or for a period of three RTTs, whichever occurs
+first.

More needs to be said here about why this might be beneficial.

> +{{?RFC8311}}. Implementations that use the ECT(1) codepoint need to
+perform ECN validation using the reported ECT(1) counts.

This whole ECN text needs to make up its mind on whether it wants to specify that ECT(0) should be used, or if it wants to allow ECT(0) or ECT(1). At the moment, it's a mix. And if it wants to allow both, some words need to be added whether an implementation should pick one statically or randomly or whatnot.

-ECN validation MAY fail if the total count for an ECT(0) or ECT(1) marking
-exceeds the total number of packets sent with the corresponding marking.   In
-particular, an endpoint that never applies a particular marking can fail
-validation when a non-zero count for the corresponding marking is received.
-This check can detect when packets are marked ECT(0) or ECT(1) in the network.
+Out of order processing of the ECN counts can result in a validation failure.

What does "out of order processing of the ECN counts" mean? 

-ECN validation MAY fail if the total count for an ECT(0) or ECT(1) marking
-exceeds the total number of packets sent with the corresponding marking.   In
-particular, an endpoint that never applies a particular marking can fail
-validation when a non-zero count for the corresponding marking is received.
-This check can detect when packets are marked ECT(0) or ECT(1) in the network.
+Out of order processing of the ECN counts can result in a validation failure.
+An endpoint SHOULD skip ECN validation for an ACK frame that does not increase

Why is this not a MUST?

-- 
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/4059#pullrequestreview-481826612

---

From: mirjak <[email-masked]>
Date: Thu, 03 Sep 2020 07:13:30 -0700

@mirjak requested changes on this pull request.

+packet number space will be increased by two.

Is this correct? The congestion controller will only count this as one mark and for validation the sender must then also track the sent markings on a QUIC packet base and not per IP packet.

-#### Sending ECN Markings
+It is possible for faulty network devices to corrupt or erroneously drop packets
+that set an ECN codepoint.  To provide robust connectivity in the presence of
+such devices, an endpoint validates the ECN counts for each network path  and

This needs more explanation because you say first something about "drops" and the "validate ECN counts" but to check for drops you need to detect loss of ECT marked packet which is not really discussed here.

Didn't we also discuss a check were all packets get marked as CE (including non-ECT packets)? That's actually the only check that is basically implemented today in TCP (checking if the SYN is CE which should be non-ECT).

I don't think normative language makes sense here as we already say in the intro sentence "an endpoint SHOULD do the following"

I think it actually not beneficial :-)

-- 
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/4059#pullrequestreview-481867260

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 03 Sep 2020 09:45:27 -0700

Doesn't every IP packet carry an ECN code point. Maybe we need to say ECT(0) or ECT(1) code point then?

-- 
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/4059#discussion_r483117491

---

From: Lars Eggert <[email-masked]>
Date: Thu, 03 Sep 2020 09:50:07 -0700

@larseggert commented on this pull request.

Or a non-zero ECN codepoint?

-- 
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/4059#discussion_r483120106

---

From: Lars Eggert <[email-masked]>
Date: Thu, 03 Sep 2020 09:50:23 -0700

That's fine, too.

-- 
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/4059#discussion_r483120255

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 03 Sep 2020 09:59:25 -0700

This mechanism was there already, so it wasn't my idea. When I edited,  I didn't see where it did hard - nor did I see benefit - so is it needed?

-- 
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/4059#discussion_r483125648

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 07 Sep 2020 00:08:42 -0700

@mirjak - Can you suggest what text you think is missing, so we can complete the edit?

-- 
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/4059#discussion_r484223565

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 07 Sep 2020 00:09:24 -0700

@mirjak - What do you suggest (remove this text)?

-- 
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/4059#discussion_r484223864

---

From: mirjak <[email-masked]>
Date: Mon, 07 Sep 2020 01:08:15 -0700

@mirjak commented on this pull request.

I argued in another discussion already that we should just recommend to mark all packets as ECT(0) until validation fails as this case where packet get dropped is basically not existence anymore. However, this was not accepted as people felt this approach would not increase implementation complexity. I still think the text should note that this is a really rare case and I had an editorial PR for that that was also not expected, so I would propose anything again. Just wanted to state my opinion...

-- 
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/4059#discussion_r484262204

---

From: mirjak <[email-masked]>
Date: Mon, 07 Sep 2020 01:10:50 -0700

This might be more than an edit but I guess there could be another case, that checks e.g. after one RTT if all packets got CE marked and then fails validation.

-- 
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/4059#discussion_r484263570

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 07 Sep 2020 02:42:25 -0700

I think we all need to find text we can agree upon. Here is a proposal:

"If an endpoint has cause to expect that packets setting an ECT codepoint are being dropped by faulty network element, the endpoint could set an ECT codepoint for only the first ten outgoing packets on a path, or for a period of three RTTs. If an endpoint then discovers all non-zero ECN codepoints fail to be delivered, it can assume the path did not support ECN.

Other methods of probing paths for ECN support are possible,..."

-- 
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/4059#discussion_r484317976

---

From: mirjak <[email-masked]>
Date: Mon, 07 Sep 2020 04:10:42 -0700

Looks good to me!

-- 
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/4059#discussion_r484364050

---

From: Lars Eggert <[email-masked]>
Date: Mon, 07 Sep 2020 04:27:19 -0700

Who holds the pen on this PR?

-- 
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/4059#issuecomment-688263251

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 07 Sep 2020 04:37:17 -0700

Martin, (cc Lars)

I think, if I understand, that we now have text that is "mostly" 
correctable from the comments. I supect the proposed changes fix most 
(all?) of the confusion I originally had.  You own the edit token.

On 07/09/2020 12:27, Lars Eggert wrote:
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub 
> <https://github.com/quicwg/base-drafts/pull/4059#issuecomment-688263251>, 
> or unsubscribe 
> <https://github.com/notifications/unsubscribe-auth/ABYLLEXJ7CRAILXKLKVZY4LSES7Q3ANCNFSM4QMZAVOA>.
>
* I don't think I have edit permission to fix the PR text where the PR 
comments propose a change, but you likely can edit!  - or you can make 
this possible for me to do, and I will re-edit these in.

* I think Mirja has a few remaining issues, which I think I'd sympathise 
with, but talking off-list I think the resolution of the comments in the 
PR edit may go a long way to alaso address these changes.

I don't know how best to progress, should we try to land the ECN rewrite 
making all agreed corrections, and then review what is left?

Gorry

-- 
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/4059#issuecomment-688268297

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Sep 2020 16:51:22 -0700

@martinthomson pushed 1 commit.

245c51cb4d401212d48ef75d6260bdaf04ab1107  Many suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/2a96678726bdd6b4c36589de9761854c5475826c..245c51cb4d401212d48ef75d6260bdaf04ab1107

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Sep 2020 16:57:46 -0700

a00bb9b96401f9c33cd40b32c64de923643355fe  My own suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/245c51cb4d401212d48ef75d6260bdaf04ab1107..a00bb9b96401f9c33cd40b32c64de923643355fe

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Sep 2020 17:30:35 -0700

@martinthomson pushed 2 commits.

0a22dfe572fa555b504730a7672d781a0a17552d  Review feedback
a6928b35a82e75ea934c8fffefa6afac5edd63e1  Boundary conditions for validation

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Sep 2020 19:01:04 -0700

70e43d5cb8c041ce6e0f8948a7c7595ef803434f  Wrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/a6928b35a82e75ea934c8fffefa6afac5edd63e1..70e43d5cb8c041ce6e0f8948a7c7595ef803434f

---

From: Martin Thomson <[email-masked]>
Date: Mon, 07 Sep 2020 19:06:50 -0700

@martinthomson commented on this pull request.

-* Set the ECT(0) codepoint in the IP header of early outgoing packets sent on a
-  new path to the peer ({{!RFC8311}}).
+* The endpoint sets an ECT(0) or ECT(1) codepoint in the IP header of early

To Lars' point below, adding ECT(1) probably isn't a good idea here.  This is probably the critical point at which we mention only ECT(0).  So I've struck ECT(1) from this.

I agree with Jana here.  The original text is better.  The only tweak I might make is to say "counts to be greater than the number of packets *that are newly* acknowledged *by* an ACK frame."

I've reverted and made that change.

I've reworded this; let me know if this works.

This should be the only place that ECT(1) is mentioned, aside from the requirement to count and report the count.

I've tried to reword this to be clearer.  Let me know if this makes more sense now.

This is how the protocol works.  The expectation is that a sender will correct for any double-counting.  That is, it will map newly acknowledged packets to the expected increase in counts and the number of *IP* packets that were sent.

This is introduction.  The explanation you seek follows.

-- 
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/4059#pullrequestreview-483728655

---

From: mirjak <[email-masked]>
Date: Tue, 08 Sep 2020 00:26:24 -0700

I just thought the two sentences don't fit together because the first talks about corrupt and drop but the second sentence only talk about validating counts and not about checking on loss. This is an editorial comment.

-- 
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/4059#discussion_r484706050

---

From: mirjak <[email-masked]>
Date: Tue, 08 Sep 2020 00:28:43 -0700

I don't think your rewording did change anything from the original text.

-- 
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/4059#discussion_r484707543

---

From: mirjak <[email-masked]>
Date: Tue, 08 Sep 2020 00:31:39 -0700

-If validation fails, then the endpoint stops sending ECN markings in subsequent
-IP packets with the expectation that either the network path or the peer does
-not support ECN.
+If validation fails, then the endpoint MUST disable ECN. It stops setting the

I don't think this should be a MUST. If normative language is needed I guess it should be a SHOULD with a warning that remarking by the network can conceal congestion. I would however rather simply not use normative language here.

-- 
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/4059#pullrequestreview-483866098

---

From: mirjak <[email-masked]>
Date: Tue, 08 Sep 2020 00:33:59 -0700

That was my question: do we specify correctly that the sender has to correct for double-counting?

-- 
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/4059#discussion_r484710481

---

From: mirjak <[email-masked]>
Date: Tue, 08 Sep 2020 00:46:21 -0700

Sorry, I think your wording is fine. I looked at this text early in the same section which still seems to indicate that dropping is a real problem:

```
It is possible for faulty network devices to corrupt or erroneously drop
 packets that carry a non-zero ECN codepoint. To ensure connectivity in the
 presence of such devices, an endpoint validates the ECN counts for each network
 path and disables use of ECN on that path if errors are detected.
```

-- 
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/4059#discussion_r484717522

---

From: Gorry Fairhurst <[email-masked]>
Date: Tue, 08 Sep 2020 01:11:32 -0700

I agree,  and suggest this sentence can now be removed, and is now covered by the clarified later sentence: "It is possible for faulty network devices to corrupt or erroneously drop packets that carry a non-zero ECN codepoint."

-- 
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/4059#discussion_r484731938

---

From: Gorry Fairhurst <[email-masked]>
Date: Tue, 08 Sep 2020 01:14:05 -0700

I think it is a MUST, until the next round of validation starts. The endpoint shouldn't continue to set ECT(?) and not be able to respond when CE is marked.

-- 
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/4059#discussion_r484733441

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Sep 2020 16:01:15 -0700

@janaiyengar approved this pull request.

This is what the original text said, so I think the intent is that of a MUST.

-- 
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/4059#pullrequestreview-484550401

---

From: Martin Thomson <[email-masked]>
Date: Tue, 08 Sep 2020 17:03:16 -0700

Plan here is to have Ian review this and merge it for -30 unless there is something that comes up.

-- 
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/4059#issuecomment-689200258

---

From: ianswett <[email-masked]>
Date: Wed, 09 Sep 2020 16:09:32 -0700

@ianswett requested changes on this pull request.

Some suggestions, some of which I don't have suggestions for.

 ### ECN Counts
 
-On receiving a QUIC packet with an ECT or CE codepoint, an ECN-enabled endpoint
-that can access the ECN codepoints from the enclosing IP packet increases the
-corresponding ECT(0), ECT(1), or CE count, and includes these counts in
-subsequent ACK frames; see {{generating-acks}} and {{frame-ack}}.
-
-An IP packet that results in no QUIC packets being processed does not increase
-ECN counts.  A QUIC packet detected by a receiver as a duplicate does not
-affect the receiver's local ECN codepoint counts; see {{security-ecn}} for
-relevant security concerns.
+Use of ECN requires the receiving endpoint to read the ECN codepoint from an IP

nit: There's sending ECN and receiving ECN.  I believe there are platforms where receiving is possible, but sending is not.   As such, I'm unclear if "Use of ECN" means a sender negotiating it and receiving the feedback in ACK frames or something else?  Possible suggestion below.

```suggestion
In order for the sender to use ECN in its congestion controller, the sender needs to be able to set the ECN codepoint in the IP header and the receiver needs to be able to to read the ECN codepoint from an IP
```

-Coalesced packets (see {{packet-coalesce}}) mean that several packets can share
-the same IP header.  The ECN counts for the ECN codepoint received in the
-associated IP header are incremented once for each QUIC packet, not per
-enclosing IP packet or UDP datagram.
+On receiving a QUIC packet with an ECT(0), ECT(1) or CE codepoint, an

nit: QUIC packets don't have ECT codepoints, IP packets do.

```suggestion
On receiving an IP packet with an ECT(0), ECT(1) or CE codepoint, an
```

-Coalesced packets (see {{packet-coalesce}}) mean that several packets can share
-the same IP header.  The ECN counts for the ECN codepoint received in the
-associated IP header are incremented once for each QUIC packet, not per
-enclosing IP packet or UDP datagram.
+On receiving a QUIC packet with an ECT(0), ECT(1) or CE codepoint, an
+ECN-enabled endpoint accesses the ECN codepoint from the enclosing IP packet and

```suggestion
ECN-enabled endpoint accesses the ECN codepoint and
```

> +ECN counts.  Coalesced QUIC packets (see {{packet-coalesce}}) share the same IP
+header.  The ECN counts for the ECN codepoint received in the associated IP
+header are incremented once for each QUIC packet.

I find this structure a bit awkward, so suggestion below.

```suggestion
ECN counts.  Coalesced QUIC packets (see {{packet-coalesce}}) share the same IP
header so the ECN counts are incremented once for each QUIC packet.
```

-### ECN Validation {#ecn-validation}
+For example, if one each of an Initial, 0-RTT, Handshake, and 1-RTT QUIC packet

This is a bad example as there's no practical use case where 0-RTT would be coalesced with 1-RTT as one would never have both keys at once.  I understand the point of the example, but I think it's important to keep to practical examples.

```suggestion
For example, if one each of an Initial, Handshake, and 1-RTT QUIC packet
```

-It is possible for faulty network devices to corrupt or erroneously drop packets
-with ECN markings.  To provide robust connectivity in the presence of such
-devices, each endpoint independently validates ECN counts and disables ECN if
-the path is not showing consistent support for ECN.
+An ECN count is not incremented when no QUIC packets from the received IP

```suggestion
ECN counts are only incremented when QUIC packets from the received IP
```

> +packet are processed. A QUIC packet detected by a receiver as a duplicate also
+does not increase an ECN count; see {{security-ecn}} for relevant security

You don't process duplicate packets, so suggestion below.

```suggestion
packet are processed. As such, duplicate QUIC packets are not processed and
do not increase ECN counts; see {{security-ecn}} for relevant security
```

-* If all packets that were sent with the ECT(0) codepoint are eventually deemed
-  lost (Section 6 of {{QUIC-RECOVERY}}), validation is deemed to have failed.
+If an endpoint has cause to expect that IP packets with an ECT codepoint might

Setting ECN on 10 packets if you think they're going to be lost seems odd and I can't imagine anyone would do that.  This text is such a departure from the existing text, I'm unsure how to fix it.

> +An endpoint thus attempts to use ECN and validates this for each new connection,
+when switching to a server's preferred address, and on active connection
+migration to a new path.  {{ecn-alg}} describes one possible algorithm.

```suggestion
An endpoint negotiates ECN for each new connection or new path.
{{ecn-alg}} describes one possible algorithm.
```

-ECN validation fails if the sum of the increase in ECT(0) and ECN-CE counts is
-less than the number of newly acknowledged packets that were originally sent
+ECN validation also fails if the sum of the increase in ECT(0) and ECN-CE counts

undo

```suggestion
ECN validation also if the sum of the increase in ECT(0) and ECN-CE counts
```

 increases to ECT(1) and ECN-CE counts is less than the number of newly
 acknowledged packets sent with an ECT(1) marking.  These checks can detect
-removal of ECN markings in the network.
+remarking of ECN-CE markings by the network.

remarking and removal are completely different and I believe removal is correct.  I'm unsure what's meant by remarking.

```suggestion
removal of ECN-CE markings by the network.
```

> +an ACK frame. This is why ECN counts are permitted to be larger than the value

This is unfortunately incorrect.  One could in theory start with a packet number of 1 million and it should not affect correctness.

```suggestion
an ACK frame. This is why ECN counts are permitted to be larger than the total
number of acknowledged packets within a packet number space.
```

-Processing ECN counts out of order can result in validation failure.  An
-endpoint SHOULD skip ECN validation on an ACK frame that does not increase the
-largest acknowledged packet number.
+ECN validation can fail if the received total count for either ECT(0) or ECT(1)

Can fail?  Isn't it required to fail?

-Processing ECN counts out of order can result in validation failure.  An
-endpoint SHOULD skip ECN validation on an ACK frame that does not increase the
-largest acknowledged packet number.
+ECN validation can fail if the received total count for either ECT(0) or ECT(1)
+exceeds the total number of packets sent with each corresponding ECT codepoint.
+In particular, validation will fail when an endpoint receives a non-zero ECN
+count corresponding to an ECT codepoint that it never applied.  This check
+detects when packets are remarked to ECT(0) or ECT(1) in the network.

```suggestion
detects when packets are marked ECT(0) or ECT(1) by the network.
```

-- 
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/4059#pullrequestreview-485432064

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 09 Sep 2020 17:47:07 -0700

@janaiyengar commented on this pull request.

 ### ECN Counts

Perhaps the following makes it easier? "requires the sending endpoint to write an ECN codepoint to IP headers and the receiving endpoint to read the ECN ..."

Do we define what "processed" means? We probably should.

This isn't a departure from existing text -- see lines 3953-3956 in the old text.

What are you undo'ing?

Remarking means that the network marks the bits to something else. The point here is that something in the  network marked the bits to CE, and now another element is remarking it to something else. Perhaps "erasing of ECN-CE markings"?

This is a reword of lines 3992-3996. I don't remember why this was written as a MAY earlier (as against a MUST, or just "fails"), but the new language is changing phrasing to "can" from "MAY". @martinthomson might remember why this isn't a failure requirement.

"Remarked" is important here, since it means that the endpoint marked packets as ECT(0) or ECT(1), and the network remarked it to one of those.

-- 
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/4059#discussion_r485991483

---

From: ianswett <[email-masked]>
Date: Wed, 09 Sep 2020 18:41:32 -0700

@ianswett commented on this pull request.

 ### ECN Counts

I was also unclear on what "Use of ECN" means, so I wanted to specify that as well.

-- 
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/4059#discussion_r486012499

---

From: ianswett <[email-masked]>
Date: Wed, 09 Sep 2020 18:46:01 -0700

Something to indicate the bits were cleared is important, as the protocol can detect marking of packet that shouldn't be marked in most cases.

-- 
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/4059#discussion_r486013709

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 10 Sep 2020 07:31:02 +0000 (UTC)

### ECN Counts

Can I suggest: /To enable the congestion controller to react to CE-marked packets, the sender needs to be able to set an ECT codepoint in the IP header and the receiver needs to be able to read the ECN field from an IP packet./
...i.e.,  removing /in order/ ... /ECN code point/ is the /ECN-field/ or name the code point value(s).

-- 
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/4059#discussion_r486124740

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 00:31:36 -0700

69f1f5ff3d8718e0ce14ee9a394dda66693a4091  Ian had some good suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/05e3f8505c26566dae9a42c6acc56c6492f61912..69f1f5ff3d8718e0ce14ee9a394dda66693a4091

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 00:31:53 -0700

+increases the corresponding ECT(0), ECT(1), or CE count, These ECN counts are

```suggestion
increases the corresponding ECT(0), ECT(1), or CE count. These ECN counts are
```

 ### ECN Counts

This section is about feedback only, so I think that it is OK to only talk about reading the markings.

That's true, except that this invalidates the other part of this paragraph...  This needs more thought.

I don't want to say negotiates.  And I'm not sure that we need the extreme economy you propose otherwise.

How about:

```suggestion
an ACK frame. This is why ECN counts are permitted to be larger than the total
number of packets that are acknowledged.
```
I don't think we need to say "packet number space" again here.

-- 
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/4059#pullrequestreview-485649924

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 00:32:11 -0700

1801df5a983519ae72650ae87a1c3c1cbeb7b6bf  Period

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/69f1f5ff3d8718e0ce14ee9a394dda66693a4091..1801df5a983519ae72650ae87a1c3c1cbeb7b6bf

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 10 Sep 2020 00:32:59 -0700

/ECN codepoint/ECN field/

-- 
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/4059#discussion_r486125829

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 00:35:58 -0700

Is this throughout?  Because we have lots of use that should be "ECN field" if that is the case.

-- 
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/4059#discussion_r486127643

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 10 Sep 2020 00:36:05 -0700

I also think the original is much clearer.

-- 
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/4059#discussion_r486127687

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 00:38:25 -0700

1a28ce1acb4173c89198c6867b93eb7228e0dc19  ECN field when talking about the field, marking or codepoint only for values
4333941fb711e92505f284aa24df9d2f5833d396  Just one per

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/1801df5a983519ae72650ae87a1c3c1cbeb7b6bf..4333941fb711e92505f284aa24df9d2f5833d396

---

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 10 Sep 2020 01:13:34 -0700

So, I’m going to chime-in here. Indeed it was in the “original” text, which is why I included this. However, we now have several people (me included) who doubt this is yet correct, so here are my thoughts:

(1)  I think it *is* important to be robust and describe how the protocol falls back in the corner case where the path drops IP packets with a non-zero ECN field - and we should very clearly say this.
(2) I think this can actually happen, but we should *not* make it sound like this often happens in general. I’m confident this is not “normal”.
(3) If we need to *measure* the path characteristic, using 10 packets seems a good test to me. However, as a fall back detection mechanism in a transport protocol this seems odd to stop using ECN after 10 packets when we expect that ECT is forwarded. Note: The text says {{ecn-ack}} says 10, the current text does not.

So.... This is a proposed change: We seem to be converging on clear text, would it be possible to remove the "10 packets" and revise this mechanism to say that the ECN counts need to increment, and pathological loss with no incrementing of counts is an indication that the sender should become “failed” for this connection? I think the text already virtually says this.

-- 
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/4059#discussion_r486150116

---

From: Martin Thomson <[email-masked]>
Date: Thu, 10 Sep 2020 02:22:29 -0700

96c8c6f30f586bec51ef06ec607e980ee7305b86  Reword again

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/4333941fb711e92505f284aa24df9d2f5833d396..96c8c6f30f586bec51ef06ec607e980ee7305b86

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 10 Sep 2020 17:17:57 -0700

"Pathological loss" isn't going to be good enough as a mechanism, and there's a rathole waiting there to define precisely what "pathological" means. We chose 10 packets based on the IW, and also because we wanted to start somewhere and then change the mechanism as we learned more through experience.

I suggest that we don't muddle this particular (large) PR with an additional, potentially-contentious discussion about a mechanism change.

-- 
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/4059#discussion_r486702102

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 10 Sep 2020 17:20:36 -0700

The bits aren't necessarily cleared, they could be set to ECT(0) or ECT(1) by the network and that can be detected. This is why remarking is a better word.

-- 
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/4059#discussion_r486702800

---

From: Gorry Fairhurst <[email-masked]>
Date: Fri, 11 Sep 2020 01:33:07 -0700

I do like Jana's proposal to not muddle this particular (large) PR with this "issue". Would others agree to finalise the PR and then discuss this as a separate topic - afterwards?

-- 
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/4059#discussion_r486860000

---

From: mirjak <[email-masked]>
Date: Fri, 11 Sep 2020 07:17:43 -0700

I think we discussed this all ready once. I'm fine with 10 packet as it's the initial window (but maybe that's what we should say), however, given the result is the same I would rather state it as enable ECT marking and if all packet of your first flight are lost validation fails and you disable. Describing this specifically as probably makes it just sounds more complicated that it is.

-- 
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/4059#discussion_r487075657

---

From: ianswett <[email-masked]>
Date: Fri, 11 Sep 2020 10:39:34 -0700

The github diff didn't line up well, hence my confusion.  The way this is described still seems a bit unrealistic to me, but I can see this PR doesn't substantively change it, so I'm fine with that.

-- 
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/4059#discussion_r487192747

---

From: ianswett <[email-masked]>
Date: Fri, 11 Sep 2020 10:48:02 -0700

If I'm reading correctly, the preceding two sentences describe how to detect if ECN bits have been set to 0.  Detecting remarking would indicate to me that you can also detect a bit that was 0 set to 1, but the next paragraph explains why it's ok for the counts to be larger than expected.

That being said, it sounds like people like the word 'remarked' when talking about ECN, so I'm fine to keep it as is.

-- 
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/4059#discussion_r487197110

---

From: ianswett <[email-masked]>
Date: Fri, 11 Sep 2020 10:50:40 -0700

Ok, happy to fix this in a separate PR if that makes sense?

-- 
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/4059#discussion_r487198440

---

From: ianswett <[email-masked]>
Date: Fri, 11 Sep 2020 10:57:14 -0700

-Note that supporting ECN requires being able to set and read the ECN codepoints
-in the IP headers of datagrams carrying QUIC packets. On platforms where
- this is not possible, QUIC cannot support ECN.
-
-To use ECN, QUIC endpoints first determine whether a path supports ECN marking
-and the peer is able to access the ECN codepoint in the IP header.  A network
-path does not support ECN if ECN marked packets get dropped or ECN markings are
-rewritten on the path. An endpoint validates the use of ECN on the path, both
-during connection establishment and when migrating to a new path
-({{migration}}).
+To enable ECN, a sending QUIC endpoint first determines whether a path supports
+ECN marking and whether the peer reports the ECN values in received IP headers;
+see {{ecn-validation}}.

 ### ECN Counts

```suggestion
### Reporting ECN Counts
```

-- 
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/4059#pullrequestreview-487010467

---

From: Martin Thomson <[email-masked]>
Date: Sun, 13 Sep 2020 15:39:20 -0700

5c39ab2cfa12a68c48aacc4e4d8e51379bebe61d  Update heading

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/96c8c6f30f586bec51ef06ec607e980ee7305b86..5c39ab2cfa12a68c48aacc4e4d8e51379bebe61d

---

From: Martin Thomson <[email-masked]>
Date: Sun, 13 Sep 2020 23:51:53 -0700

@larseggert @ianswett, you both hold blocking reviews on this.  There are enough comments here that I wasn't able to work out whether we'd addressed those comments.  Can you check and again and let us know if this is still blocking?

-- 
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/4059#issuecomment-691854519

---

From: Lars Eggert <[email-masked]>
Date: Mon, 14 Sep 2020 04:53:53 -0700

@larseggert approved this pull request.

I flagged some minor nits, but otherwise good to go.

-### ECN Validation {#ecn-validation}
+### Reporting ECN Counts
+
+Use of ECN requires the receiving endpoint to read the ECN field from an IP
+packet, which is not possible on all platforms. If an endpoint does not
+implement ECN support or does not have access to received ECN field, it

```suggestion
implement ECN support or does not have access to received ECN fields, it
```

-### ECN Validation {#ecn-validation}
+### Reporting ECN Counts
+
+Use of ECN requires the receiving endpoint to read the ECN field from an IP
+packet, which is not possible on all platforms. If an endpoint does not
+implement ECN support or does not have access to received ECN field, it
+does not report ECN counts for packets it receives.
+
+Even if an endpoint does not set an ECT field on packets it sends, the endpoint
+MUST provide feedback about ECN markings it receives, if these are accessible.
+Failing to report the ECN counts will cause the sender to disable use of ECN
+for packets to this receiver.

```suggestion
for this connection.
```

-It is possible for faulty network devices to corrupt or erroneously drop packets
-with ECN markings.  To provide robust connectivity in the presence of such
-devices, each endpoint independently validates ECN counts and disables ECN if
-the path is not showing consistent support for ECN.
+Each packet number space maintains separate acknowledgement state and separate
+ECN counts.  Coalesced QUIC packets (see {{packet-coalesce}}) share the same IP
+header so the ECN counts are incremented once for each QUIC packet.

```suggestion
header so the ECN counts are incremented once for each coalesced QUIC packet.
```

-It is possible for faulty network devices to corrupt or erroneously drop packets
-with ECN markings.  To provide robust connectivity in the presence of such
-devices, each endpoint independently validates ECN counts and disables ECN if
-the path is not showing consistent support for ECN.
+Each packet number space maintains separate acknowledgement state and separate
+ECN counts.  Coalesced QUIC packets (see {{packet-coalesce}}) share the same IP
+header so the ECN counts are incremented once for each QUIC packet.
+
+For example, if one each of an Initial, Handshake, and 1-RTT QUIC packet are
+coalesced into a single UDP datagram, the ECN counts for all three packet
+number spaces will be incremented by one each.

```suggestion
number spaces will be incremented by one each, based on the ECN field of the single IP header.
```

-To reduce the chances of misinterpreting congestive loss as packets dropped by a
-faulty network element, an endpoint could set the ECT(0) codepoint for only the
-first ten outgoing packets on a path, or for a period of three RTTs, whichever
-occurs first.
+If an endpoint has cause to expect that IP packets with an ECT codepoint might
+be dropped by faulty network element, the endpoint could set an ECT codepoint

```suggestion
be dropped by a faulty network element, the endpoint could set an ECT codepoint
```

-Erroneous application of ECN marks in the network can result in degraded
-connection performance.  An endpoint that receives an ACK frame with ECN
-counts therefore validates the counts before using them. It performs this
-validation by comparing newly received counts against those from the last
-successfully processed ACK frame. Any increase in ECN counts is validated
-based on the markings that were applied to packets that are newly
-acknowledged in the ACK frame.
+Erroneous application of ECN-CE markings by the network can result in degraded

```suggestion
Erroneous application of CE markings by the network can result in degraded
```

> +If an ACK frame newly acknowledges a packet that the endpoint sent setting
+either the ECT(0) or ECT(1) codepoint, ECN validation fails if the

```suggestion
If an ACK frame newly acknowledges a packet that the endpoint sent with
either the ECT(0) or ECT(1) codepoint set, ECN validation fails if the
```

> +network element that zeroes the ECN field or a peer that is unable to access
+ECN the markings.

```suggestion
network element that zeroes the ECN field or a peer that does not report 
ECN markings.
```

-- 
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/4059#pullrequestreview-487669401

---

From: ianswett <[email-masked]>
Date: Mon, 14 Sep 2020 15:22:44 +0000 (UTC)

I think this is technically legal, so I'm suggesting a note about how it's extremely unlikely.

-- 
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/4059#discussion_r488018168

---

From: ianswett <[email-masked]>
Date: Mon, 14 Sep 2020 08:25:51 -0700

```suggestion
number spaces will be incremented by one each.  Due to when keys are discarded, it's extremely unlikely a packet with all 4 types will arrive in the same datagram, but this forms a complete example.
```

-- 
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/4059#pullrequestreview-487888160

---

From: ianswett <[email-masked]>
Date: Mon, 14 Sep 2020 08:26:41 -0700

@ianswett approved this pull request.

I made a suggestion on the 4 packets in a datagram issue that resolves it for me.  PTAL and otherwise I think this is fine.

-- 
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/4059#pullrequestreview-487889858

---

From: Martin Thomson <[email-masked]>
Date: Mon, 14 Sep 2020 16:51:56 -0700

0bcbe5c9e796b092014a99177c688d03ba421036  Lars' review suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/5c39ab2cfa12a68c48aacc4e4d8e51379bebe61d..0bcbe5c9e796b092014a99177c688d03ba421036

---

From: Martin Thomson <[email-masked]>
Date: Mon, 14 Sep 2020 16:52:42 -0700

Ian, this lost your addition (regarding the four types).  Do you think that we still need that?

Isn't this based on old text?  This example lists three types only, which is possible (our stack can do three at the server in some cases).

-- 
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/4059#pullrequestreview-488229660

---

From: ianswett <[email-masked]>
Date: Mon, 14 Sep 2020 17:00:18 -0700

> Ian, this lost your addition (regarding the four types). Do you think that we still need that?

Nope, your change is probably preferable.  I should have re-read the text more closely before posting the suggestion.

-- 
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/4059#issuecomment-692380088

---

From: Martin Thomson <[email-masked]>
Date: Mon, 14 Sep 2020 17:02:23 -0700

fbfd077a8f5dcfebdd520384239a7a42f1937abd  Wrap

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4059/files/0bcbe5c9e796b092014a99177c688d03ba421036..fbfd077a8f5dcfebdd520384239a7a42f1937abd

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 15 Sep 2020 15:15:30 -0700

Merged #4059 into master.

-- 
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/4059#event-3770527820