Subject: [tcpm] FW: Review of draft-bagnulo-tcpm-generalized-ecn (David Black)

From: "Black, David" <[email-masked]>
Date: Fri, 7 Apr 2017 15:46:16 +0000

I sent this review to the draft authors privately, and they asked that I forward to the TCPM list; this version had been edited slightly from what I sent to the draft authors.  Please cc: me directly on any responses or follow-up emails, as I'm not currently on the TCPM list.

In general, I think this draft is headed in the right direction.  I've limited this review to items with at least moderate technical impact.  I apologize that it's taken over 4 months to get this review done - the day job has been putting much more pressure on my IETF activity that expected since the start of this year.

Overall concern: Abstract and Introduction should make it clear  that this draft is primarily about Accurate ECN, with applicability to vanilla RFC 3168 ECN being an open question for further discussion.  It's also not clear to me whether applicability to vanilla RFC 3168 ECN is a single global decision for all packet types vs. a per-packet-type decision.

-- Section 1.3

OLD
   RFC3168 does not prevents from using ECN in TCP
   control packets lightly.  It provides a number of specific reasons
   for each packet type.  In this Section 4, we revisit each of the
   arguments provided by RFC3168 and explore possibilities to enable the
   ECN capability in the different packet types.
NEW
   RFC3168 does not prohibit use of ECN for TCP
   control packets lightly.  It provides a number of specific reasons why
   ECN should not be used for each packet type.  In this Section 4, we analyze
   each of the reasons provided by RFC3168 as part of explaining why this
   experiment is reasonable to conduct for each of the different packet types.

Last sentence in old text is too weak - "explore possibilities" doesn't sound like providing a solid rationale for experimentation.

-- Section 2

OLD
   Retransmission: A TCP segment that has been retransmitted by the TCP
   sender because it determined that the original segment was lost,
   which may or may not be the case.
NEW
   Retransmission: A TCP segment that has been retransmitted by the TCP
   sender.

Shortened definition to improve clarity.

-- Section 3.1

This section is problematic as written, as it appears to require a router to parse TCP packets and maintain enough state to detect retransmissions.  That's surely not what was intended ;-).   This section should be rewritten and dramatically shorted to require that routers treat ECT-marked TCP control packets and retransmissions just like any other ECT-marked packet, as specified in RFC 3168 (and don't repeat lengthy RFC 3168 text here).

The reason to rewrite this section to point to RFC 3168 is that any divergence from RFC 3168 network node behavior for other ECT-marked packets is likely to not only be a bad idea, but to also be fatal to this proposal.  It's ok to cite RFCs beyond 3168 if needed to specify  the network node behavior.

-- Section 3.2.1.1

   The server SHOULD not set any of the ECT codepoints if
   the server is included in the cache as not supporting ECN in the SYN
   packet.

Which cache?  The natural reading of this text is "the client's cache" in which case this requirement is unimplementable as written because the server can't see the client's cache.  I suspect that the actual requirement is that the server not change its ECN behavior until any client cache entry for it is certain to have aged out (how long would that be? provide an answer ...).   There appears to be an analogous concern with the server cache in 3.2.1.2.

--Section 3.2.2.1

   Also, it should be noted than if an ACK
   is dropped due to congestion the sender of the ACK does not react by
   reducing the load in any way.

Given that statement, much of the discussion of how to reduce sender load on the network may be irrelevant.   In the quoted sentence, at  least the first instance of "ACK" should be "pure ACK".

-- Section 3.2.3.1

I think the reduction in offered load discussion in this section is pointless; the sender should behave as if the Zero Window Probe were dropped.

-- End of Section 3.2

This would be a good place to summarize sender and receiver behavior changes.  That summary is needed anyhow, as the changes to RFC 3168 need to be clearly called out, and preferably summarized in one place.

-- Section 4.1

   In particular, setting the CE
   codepoint in the very same packet seem to fulfill this criteria,
   since either the packet is delivered and the CE codepoint signal is
   delivered to the endpoint, or the packet is dropped, so the original
   congestion signal through the packet loss is delivered to the
   endpoint.

That assumes that "the very same packet" will be retransmitted, which is correct for data packets, but not control packets (e.g., a pure ACK  or a zero window probe).  What may be missing (or not well stated in the rest of this paragraph) is that drops of these packets don't cause congestion reactions. 

   As currently specified, ECN adoption implies an increased reliability
   of the ECN congestion signal and a decrease in the reliability in the
   TCP control packets.  We believe that it is possible and desirable to
   restore the tradeoff existent in non ECN capable networks in terms of
   reliability, where the congestion signal delivery is as reliable as
   in a non ECN capable network and so it is the delivery of TCP control
   packets.

Add a section reference for decrease in reliability - to Section 1.1 ??

Thanks, --David
--------------------------------------------------------
David L. Black, Distinguished Engineer
Dell EMC, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953     Cell: +1 (978) 394-7754
[email-masked]  <=== NEW ===
--------------------------------------------------------

---

From: Bob Briscoe <[email-masked]>
Date: Fri, 7 Apr 2017 23:05:15 +0100

David,

Thank you for these useful comments.
Detailed responses inline.

In summary, I think all your proposed changes improve clarity. I don't 
think you've disagreed with any technical effect of the proposed protocol.

On 07/04/17 16:46, Black, David wrote:
> Overall concern: Abstract and Introduction should make it clear  that this draft is primarily about Accurate ECN, with applicability to vanilla RFC 3168 ECN being an open question for further discussion.
[BB] We obviously need to clarify that AccECN is solely relevant to 
SYNs, and all other control packets could be made ECN-capable whether or 
not AccECN was being used. Actually, AccECN is only mentioned in the 
sections about SYNs. However, I accept that it would help to point that 
out explicitly. And to explain why. Here's why:

In RFC3168 ECN feedback, the Echo Congestion Experienced (ECE) flag is 
used for capability negotiation during the 3WHS. So, even if the client 
makes the SYN ECN-capable and then it's marked CE, the SYN-ACK has 
nowhere for the server to feed this back. In contrast, an AccECN server 
supports feedback in the SYN/ACK of any CE on the SYN.

The relevant sections of AccECN are:
https://tools.ietf.org/html/draft-ietf-tcpm-accurate-ecn-02#section-2.5
https://tools.ietf.org/html/draft-ietf-tcpm-accurate-ecn-02#section-3.1

BTW, there is one mention of AccECN in the section on Pure ACKs as well 
- necessary to consider whether AccECN would count CE on Pure ACKs. 
We'll clarify that this is not saying that AccECN is necessary.

> It's also not clear to me whether applicability to vanilla RFC 3168 ECN is a single global decision for all packet types vs. a per-packet-type decision.
[BB] This is a consequence of the process of reaching consensus.

Initially, we had to assume that someone might come up with a good 
argument against setting ECT on one of the types. So it is written as if 
the decision on each type is independent. Nonetheless, I think it would 
now make sense to add a sentence saying:

To comply with this spec, a TCP sender will set ECT on all types of 
control packets.

However, RFC3168 allows an ECN sender to choose not to set ECT on any 
individual packet (although it gives no example reasons why a sender 
would not set ECT). So I don't think we don't need to /require/ a TCP 
sender to set all types of packet to ECT.

