Re: [quicwg/base-drafts] ECN verification text (#2752)

[Martin Thomson <[e-mail]>]

martinthomson requested changes on this pull request.

One big problem, the rest looks like a solid improvement.

>  
-Each endpoint independently verifies and enables use of ECN by setting the IP
-header ECN codepoint to ECN Capable Transport (ECT) for the path from it to the
-other peer. Even if not setting ECN codepoints on packets it transmits, the
-endpoint SHOULD provide feedback about ECN markings received (if accessible).
+It is possible for faulty network devices to corrupt or erroneously drop packets
+which have ECN markings.  To provide robust connectivity in the presence of such

```suggestion
with ECN markings.  To provide robust connectivity in the presence of such
```
CMOS nit

>  
-Each endpoint independently verifies and enables use of ECN by setting the IP
-header ECN codepoint to ECN Capable Transport (ECT) for the path from it to the
-other peer. Even if not setting ECN codepoints on packets it transmits, the
-endpoint SHOULD provide feedback about ECN markings received (if accessible).
+It is possible for faulty network devices to corrupt or erroneously drop packets
+which have ECN markings.  To provide robust connectivity in the presence of such
+devices, each endpoint independently validates, and enables or disables ECN

```suggestion
devices, each endpoint independently validates ECN counts and disables ECN
if errors are detected.
```

>  
-To verify both that a path supports ECN and the peer can provide ECN feedback,
-an endpoint sets the ECT(0) codepoint in the IP header of all outgoing
-packets {{!RFC8311}}.
+ECN validation is local to a path, and MUST be independently performed for each

```suggestion
Endpoints validate ECN for packets sent on each network path independently.
```

>  
-To verify both that a path supports ECN and the peer can provide ECN feedback,
-an endpoint sets the ECT(0) codepoint in the IP header of all outgoing
-packets {{!RFC8311}}.
+ECN validation is local to a path, and MUST be independently performed for each
+path from an endpoint to its peer.  An endpoint thus independently validates ECN

```suggestion
An endpoint thus validates ECN
```

>  
-If an ECT codepoint set in the IP header is not corrupted by a network device,
-then a received packet contains either the codepoint sent by the peer or the
-Congestion Experienced (CE) codepoint set by a network device that is
-experiencing congestion.
+Even if an endpoint does not use ECN markings on packets it transmits, the
+endpoint SHOULD provide feedback about ECN markings received from the peer if

No need to SHOULD here.

```suggestion
endpoint MUST provide feedback about ECN markings received from the peer if
```

>  
-If an ECT codepoint set in the IP header is not corrupted by a network device,
-then a received packet contains either the codepoint sent by the peer or the
-Congestion Experienced (CE) codepoint set by a network device that is
-experiencing congestion.
+Even if an endpoint does not use ECN markings on packets it transmits, the
+endpoint SHOULD provide feedback about ECN markings received from the peer if
+they are accessible. Not doing so will cause the peer to disable ECN marking.

```suggestion
they are accessible. Failing to report ECN counts will cause the peer to disable ECN marking.
```

> +To start ECN validation, an endpoint SHOULD do the following when sending
+packets on a new path to a peer:
+
+* Set the ECT(0) codepoint in the IP header of early outgoing packets sent on a
+  new path to the peer {{!RFC8311}}.
+
+* If all packets that were sent with the ECT(0) codepoint are eventually deemed
+  lost {{QUIC-RECOVERY}}, validation is deemed to have failed.
+
+To reduce the chances of misinterpreting congestive loss as packets dropped by a
+faulty network element, an endpoint could set the ECT(0) codepoint in the first
+ten outgoing packets on a path, or for a period of three RTTs, whichever occurs
+first.  Alternate strategies are possible.  For example, an endpoint could send
+the first ten packets interleaved: five ECT(0)-marked packets interleaved with
+five unmarked packets.  This allows the endpoint to more clearly identify
+congestive loss as such.  Implementations MAY experiment with and use other

I'd drop the "For example" and "This allows the endpoint to more clearly identity" sentences.  Also the "Alternate strategies" as well.

> +
+* If all packets that were sent with the ECT(0) codepoint are eventually deemed
+  lost {{QUIC-RECOVERY}}, validation is deemed to have failed.
+
+To reduce the chances of misinterpreting congestive loss as packets dropped by a
+faulty network element, an endpoint could set the ECT(0) codepoint in the first
+ten outgoing packets on a path, or for a period of three RTTs, whichever occurs
+first.  Alternate strategies are possible.  For example, an endpoint could send
+the first ten packets interleaved: five ECT(0)-marked packets interleaved with
+five unmarked packets.  This allows the endpoint to more clearly identify
+congestive loss as such.  Implementations MAY experiment with and use other
+strategies.
+
+#### Receiving ACK Frames
+
+An endpoint that sets ECT(0) or ECT(1) codepoints on packets it transmits MUST

We only mention ECT(0) up to here.  I think that we want to either mention ECT(1) in the previous section (under experimentation) or warm up to it somehow.

>  
 * Any increase in either ECT(0) or ECT(1) counts, plus any increase in the CE
   count, MUST be no smaller than the number of packets sent with the
   corresponding ECT codepoint that are newly acknowledged in this ACK frame.
   This step detects any erroneous network remarking from ECT(0) to ECT(1) (or
   vice versa).
 
+Processing counts out of order can result in validation failure.  An endpoint

```suggestion
Processing ECN counts out of order can result in validation failure.  An endpoint
```

>  
 * Any increase in either ECT(0) or ECT(1) counts, plus any increase in the CE
   count, MUST be no smaller than the number of packets sent with the
   corresponding ECT codepoint that are newly acknowledged in this ACK frame.
   This step detects any erroneous network remarking from ECT(0) to ECT(1) (or
   vice versa).
 
+Processing counts out of order can result in validation failure.  An endpoint
+SHOULD NOT perform this validation if the ACK frame is received in a packet with
+packet number lower than a previously received ACK frame.  Validating based on

You still have the packet number thing that we talked about removing in favour of increased largest 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/2752#pullrequestreview-271177366