[tcpm] Detailed review of AccECN rev -04

[Gorry Fairhurst <[e-mail]>]

[e-mail]

I've read the -04 rev of teh above ID, and I have the following comments 
for the TCPM WG.

Overall,

* This document reads well, and seems mature.

* There are several references to other IETF activities, and the text 
concerning these appears to need to be revised before this is ready to 
proceed. See below.

* Since this document was adopted, work on ECN++ appears to have become 
inter-twined with this ID. I did not find this helpful, and made 
comments on ECN++ today that voiced this concern and requested we 
consider as a WG to best frame the proposed modification on the TCP SYN 
permitting ECT(?) to be set. See comment (6).

* I wonder if this document updates any other RFCs - I'd assume not -  
but it may if a future work proceeds along the standards track. I think 
it would be good to specifically identify in one section of the intro 
what these implications are.
For instance:
- The discussion of ECN on SYN is predecated by ECN-Experimentation 
(targeting publication as PS)
- The discussion of ECN on SYN has implications on ECN++, which 
recommends AccECN is implemented.
- A future standards-track document based on the AccECN experimental RFC 
could update RFC3168.
- etc.

* The document may intend a proposed change to RFC 5681 (4.2.  
Generating Acknowledgments). See comment (8).

Gorry

---

Some more specific comments on the text in the -04 AccECN draft:

(1) The introduction - "until the AccECN experiment suceeds, [RFC3168] 
will remain as the standards track specification for adding ECN to TCP."
- This seems odd. I think this is not the case:
Quite simply: "RFC3168 defines the current TCP feedback method."
- It may be you think a  future standards-track document based on the 
AccECN experimental RFC could update RFC3168, that is possible, but 
there is no need for an EXP to make that direct claim against as PS.

(2) Section 1.5. First para. (On loss v drop using ECN)
- I'd feel happier also noting that a router implementing AQM is still 
allowed to drop packets marked as ECT() when this is needed - and you 
could refer to the BCP to say so: /RFC/7567.

(3) Section 1.5. Last para.
- Please remove the discussion of Nonce from here!
- It is OK simply to refer to the PS specifying ECN Experimentation - or 
the process document to do this.
- If it is really necessary to discuss this, please use the form of 
words expressed in that draft, rather than saying "was never deployed".

(4) Section 3.1.1
"is being reclassified as historic."
- Again this text is now out-of-date, see comment (3).

(5) Section 3.1.1, ECN Nonce.
- This definition also contains text later that say "does not need...". 
It's not that it doesn't need, more that the IETF has changed the use of 
this field. I think it would be easy to simply remove this definition - 
but the WG wants to keep it please align the text precisely with the ECN 
Experimentation draft. The last para of section 4 repeats this and has 
the smae problem.

(6) Section 3.1.2. ECN on SYN
- This section seems to be a variant of the section expressed in the 
ECN++ draft (sect 4.2). That draft rather awkwardly refers back to thsi 
draft recommending AccECN is implemented - but the two specs differ.

I do encourage the WG to think about where the update on ECN for SYN is 
best described. There are specific issues for experimentation and for 
cahcing in ECN++ and other issues here. I'd like a cleaner spec so that 
**IF** there are results from the experiments we know how to then 
publish a PS and declare the experiment as historic. At the moment this 
seems awkard.

- Section 3.2.7.2 is also about the SYN negotiation. And similar to ECN++.

(7) Section 3.2.3 (First segment after SYN)
- TCP segments can be re-ordered, so saying the "first segment after 
SYN" should (I think) probably be the segment numbered after the SYN 
segment - noting that this is not necessarily the next segment received 
(in time order).

(8) Section 3.2.5
- The modified delayed ACK policy stated here is an experimental update 
to the IETF recommended procedure in a PS (RFC5681, 4.2.  Generating 
Acknowledgments). I think it is important that the change is referenced 
back to this RFC. The proposed change could have implications on the 
capacity used for the ACK stream - and this should be noted.

(9) RFC2119:  Section 3.2.7.1, para 1
- The ID says the client MUST NOT include the AccECN option on the SYN. 
What does a server do that receives such an option?

(10) RFC2119:   Section 3.2.7.1, para 2
- "A TCP server that confirms its support ... SHOULD also"
- I couldn't parse that sentence... Is that to imply there are valid 
reasons why the server could be implemented in a way that does not do 
this? (That doesn't seem to be explained).
- What is the meaning of "also" here. Does that mean the support is 
already confirmed in some other way? (I'm not sure also after SHOULD is 
a helpful construction of a clause).

(11) RFC2119: Section 3.2.7.1, para 3
- I think the sentence starting "A TCP Client" also contains two SHOULD 
clauses without saying what are the conditions for not doing this.  I 
think it is often wiser to start these sort of clauses by saying 
something like this order to avoid this:

... A host that receives an AccECN option SHOULD respond by returning 
the AccECN option in the first ACK at the end of the 3WHS. ... There are 
three cases when the host could decide not to ...

When the TCP client returns the ECN option, it SHOULD also include an 
AccECN option on the first data segment it sends... This is to ensure 
that...

(12) RFC2119: Please also avoid "MAY NOT" as a construct, since this can 
be mis-interpretted by different varients of English usage.

(13) Section 3.3 appears to make string normative requirements (MUST) on 
the operation of middleboxes. This may be the intention, but if so, I 
think it would be much wider to then make this evident in the section 
title and the document introduction.

(14) RFC2119: Section 3.3 appears to make mixed normative requirements 
(MUST attempt) on offload. I don't know what that means. I think this 
could be equivalent to "SHOULD" with an explanation of WHY this is so 
needed (which is sort of there).

(15) Section 3.3 on offload.
- If this is intended to change offload behaviour, I think it needs a 
separate section to say this, mentioned in the intro, otherewise people 
implementing offload will not see and read this.

(16) Section 4, Section 5, Appendix A, intro.
- You could remove (not normative) to be even clearer.

(17) For the avoidance of doubt it may be wise to state the standards 
status of the TCP AO and Conex work... e.g., "TCP AO is a 
standards-track method to ..."

Best wishes,

Gorry