> Shortened definition to improve clarity.
[BB] AOK so far
> -- Section 3.1
>
> This section is problematic as written, as it appears to require a router to parse TCP packets and maintain enough state to detect retransmissions.  That's surely not what was intended ;-).
[BB] Indeed. We intended this to apply to any middlebox (e.g. firewall) 
that already parses the TCP header.
> This section should be rewritten and dramatically shorted to require that routers treat ECT-marked TCP control packets and retransmissions just like any other ECT-marked packet,
[BB] I agree (and I hope my co-author does) that it would be clearer 
just to say this.
> as specified in RFC 3168 (and don't repeat lengthy RFC 3168 text here).

[BB] Unfortunately, RFC3168 does not specify anything about network node 
forwarding behaviour wrt ECT. So some firewall policies could have 
decided to block any TCP control packets that contravene RFC3168. That's 
why we wrote this section.

However I admit that, by not explaining that DPI was the problem we were 
trying to solve, it reads like we are advocating DPI, rather than 
removing any ambiguity that might have been interpreted as a need for 
DPI. We'll fix this.
> The reason to rewrite this section to point to RFC 3168 is that any divergence from RFC 3168 network node behavior for other ECT-marked packets is likely to not only be a bad idea, but to also be f…
[BB] As above, no network node behaviour wrt ECT is specified in RFC 
3168 (unless you can point to it - I just checked again, and I couldn't 
find it).
> Which cache?  The natural reading of this text is "the client's cache" in which case this requirement is unimplementable as written because the server can't see the client's cache.  I suspect that the actual requirement is that the server not change its ECN behavior until any client cache entry for it is certain to have aged out (how long would that be? provide an answer ...).
[BB] This is a simple typo. I'm sure the sentence was intended to start 
"The client SHOULD...", not "The server SHOULD..." (The section is 
entitled "TCP client behaviour"). Sorry about that.
> There appears to be an analogous concern with the server cache in 3.2.1.2.
[BB] I'm not sure I agree with the need for a server cache about clients 
at all. The incoming SYNs unambiguously say what type of client they 
came from.

I will check with my co-author why this sentence is here.
> Given that statement, much of the discussion of how to reduce sender load on the network may be irrelevant.
[BB] I agree. We will put this sentence first. Then say that ECT on pure 
ACKs could be used for ACK CC and refer to RFC 5690 rather than 
describing a possible ACK CC scheme here. Then make the point that an 
ECN connection not implementing ACK CC is hardly worse than a non-ECN 
connection not implementing ACK CC, both of which are the norm.
> In the quoted sentence, at  least the first instance of "ACK" should be "pure ACK".
[BB] Yup.
>
> -- Section 3.2.3.1
>
> I think the reduction in offered load discussion in this section is pointless;
[BB] The text is sort-of saying that. But I admit it could be clearer - 
we'll sort it.
> the sender should behave as if the Zero Window Probe were dropped.
[BB] Well, I don't know that any TCP implementation does anything 
specific in this case. And I certainly can't find anything written in 
RFC793 or RFC5681 about this.
> This would be a good place to summarize sender and receiver behavior changes.  That summary is needed anyhow, as the changes to RFC 3168 need to be clearly called out, and preferably summarized in…
[BB] Good, yes.
> That assumes that "the very same packet" will be retransmitted, which is correct for data packets, but not control packets (e.g., a pure ACK  or a zero window probe).  What may be missing (or not w…
[BB] Yes, we'll make this point clearer.
> Add a section reference for decrease in reliability - to Section 1.1 ??
[BB] Will do.

Thanks again,

Bob
> _______________________________________________
> tcpm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tcpm
>

-- 
________________________________________________________________
Bob Briscoe                               http://bobbriscoe.net/

---

From: "Black, David" <[email-masked]>
Date: Fri, 7 Apr 2017 22:33:08 +0000

Bob,

Two comments:

1) I'll wait for new text before commenting further on Accurate ECN vs. vanilla RFC 3168 ECN.  The current text left me somewhat confused, and the summary of changes from RFC 3168 will almost certainly help.

2) On network node treatment of ECT, RFC 3168 is brief, e.g. (Section 5, top of p.7):

   The CE codepoint '11' is set by a router to indicate congestion to
   the end nodes.  

One wants to avoid text that could be read as modifying that sentence by adding possible exceptions for TCP control packets and/or retransmissions.

> why we wrote this section.

If "firewalls" is what was meant, use that word ;-) ... or as you write ...

> trying to solve, rather than removing any ambiguity that might have been
> interpreted as a need for DPI. We'll fix this.

In doing so, please keep in mind that in this draft, discussion of forwarding  and dropping behavior is implicitly about router queue management, not middlebox access control (e.g., based on DPI), so access control has to be called out explicitly when that is what is meant.

Thanks, --David

> -----Original Message-----
> From: Bob Briscoe [mailto:[email-masked]]
> Sent: Friday, April 7, 2017 6:05 PM
> To: Black, David <[email-masked]>
> Cc: [email-masked]
> Subject: Re: [tcpm] FW: Review of draft-bagnulo-tcpm-generalized-ecn
> (David Black)
> 
> David,
> 
> > I sent this review to the draft authors privately, and they asked that I
> forward to the TCPM list; this version had been edited slightly from what I
> sent to the draft authors.  Please cc: me directly on any responses or follow-
> up emails, as I'm not currently on the TCPM list.
> >
> > In general, I think this draft is headed in the right direction.  I've limited this
> review to items with at least moderate technical impact.  I apologize that it's
> taken over 4 months to get this review done - the day job has been putting
> much more pressure on my IETF activity that expected since the start of this
> year.
> >
> > Overall concern: Abstract and Introduction should make it clear  that this
> draft is primarily about Accurate ECN, with applicability to vanilla RFC 3168
> ECN being an open question for further discussion.
> > It's also not clear to me whether applicability to vanilla RFC 3168 ECN is a
> single global decision for all packet types vs. a per-packet-type decision.
> > Last sentence in old text is too weak - "explore possibilities" doesn't sound
> like providing a solid rationale for experimentation.
> >
> > -- Section 2
> >
> > OLD
> > This section is problematic as written, as it appears to require a router to
> parse TCP packets and maintain enough state to detect retransmissions.
> That's surely not what was intended ;-).
> > This section should be rewritten and dramatically shorted to require that
> routers treat ECT-marked TCP control packets and retransmissions just like
> any other ECT-marked packet,
> > The reason to rewrite this section to point to RFC 3168 is that any
> divergence from RFC 3168 network node behavior for other ECT-marked
> packets is likely to not only be a bad idea, but to also be fatal to this proposal.
> It's ok to cite RFCs beyond 3168 if needed to specify  the network node
> behavior.
> > Which cache?  The natural reading of this text is "the client's cache" in
> which case this requirement is unimplementable as written because the
> server can't see the client's cache.  I suspect that the actual requirement is
> that the server not change its ECN behavior until any client cache entry for it
> is certain to have aged out (how long would that be? provide an answer ...).
> > This would be a good place to summarize sender and receiver behavior
> changes.  That summary is needed anyhow, as the changes to RFC 3168 need
> to be clearly called out, and preferably summarized in one place.
> [BB] Good, yes.
> >
> > -- Section 4.1
> >
> > That assumes that "the very same packet" will be retransmitted, which is
> correct for data packets, but not control packets (e.g., a pure ACK  or a zero
> window probe).  What may be missing (or not well stated in the rest of this
> paragraph) is that drops of these packets don't cause congestion reactions.

---

From: Bob Briscoe <[email-masked]>
Date: Sat, 8 Apr 2017 01:11:51 +0100

David,

On 2) I intend to replace the whole of "3.1 Network behaviour" with the 
following, which calls out the cases explicitly:

Previously RFC3168 required the sender to set not-ECT on certain TCP 
control packets. Some readers might have erroneously interpreted this 
aspect of RFC3168 as a requirement for firewalls, intrusion detection 
systems, etc. to check and enforce this behaviour. Now that the present 
experimental specification allows TCP senders to set ECT on all TCP 
packets (control and data), it needs to be clear that a firewall (or any 
network node) SHOULD NOT treat any ECT packet differently dependent on 
what type of TCP packet it is.

One potential exception is envisaged, otherwise the previous sentence 
could have said "MUST NOT" rather than "SHOULD NOT". The exception 
allows a security function that has detected an ongoing attack to drop 
more ECT marked SYNs than not-ECT marked SYNs. Such a policy SHOULD NOT 
be applied routinely. It SHOULD only be applied if an attack is 
detected, and then only if it is determined that the ECT capability is 
intensifying the attack.

Bob

---

From: Bob Briscoe <[email-masked]>
Date: Fri, 14 Apr 2017 02:13:58 +0100

David,

I hope you will find that I have addressed all your concerns - so at 
least you will be able to understand it now.
The draft is now unexpired.

I have also addressed all the points in Mirja's review (some 
intersecting with your points).

You will see that it's actually turned into quite a major re-write. The 
diff is nearly total, mainly cos I was turning Spanglish into English as 
I went (Marcelo, it was quite understandable, but just not quite how a 
native English speaker would say it).

However, I have changed technical points in a few places too - the more 
one thinks about this, the more depth one finds.

I just noticed I missed one of your nits (shortening the definition of 
Retransmission), which I have already fixed in my local copy of the xml 
for the next rev.

Bob

On 08/04/17 01:11, Bob Briscoe wrote:
> David,
>
> On 2) I intend to replace the whole of "3.1 Network behaviour" with 
> the following, which calls out the cases explicitly:
>
> systems, etc. to check and enforce this behaviour. Now that the 
> present experimental specification allows TCP senders to set ECT on 
> all TCP packets (control and data), it needs to be clear that a 
> firewall (or any network node) SHOULD NOT treat any ECT packet 
> differently dependent on what type of TCP packet it is.
>
> more ECT marked SYNs than not-ECT marked SYNs. Such a policy SHOULD 
> NOT be applied routinely. It SHOULD only be applied if an attack is 
>> 1) I'll wait for new text before commenting further on Accurate ECN 
>> vs. vanilla RFC 3168 ECN.  The current text left me somewhat 
>> confused, and the summary of changes from RFC 3168 will almost 
>> certainly help.
>>
>> One wants to avoid text that could be read as modifying that sentence 
>> by adding possible exceptions for TCP control packets and/or 
>> retransmissions.
>>
>> In doing so, please keep in mind that in this draft, discussion of 
>> forwarding  and dropping behavior is implicitly about router queue 
>> management, not middlebox access control (e.g., based on DPI), so 
>> access control has to be called out explicitly when that is what is 
>> meant.
>>
>> Thanks, --David
>>
>
>

---

From: Bob Briscoe <[email-masked]>
Date: Tue, 18 Apr 2017 10:40:50 +0100

David, Mirja, list,

Thanks again for your reviews of the -00. I've been busy over the w/e 
after thinking more deeply about some of your comments and after 
realizing that RFC3168 contained an assumption that a single pure ACK 
implies that all other packets in that direction are pure ACKs, which is 
not true in general.

We've produced another rev (draft-bagnulo-tcpm-generalized-ecn-02 
<https://tools.ietf.org/html/draft-bagnulo-tcpm-generalized-ecn-02>).

This is a general invitation for anyone on the list, including you two, 
to review this again, so we can ask for an informed adoption call.

We need people with the following expertise:
* TCP hijacking and DoS/flooding attacks
* Specific TCP implementations (Linux, FreeBSD, iOS, Windows, etc).
* All the common variants of TCP

We have identified clearly where decisions are needed.
As well as where more measurements are needed.

Main changes:
* Moved more discursive text from S.3 (Spec) to S.5 (Arguments against 
RFCs)
* Major changes to Pure ACK sections and the reliability argument, to 
address Mirja's arguments better, and to separately address cwnd 
response (required) and ACK-rate response (optional but not required).
* Referred to Network Behaviour in ecn-experimentation, rather than 
repeating it.
* Fixed description of window probe.
* Recommended RFC 5961, which gives much stronger packet validity checks 
for retransmissions, RSTs & FINs.

Cheers

Bob

On 14/04/17 02:13, Bob Briscoe wrote:
> You will see that it's actually turned into quite a major re-write. 
> The diff is nearly total, mainly cos I was turning Spanglish into 
> English as I went (Marcelo, it was quite understandable, but just not 
> quite how a native English speaker would say it).
>
> However, I have changed technical points in a few places too - the 
> more one thinks about this, the more depth one finds.
>
> Retransmission), which I have already fixed in my local copy of the 
> xml for the next rev.
>
>
>
> Bob
>
>
>> allows a security function that has detected an ongoing attack to 
>> drop more ECT marked SYNs than not-ECT marked SYNs. Such a policy 
>> SHOULD NOT be applied routinely. It SHOULD only be applied if an 
>> attack is detected, and then only if it is determined that the ECT 
>> capability is intensifying the attack.
>>
>>
>> Bob
>>
>>> One wants to avoid text that could be read as modifying that 
>>> sentence by adding possible exceptions for TCP control packets 
>>> and/or retransmissions.
>>>

---

From: Mirja Kühlewind <[email-masked]>
Date: Thu, 20 Apr 2017 19:26:45 +0200

Hi Bob,

given it seems I spend most of my time these days to worry about different 
ECN-related RFCs, I've also reviewed this draft (actually only sections 1-4; 
I may review section 5 at a later point of time but I trust you that you have 
addressed my previous comments here).

Here is my feedback as individual contributor: In general, I'm happy with the 
spec and the overall draft now. Thanks for the work! It reads very well now. 
I have a few minor technical comments and some editorial proposals and a 
proposal to restructure some smaller parts below.

So te document is in really good shape and the technical specification is 
good. So I think that this version is definitely ready for adoption and would 
like to see an adoption call very soon!

Mirja

-----------------
High-level, minor technical points:

1) The description of TFO in section 4.2 is not quite correct:
"If a TFO initiator has cached that the server supported ECN in the
    previous connection, it would be safe to set ECT on any data segments
    it sends before a SYN-ACK returns from the responder (server).  Note
    that there is no space in the SYN-ACK itself (whether classic or
    AccECN feedback has been negotiated) to include feedback about any CE
    on data packets.  Nonetheless, it is safe to set ECT on data packets
    within the handshake because any CE-marking on these data segments
    can be fed back by the responder on the first data segment it sends
    after the SYN-ACK (or on an additional pure ACK if it has no more
    data to send)."
TFO allows the initiator to send data in the SYN but no additional data 
packets, and the responder to send the SYN/ACK as well as an IW full of data 
packets. So I think the problem here goes away. However, accECN actually does 
feedback information of CE marked data in the SYN/ACK because the SYN/ACK 
carries the AccECN option that holds this information.

2) Regarding this question in section 3.2.3
"DISCUSSION: An AccECN deployment or an implementation of RFC 3168
       that feeds back CE on pure ACKs will be at a disadvantage compared
       to an RFC 3168 implementation that does not.  To solve this, the
       WG could decide to prohibit setting ECT on pure ACKs unless AccECN
       has been negotiated."
I've checked REF3168 and it does not say anything about handling of pure ACKs 
that are CE marked because it probably just assumes that that case will never 
happen. However, implementations may or may not provide feedback. I think I 
would support to only have pure ACKs ECT marked if AccECN is supported 
because of this. I also think that losing ACKs is probably less bad that 
losing any of the other (control) packets that are discussed in this 
document. So I think this restriction would be okay.

3) This is a suggestion to improve the situation, however it probably needs 
further discussion: in sect 3.2.1.4 you say that one should still use ECT 
SYNs even if a connection attempt previously failed. Maybe we can use a 
smaller time-out in this case...?

And now first a couple of comments on the structure:

1) I would move the network behavior in any case AFTER the endpoint behavior 
spec. Actually I would even propose to have it in a different section (not 
under 3. Specification) because it actually does not specify anything; it 
only comments on the current situation.

2) And then you would actually not need a separate subsection for the 
Endpoint behavior; you could just call section 3 "Specification of Endpoint 
Behavior" or even "Specification of Sender Behavior" and move the first two 
paragraphs of section 3.2 to the intro that explains that this 
mechanism/document only needs to talk about the sender behavior.

3) I would include the text on TFO and IW10 directly into section 3.2.1.3 
instead of just giving a reference to a later section.

4) The rationale in section 3.2.6.1 could also go somewhere into section 5 
(to keep the spec part as short as possible). At least I think you don't need 
the whole text here. If you want to keep something you probably only need the 
second paragraph (and not an own subsection).

5) Maybe have the section on Retransmissions (3.2.7.) earlier before RST, 
FIN, and Window probe because it seems more "important". But actually I don't 
think the order matters that much. So this is a minor comment only.

6) The paragraph on SCTP (section 4.1) could go into the intro. And if you 
then also integrate the TFP and IW10 stuff into section 3.2.1.3, section 4 
would actually go away completely.

And now a bunch of editorial comments/proposals:

1) I though that it might be good to mention AccECN already in the Intro 
because that the enabler for the SYN part...

2) section 1.1:
OLD
"Preventing TCP control
    packets from obtaining the benefits of ECN would not only expose them
    to the prevailing level of congestion loss, but it would also stop
    them from being classified into the low latency (L4S) queue, which
    would greatly degrade L4S performance."
I think this can be phrased more general:
NEW
  "Preventing TCP control
    packets from obtaining the benefits of ECN would not only expose them
    to the prevailing level of congestion loss, but it would also put control
    packet into a different queue with different network treatment, which
    may also lead to reordering and therefore can greatly degrade TCP
    performance."

3) section 3.2.1.1:
OLD
"Accurate ECN (AccECN)
    feedback [I-D.ietf-tcpm-accurate-ecn] provides two codepoints in the
    SYN-ACK"
NEW
"Accurate ECN (AccECN)
    feedback [I-D.ietf-tcpm-accurate-ecn] provides one flag in the
    SYN-ACK"
I think that's clearer, at least for me.

4) section 3.2.1.2
This sentence is correct but I had to read it twice to make sure it's correct:
"Servers that
    do not support ECN as a whole probably do not need to be recorded
    separately from non-support of AccECN because, even when a server
    does not support classic [RFC3168] ECN, there is no performance
    penalty in always requesting to use it."
NEW
"Servers that
    do not support ECN as a whole probably do not need to be recorded
    separately from non-support of AccECN. Meaning, if AccECN is noted as not 
supported by the server, the initiator may decide to not mark the SAN as ECT 
but it still may try to negotiate classic ECN or even AccECN (to quickly 
detect server upgrades)."

5) section 3.2.1.4:
OLD
"To expedite connection set-up if, after sending an ECT SYN, the
    retransmission timer expires, the TCP initiator SHOULD send a SYN
    with the not-ECT codepoint in the IP header and not attempt to
    negotiate AccECN.  It would make sense to also remove any other
    experimental fields or options on the SYN, but that will depend on
    the specification of the other option(s). "
I wouldn't make any statements about things that are not in scope of this 
doc, so I would remove the later part (including fallback for AccECN):
NEW
"To expedite connection set-up if, after sending an ECT SYN, the
    retransmission timer expires, the TCP initiator SHOULD send a SYN
    with the not-ECT codepoint in the IP header."

6) section 3.2.2 says:
"In either case no change is required to RFC 5562 or the AccECN specification."

It would probably be good to also state in this draft what the procedure is 
that is described in RFC5562:
"When the responder (node B) receives the ECN-Echo packet reporting
    the Congestion Experienced indication in the SYN/ACK packet, the
    responder sets the initial congestion window to one segment, instead
    of two segments as allowed by [RFC2581], or three or four segments
    allowed by [RFC3390].  As illustrated in Figure 2, if the responder
    (node B) receives an ECN-Echo packet informing it of a Congestion
    Experienced indication on its SYN/ACK packet, the responder sends a
    SYN/ACK packet that is not ECN-Capable, in addition to setting the
    initial window to one segment.  The responder does not advance the
    send sequence number.  The responder also sets the retransmission
    timer.  The responder follows RFC 2988 [RFC2988] in setting the RTO
    (retransmission timeout)."

7) section 3.2.3:
OLD
"It MAY also
    implement AckCC [RFC5690] to regulate the pure ACK rate, but this is
    not required.  Note that, in comparison, [RFC5681] does not require..."
NEW
"It MAY also
    implement Acknowledgement Congestion Control (AckCC) [RFC5690] to 
regulate the pure ACK rate, but this is
    not required.  Note that, in comparison, TCP Congestion Control [RFC5681] 
does not require..."

8) section 3.2.3:
"The question of whether the receiver of pure ACKs is required to feed back 
any CE marks on them is a matter for the relevant feedback specification 
([RFC3168] or [I-D.ietf-tcpm-accurate-ecn]).  It is outside the scope of the 
present specification."
However, as mention about I believe that RFC3168 does not say anything about 
feedback how to handle CE marks of pure ACK because it assumes that this case 
never occurs. So that might be implementation dependent, so it might be 
better to spell that out:
"[I-D.ietf-tcpm-accurate-ecn] includes handling of control packets. However, 
[RFC3168] does not specify handling of pure ACKs that are CE mark, so it 
might be implementation specific if feedback is provided or not."

Related to this comment in the same section:
"DISCUSSION: Before the AccECN specification is published, a
       requirement to count CE marks on all packets could be added."
I pretty sure that this is what the AccECN draft is doing but I can double 
check. If not it clearly should be added. Maybe it's not spelled out clearly 
enough. I'll check!

9) section 3.2.6.1:
OLD
"The following factors have been considered before deciding whether
    ECT ought to be allowed on a RST message:"
NEW
"The following factors have been considered before deciding whether
    the RST message should be ECT marked:"

That's it. So nothing big. Let move on with this draft!

On 18.04.2017 11:40, Bob Briscoe wrote:
> David, Mirja, list,
>
> Thanks again for your reviews of the -00. I've been busy over the w/e after
> thinking more deeply about some of your comments and after realizing that
> RFC3168 contained an assumption that a single pure ACK implies that all other
> packets in that direction are pure ACKs, which is not true in general.
>
> This is a general invitation for anyone on the list, including you two, to
> review this again, so we can ask for an informed adoption call.
>
> * Moved more discursive text from S.3 (Spec) to S.5 (Arguments against RFCs)
> * Major changes to Pure ACK sections and the reliability argument, to address
> Mirja's arguments better, and to separately address cwnd response (required)
> and ACK-rate response (optional but not required).
> * Referred to Network Behaviour in ecn-experimentation, rather than repeating
> it.
> * Recommended RFC 5961, which gives much stronger packet validity checks for
> retransmissions, RSTs & FINs.
>
> Cheers
>
>
> Bob
>
>
>> I hope you will find that I have addressed all your concerns - so at least
>> you will be able to understand it now.
>> I have also addressed all the points in Mirja's review (some intersecting
>> with your points).
>>
>> diff is nearly total, mainly cos I was turning Spanglish into English as I
>> went (Marcelo, it was quite understandable, but just not quite how a native
>> English speaker would say it).
>>
>> However, I have changed technical points in a few places too - the more one
>> thinks about this, the more depth one finds.
>>
>> Retransmission), which I have already fixed in my local copy of the xml for
>> the next rev.
>>
>>
>>
>> Bob
>>
>>
>>> could have said "MUST NOT" rather than "SHOULD NOT". The exception allows
>>> a security function that has detected an ongoing attack to drop more ECT
>>> marked SYNs than not-ECT marked SYNs. Such a policy SHOULD NOT be applied
>>> routinely. It SHOULD only be applied if an attack is detected, and then
>>> only if it is determined that the ECT capability is intensifying the attack.
>>>
>>>
>>> Bob
>>>
>>>> 1) I'll wait for new text before commenting further on Accurate ECN vs.
>>>> vanilla RFC 3168 ECN.  The current text left me somewhat confused, and
>>>> the summary of changes from RFC 3168 will almost certainly help.
>>>>
>>>> One wants to avoid text that could be read as modifying that sentence by
>>>> adding possible exceptions for TCP control packets and/or retransmissions.
>>>>
>>>> management, not middlebox access control (e.g., based on DPI), so access
>>>> control has to be called out explicitly when that is what is meant.
>>>>
>>>> Thanks, --David
>>>>
>>>
>>>
>>
>
> --

---

From: Bob Briscoe <[email-masked]>
Date: Mon, 24 Apr 2017 20:41:34 +0100

Mirja,

Thx for the rapid and extensive re-review.

I shall get on with making the edits outlined below and, unless I see 
objections to anything in this email, I'll submit an update when done.

On 20/04/17 18:26, Mirja Kühlewind wrote:
> Hi Bob,
>
> given it seems I spend most of my time these days to worry about 
> different ECN-related RFCs, I've also reviewed this draft (actually 
> only sections 1-4; I may review section 5 at a later point of time but 
> I trust you that you have addressed my previous comments here).
>
> Here is my feedback as individual contributor: In general, I'm happy 
> with the spec and the overall draft now. Thanks for the work! It reads 
> very well now. I have a few minor technical comments and some 
> editorial proposals and a proposal to restructure some smaller parts 
> below.
>
> So te document is in really good shape and the technical specification 
> is good. So I think that this version is definitely ready for adoption 
> and would like to see an adoption call very soon!
[BB] Strongly agree :)

Seriously, I think you should make it clear whether you are saying this 
as AD or individual.

> TFO allows the initiator to send data in the SYN but no additional 
> data packets, and the responder to send the SYN/ACK as well as an IW 
> full of data packets. So I think the problem here goes away. However, 
> accECN actually does feedback information of CE marked data in the 
> SYN/ACK because the SYN/ACK carries the AccECN option that holds this 
> information.

[BB] I'll remove the whole section about TFO, and instead include a 
brief sentence explaining why TFO is not relevant in the intro to S.4. 
Thanks.

I was thinking that there is nothing in TFO [RFC7413] to preclude the 
initiator sending data segments before receiving the SYN-ACK.
However, you've made me think this through further, and I've realized 
the initiator would have to send data without the ACK flag set (or with 
the ACK flag faked but no valid ackno). Altho the TFO cookie would make 
this safe from a security perspective, it's not going to happen due to 
middleboxes.

> I've checked REF3168 and it does not say anything about handling of 
> pure ACKs that are CE marked because it probably just assumes that 
> that case will never happen. However, implementations may or may not 
> provide feedback. I think I would support to only have pure ACKs ECT 
> marked if AccECN is supported because of this. I also think that 
> losing ACKs is probably less bad that losing any of the other 
> (control) packets that are discussed in this document. So I think this 
> restriction would be okay.
[BB] OK, it's good to have one opinion on this.
We'd like to hear more opinions before changing this tho.

FYI, RFC3168 does vaguely hint that the receiver of Pure ACKs carrying 
ECT (or CE) probably ought not to reject them (but it doesn't say what 
to do with a CE mark). In S.6.1.4 it says:

       For the current generation of TCP congestion control algorithms,
       pure acknowledgement packets [...] MUST be sent with the not-ECT codepoint.

Saying "current generation" implies a future generation might change 
this, which ought to make an implementer wary of rejecting ECT on Pure ACKs.

>
> 3) This is a suggestion to improve the situation, however it probably 
> needs further discussion: in sect 3.2.1.4 you say that one should 
> still use ECT SYNs even if a connection attempt previously failed. 
> Maybe we can use a smaller time-out in this case...?
[BB] I'm wary of jumping to the conclusion that a loss is not due to 
congestion just because it's becoming more likely it could be due to a 
black hole. E.g. if there's a black hole, the probability of losing an 
ECT SYN is 100%, but if the underlying loss level is 1%, the probability 
that 2 SYNs are lost is 0.01% as an alternative.

So, when 2 ECT SYNs at different times have timed out, but non-ECT SYNs 
got through, I suppose it's possible to argue that the probability that 
the second loss was due to congestion has reduced. So the timeout could 
be reduced.

I'll add this, and flag it "FOR DISCUSSION".

> 1) I would move the network behavior in any case AFTER the endpoint 
> behavior spec. Actually I would even propose to have it in a different 
> section (not under 3. Specification) because it actually does not 
> specify anything; it only comments on the current situation.
[BB] I did consider this when I removed the normative text from this 
section. But this this section is intended to do more than just comment. 
We want it to incorporate the "SHOULD" in ecn-experimentation as part of 
the specification for this experiment. And we figured this text is short 
enough to put first, to reduce the chance it gets buried and overlooked 
operators, without distracting host implementers too much.

So I would rather make the following change than move the section:

s/So operators are encouraged to alter their firewall rules/
  /So operators are RECOMMENDED to alter their firewall rules/

Rationale: When updates to a system for an experiment are all spread 
about in different docs, it's not just helpful but also necessary to 
call out all the parts in their different docs. Similarly, we have 
incorporated RFC5562 (ECT on SYN-ACK) and AccECN into this draft - both 
as normative references.

> Endpoint behavior; you could just call section 3 "Specification of 
> Endpoint Behavior" or even "Specification of Sender Behavior" and move 
> the first two paragraphs of section 3.2 to the intro that explains 
> that this mechanism/document only needs to talk about the sender 
> behavior.
[BB] Depends on resolution of the above.
>
> 3) I would include the text on TFO and IW10 directly into section 
> 3.2.1.3 instead of just giving a reference to a later section.
[BB] This is a matter of style. I prefer to distinguish normative 
dependencies between
* dependencies that are essential,
* and then to split optional dependencies between:
   - STD
   - EXPT

The above structure forces implementers (and authors) to make 
dependencies modular, in case an experiment fails. E.g. we do not yet 
know whether an attack will be invented against TFO.

For instance, look at all the mess that has had to be cleared up because 
everyone assumed the ECN nonce experiment would succeed.

>
> 4) The rationale in section 3.2.6.1 could also go somewhere into 
> section 5 (to keep the spec part as short as possible). At least I 
> think you don't need the whole text here. If you want to keep 
> something you probably only need the second paragraph (and not an own 
> subsection).
[BB] The scope of Section 5 is (currently) countering arguments in other 
RFCs (mostly RFC3168).
ECT on RSTs wasn't mentioned in an RFC so far, which is I why I put this 
section here.

One solution would be to rename section 5 "Rationale". Then I would be 
happy to move 3.2.6.1 to section 5. I'll do that.

>
> 5) Maybe have the section on Retransmissions (3.2.7.) earlier before 
> RST, FIN, and Window probe because it seems more "important". But 
> actually I don't think the order matters that much. So this is a minor 
> comment only.
[BB] Currently the order is chosen to match the order of the lists in 
the introduction so they can be summarized as "control packets and 
retransmissions". I'll leave it as it is.
>
> 6) The paragraph on SCTP (section 4.1) could go into the intro. And if 
> you then also integrate the TFP and IW10 stuff into section 3.2.1.3, 
> section 4 would actually go away completely.
[BB] Now, with the removal of the TFO section, none of S.4 contains 
anything normative. Now it is just saying "For all these variants, 
here's why you don't need to do anything different". So I am inclined to 
move it /after/ section 5.

I certainly don't want to integrate the TFO and IW10 comments into the 
spec section, because that would just complicate the spec with text that 
essentially ends up saying "You didn't need to read this to implement 
the spec".

I also don't want to add a detailed para about SCTP to a nice 3-para 
Intro to a spec about a modification to TCP.
However, I agree that "scope" is one of the important things to cover in 
an introduction.

In summary, I would rather move this section about variants to after 
S.5. And in the intro, say this is an experimental modification to 
standards track TCP, but also flag that it discusses (non-)interaction 
with variants such as SCTP, TFO, IW10.

> 1) I though that it might be good to mention AccECN already in the 
> Intro because that the enabler for the SYN part...
[BB] OK, that would be sensible, along with the above sentence about 
variants. Will do.
>    may also lead to reordering and therefore can greatly degrade TCP
[BB] Good point. Given this sentence was in a para about L4S, I shall 
split it off and make it into a summary section.
> I think that's clearer, at least for me.
[BB] Eh? I'm not going to change this, cos the word flag would imply 
this is a bit with a meaning orthogonal to the other bits.

Here's a copy of the relevant part of the table...

    |  NS CWR ECE  |                      |
    |  0   1   0   | AccECN               |
    |  1   1   0   | AccECN (CE on SYN)   |
    |  1   0   1   | classic ECN          |
... etc

The first bit only distinguishes "whether or not the SYN arrived marked 
CE" when the 3-bit codepoint as a whole is 'X10'. So it's not a flag, 
it's 2 codepoints.

>    separately from non-support of AccECN. Meaning, if AccECN is noted 
> as not supported by the server, the initiator may decide to not mark 
> the SAN as ECT but it still may try to negotiate classic ECN or even 
> AccECN (to quickly detect server upgrades)."
[BB] I realize now that this text is all wrong. I shall attempt a rewrite.

I now realize my heading of 3.2.1.2 is wrong; it was not meant to be 
about "Caching Failed Connection Attempts". It was meant to be mostly 
about "Caching Lack of Recognition of ECT SYNs", with just a reference 
forward to caching failed connection attempts ("3.2.1.4 Fall-Back 
Following No Response to an ECT SYN").

I intended to say that, when a server is cached as not supporting 
AccECN, subsequent attempts should still request AccECN, but not with 
ECT on the SYN. Because there is no harm in always attempting to 
negotiate AccECN (as long as such requests are not black-holed), because 
the responder will still respond properly to an AccECN request if it 
only supports classic ECN or no ECN at all, without any performance 
penalty.

I shall also make the point in S.3.2.1.4 that if an ECT SYN is 
black-holed, it might still be possible to request AccECN, but without 
ECT on the SYN.

> I wouldn't make any statements about things that are not in scope of 
> this doc, so I would remove the later part (including fallback for 
> AccECN):
> NEW
[BB] I agree that I shouldn't have said to not attempt AccECN. So I will 
use your new text.

But I disagree about not even mentioning that the cause might be other 
experimental options, and to encourage the reader to read those specs as 
well. I'll use the following unless you shout:

"To expedite connection set-up if, after sending an ECT SYN, the
    retransmission timer expires, the TCP initiator SHOULD send a SYN
    with the not-ECT codepoint in the IP header.  If other experimental
    fields or options were on the SYN, it will also be necessary to follow
    their specifications for fall-back too. It would make sense to co-
    ordinate all the strategies for fall-back in order to isolate the 
specific
    cause of the problem."

> It would probably be good to also state in this draft what the 
> procedure is that is described in RFC5562:
[BB] Two responses:

1) I think it's dangerous to quote a small part of another spec. as if 
it's a good summary of the behaviour. This loses all the detail in RFC 
5562 (e.g. what if the responder implements SYN cookies, yada yada).

2) Oh dear. I had forgotten that RFC5562 specified a different protocol 
to the "ECN+" paper. It's ultra-ultra-conservative.

So, I think we had better include the original proposal in the "ECN+" 
paper within the generalized ECN draft, and include a counter-argument 
to RFC5562 as well as to RFC3168.

>    not required.  Note that, in comparison, TCP Congestion Control 
> [RFC5681] does not require..."
[BB] OK.
>
>
> 8) section 3.2.3:
> "The question of whether the receiver of pure ACKs is required to feed 
> back any CE marks on them is a matter for the relevant feedback 
> specification ([RFC3168] or [I-D.ietf-tcpm-accurate-ecn]).  It is 
> outside the scope of the present specification."
> However, as mention about I believe that RFC3168 does not say anything 
> about feedback how to handle CE marks of pure ACK because it assumes 
> that this case never occurs. So that might be implementation 
> dependent, so it might be better to spell that out:
> "[I-D.ietf-tcpm-accurate-ecn] includes handling of control packets. 
> However, [RFC3168] does not specify handling of pure ACKs that are CE 
> mark, so it might be implementation specific if feedback is provided 
> or not."
[BB] OK.

Anyway, this will be moot if we do not allow ECT on Pure ACKs unless 
AccECN has been negotiated.
> I pretty sure that this is what the AccECN draft is doing but I can 
> double check. If not it clearly should be added. Maybe it's not 
> spelled out clearly enough. I'll check!

[BB] You're right. I will fix this.

I too thought that AccECN counted CE on pure ACKs, but I checked the 
AccECN draft by searching for "pure ACK" and couldn't find any mention 
of this aspect. However, I've just done a more thorough search, and 
you're right - it says so three times, but none use the words "pure ACK":

    The fourth counts
    the number of packets arriving marked with a CE codepoint (including
    control packets without payload if they are CE-marked).

[...]

    The CE
    packet counter includes control packets that do not have payload
    data,

[...]

    The CE packet counter (r.cep), counts the number of packets
    the host receives with the CE code point in the IP ECN field,
    including CE marks on control packets without data.

>    the RST message should be ECT marked:"
[BB]
* I had already edited out any occurrence of the phrase "ECT marked" 
(because "marked" generally implies a congestion marking, so this could 
be misinterpreted).
* I always avoid the word "should" in lower case.
* I try to avoid writing sentences in the passive (but I missed this one).

Other than all that, I grok what you mean; I will use:

"The following factors have been considered before deciding whether
    the sender ought to set ECT on a RST message:"
> That's it. So nothing big. Let move on with this draft!

[BB]
Cheers

Bob
>> after
>> address
>> repeating
>>> more one
>>> xml for
>>>> the
>>>> present
>>>> allows
>>>>> 2) On network node treatment of ECT, RFC 3168 is brief, e.g. 
>>>>> (Section 5,
>>>>> top of p.7):
>>>>>
>>>>> sentence by
>>>>> access

---

From: Mirja Kühlewind <[email-masked]>
Date: Mon, 24 Apr 2017 22:56:59 +0200

Sorry, yes to be clear: this feedback was all meant as individual. It’s the chairs decision to run the adoption call. It’s just my personal opinion that we could do this right now with the current version (or we can also wait for your next version if you anyway working on it right now).

Mirja

> Am 24.04.2017 um 21:41 schrieb Bob Briscoe <[email-masked]>:
> 
> Mirja,
> 
> I shall get on with making the edits outlined below and, unless I see objections to anything in this email, I'll submit an update when done.
> 
> 
>> given it seems I spend most of my time these days to worry about different ECN-related RFCs, I've also reviewed this draft (actually only sections 1-4; I may review section 5 at a later point of time but I trust you that you have addressed my previous comments here). 
>> 
>> Here is my feedback as individual contributor: In general, I'm happy with the spec and the overall draft now. Thanks for the work! It reads very well now. I have a few minor technical comments and some editorial proposals and a proposal to restructure some smaller parts below.       
>> 
>> So te document is in really good shape and the technical specification is good. So I think that this version is definitely ready for adoption and would like to see an adoption call very soon! 
> [BB] Strongly agree :)
> 
> Seriously, I think you should make it clear whether you are saying this as AD or individual.
> 
>> 
>> Mirja 
>> 
>> ----------------- 
>> TFO allows the initiator to send data in the SYN but no additional data packets, and the responder to send the SYN/ACK as well as an IW full of data packets. So I think the problem here goes away. However, accECN actually does feedback information of CE marked data in the SYN/ACK because the SYN/ACK carries the AccECN option that holds this information. 
> 
> [BB] I'll remove the whole section about TFO, and instead include a brief sentence explaining why TFO is not relevant in the intro to S.4. Thanks.
> 
> I was thinking that there is nothing in TFO [RFC7413] to preclude the initiator sending data segments before receiving the SYN-ACK.
> However, you've made me think this through further, and I've realized the initiator would have to send data without the ACK flag set (or with the ACK flag faked but no valid ackno). Altho the TFO cookie would make this safe from a security perspective, it's not going to happen due to middleboxes.
> 
> 
>> I've checked REF3168 and it does not say anything about handling of pure ACKs that are CE marked because it probably just assumes that that case will never happen. However, implementations may or may not provide feedback. I think I would support to only have pure ACKs ECT marked if AccECN is supported because of this. I also think that losing ACKs is probably less bad that losing any of the other (control) packets that are discussed in this document. So I think this restriction would be okay. 
> FYI, RFC3168 does vaguely hint that the receiver of Pure ACKs carrying ECT (or CE) probably ought not to reject them (but it doesn't say what to do with a CE mark). In S.6.1.4 it says:
> Saying "current generation" implies a future generation might change this, which ought to make an implementer wary of rejecting ECT on Pure ACKs.
> 
>> 
>> 3) This is a suggestion to improve the situation, however it probably needs further discussion: in sect 3.2.1.4 you say that one should still use ECT SYNs even if a connection attempt previously failed. Maybe we can use a smaller time-out in this case...? 
> [BB] I'm wary of jumping to the conclusion that a loss is not due to congestion just because it's becoming more likely it could be due to a black hole. E.g. if there's a black hole, the probability of losing an ECT SYN is 100%, but if the underlying loss level is 1%, the probability that 2 SYNs are lost is 0.01% as an alternative. 
> 
> So, when 2 ECT SYNs at different times have timed out, but non-ECT SYNs got through, I suppose it's possible to argue that the probability that the second loss was due to congestion has reduced. So the timeout could be reduced.
> 
>> 1) I would move the network behavior in any case AFTER the endpoint behavior spec. Actually I would even propose to have it in a different section (not under 3. Specification) because it actually does not specify anything; it only comments on the current situation. 
> [BB] I did consider this when I removed the normative text from this section. But this this section is intended to do more than just comment. We want it to incorporate the "SHOULD" in ecn-experimentation as part of the specification for this experiment. And we figured this text is short enough to put first, to reduce the chance it gets buried and overlooked operators, without distracting host implementers too much.
> 
> Rationale: When updates to a system for an experiment are all spread about in different docs, it's not just helpful but also necessary to call out all the parts in their different docs. Similarly, we have incorporated RFC5562 (ECT on SYN-ACK) and AccECN into this draft - both as normative references.
> 
>> 
>> 2) And then you would actually not need a separate subsection for the Endpoint behavior; you could just call section 3 "Specification of Endpoint Behavior" or even "Specification of Sender Behavior" and move the first two paragraphs of section 3.2 to the intro that explains that this mechanism/document only needs to talk about the sender behavior. 
>> 3) I would include the text on TFO and IW10 directly into section 3.2.1.3 instead of just giving a reference to a later section. 
> [BB] This is a matter of style. I prefer to distinguish normative dependencies between 
> The above structure forces implementers (and authors) to make dependencies modular, in case an experiment fails. E.g. we do not yet know whether an attack will be invented against TFO. 
> 
> For instance, look at all the mess that has had to be cleared up because everyone assumed the ECN nonce experiment would succeed.
> 
>> 
>> 4) The rationale in section 3.2.6.1 could also go somewhere into section 5 (to keep the spec part as short as possible). At least I think you don't need the whole text here. If you want to keep something you probably only need the second paragraph (and not an own subsection). 
> [BB] The scope of Section 5 is (currently) countering arguments in other RFCs (mostly RFC3168).
> ECT on RSTs wasn't mentioned in an RFC so far, which is I why I put this section here.
> 
> One solution would be to rename section 5 "Rationale". Then I would be happy to move 3.2.6.1 to section 5. I'll do that.
> 
>> 
>> 5) Maybe have the section on Retransmissions (3.2.7.) earlier before RST, FIN, and Window probe because it seems more "important". But actually I don't think the order matters that much. So this is a minor comment only. 
> [BB] Currently the order is chosen to match the order of the lists in the introduction so they can be summarized as "control packets and retransmissions". I'll leave it as it is.
>> 
>> 6) The paragraph on SCTP (section 4.1) could go into the intro. And if you then also integrate the TFP and IW10 stuff into section 3.2.1.3, section 4 would actually go away completely. 
> [BB] Now, with the removal of the TFO section, none of S.4 contains anything normative. Now it is just saying "For all these variants, here's why you don't need to do anything different". So I am inclined to move it /after/ section 5.
> 
> I certainly don't want to integrate the TFO and IW10 comments into the spec section, because that would just complicate the spec with text that essentially ends up saying "You didn't need to read this to implement the spec".
> 
> I also don't want to add a detailed para about SCTP to a nice 3-para Intro to a spec about a modification to TCP.
> However, I agree that "scope" is one of the important things to cover in an introduction.
> 
> In summary, I would rather move this section about variants to after S.5. And in the intro, say this is an experimental modification to standards track TCP, but also flag that it discusses (non-)interaction with variants such as SCTP, TFO, IW10. 
> 
>> 
>> 1) I though that it might be good to mention AccECN already in the Intro because that the enabler for the SYN part... 
> [BB] OK, that would be sensible, along with the above sentence about variants. Will do.
>> 
>> 2) section 1.1: 
>> OLD 
>> "Preventing TCP control 
> [BB] Good point. Given this sentence was in a para about L4S, I shall split it off and make it into a summary section.
>> 
>> 
>> 3) section 3.2.1.1: 
>> OLD 
>> "Accurate ECN (AccECN) 
> [BB] Eh? I'm not going to change this, cos the word flag would imply this is a bit with a meaning orthogonal to the other bits.
> 
> The first bit only distinguishes "whether or not the SYN arrived marked CE" when the 3-bit codepoint as a whole is 'X10'. So it's not a flag, it's 2 codepoints.
> 
>> 
>> 4) section 3.2.1.2 
>>    separately from non-support of AccECN. Meaning, if AccECN is noted as not supported by the server, the initiator may decide to not mark the SAN as ECT but it still may try to negotiate classic ECN or even AccECN (to quickly detect server upgrades)." 
> I now realize my heading of 3.2.1.2 is wrong; it was not meant to be about "Caching Failed Connection Attempts". It was meant to be mostly about "Caching Lack of Recognition of ECT SYNs", with just a reference forward to caching failed connection attempts ("3.2.1.4 Fall-Back Following No Response to an ECT SYN"). 
> 
> I intended to say that, when a server is cached as not supporting AccECN, subsequent attempts should still request AccECN, but not with ECT on the SYN. Because there is no harm in always attempting to negotiate AccECN (as long as such requests are not black-holed), because the responder will still respond properly to an AccECN request if it only supports classic ECN or no ECN at all, without any performance penalty. 
> 
> I shall also make the point in S.3.2.1.4 that if an ECT SYN is black-holed, it might still be possible to request AccECN, but without ECT on the SYN.
> 
>> 
>> 5) section 3.2.1.4: 
>> OLD 
>> I wouldn't make any statements about things that are not in scope of this doc, so I would remove the later part (including fallback for AccECN): 
>> NEW 
> [BB] I agree that I shouldn't have said to not attempt AccECN. So I will use your new text.
> 
> But I disagree about not even mentioning that the cause might be other experimental options, and to encourage the reader to read those specs as well. I'll use the following unless you shout:
> 
>    ordinate all the strategies for fall-back in order to isolate the specific 
>    cause of the problem."
> 
>> 
>> 6) section 3.2.2 says: 
>> It would probably be good to also state in this draft what the procedure is that is described in RFC5562: 
> 1) I think it's dangerous to quote a small part of another spec. as if it's a good summary of the behaviour. This loses all the detail in RFC 5562 (e.g. what if the responder implements SYN cookies, yada yada).
> 
> 2) Oh dear. I had forgotten that RFC5562 specified a different protocol to the "ECN+" paper. It's ultra-ultra-conservative.
> 
> So, I think we had better include the original proposal in the "ECN+" paper within the generalized ECN draft, and include a counter-argument to RFC5562 as well as to RFC3168.
> 
>> 
>> 7) section 3.2.3: 
>> OLD 
>> "It MAY also 
>>    implement Acknowledgement Congestion Control (AckCC) [RFC5690] to regulate the pure ACK rate, but this is 
>>    not required.  Note that, in comparison, TCP Congestion Control [RFC5681] does not require..." 
> [BB] OK.
>> 
>> 
>> 8) section 3.2.3: 
>> "The question of whether the receiver of pure ACKs is required to feed back any CE marks on them is a matter for the relevant feedback specification ([RFC3168] or [I-D.ietf-tcpm-accurate-ecn]).  It is outside the scope of the present specification." 
>> However, as mention about I believe that RFC3168 does not say anything about feedback how to handle CE marks of pure ACK because it assumes that this case never occurs. So that might be implementation dependent, so it might be better to spell that out: 
>> "[I-D.ietf-tcpm-accurate-ecn] includes handling of control packets. However, [RFC3168] does not specify handling of pure ACKs that are CE mark, so it might be implementation specific if       feedback is provided or not." 
> [BB] OK.
> 
> Anyway, this will be moot if we do not allow ECT on Pure ACKs unless AccECN has been negotiated.
>> 
>> I pretty sure that this is what the AccECN draft is doing but I can double check. If not it clearly should be added. Maybe it's not spelled out clearly enough. I'll check! 
> 
> I too thought that AccECN counted CE on pure ACKs, but I checked the AccECN draft by searching for "pure ACK" and couldn't find any mention of this aspect. However, I've just done a more thorough search, and you're right - it says so three times, but none use the words "pure ACK":
>    The fourth counts
> * I had already edited out any occurrence of the phrase "ECT marked" (because "marked" generally implies a congestion marking, so this could be misinterpreted).

---

From: Bob Briscoe <[email-masked]>
Date: Wed, 26 Apr 2017 00:22:51 +0100

Mirja, list,

Thanks again for your re-review.
I've posted another non-trivial rev - now 
https://tools.ietf.org/html/draft-bagnulo-tcpm-generalized-ecn-03

The spec section is now succinct, with all rationale in a following section.

I did all the changes I promised in my responses to your review below. 
One addition: cos I had added stuff about ECT on SYN-ACK, I also added 
stuff about the interaction between TFO and ECT on SYN-ACK.

Still none of the TCP variants like TFO, IW10 require any normative text 
to be any different from ECT on all regular TCP packets. So I have 
relegated the variants section to the end, and not incorporated it into 
the main spec at all - didn't want unnecessary distractions in the 
normative spec text.

Main changes this rev:
* Added para to intro highlighting the main TCP variants that are 
discussed wrt (non)interaction (SCTP, IW10, TFO, AccECN).
* Mostly new text in "3.2.1.2 Caching Lack of Support for ECT on SYNs", 
and some changes to 3.2.1.4 SYN fall-back
* Completely new S.3.2.2  on SYN-ACK and new rationale S.4.3 for SYN-ACK 
(no longer uses RFC5622)
* Renamed section heading "Discussion of the Arguments in the RFC 
Series" to "Rationale"
* New rationale subsections for ECT on FINs and RSTs (the latter was 
previously elsewhere)
* Moved TCP Variants section to the end and added interaction between 
ECT on SYN-ACK and IW10, TFO

Bob