Re: [tcpm] Reviewed for draft-ietf-tcpm-accurate-ecn-23

On 26/03/2023 03:06, Bob Briscoe wrote:
> Gorry,
>
> Thank you as always for your extensive review. See responses tagged 
> [BB] inline.
> No comment = proposal accepted.

I see you have a plan, I made a few additional comments [GF2].

>
> On 25/03/2023 17:28, Gorry Fairhurst wrote:
>> I have just reviewed draft-ietf-tcpm-accurate-ecn-23 for a WGLC.
>>
>> This ID is significantly easier to read than an earlier version I 
>> read, but still is not an easy read - and I did not have time to 
>> re-check all the logic, but I did look at the text and have the 
>> following set of comments. Most of these relate to sentences were 
>> clarity could be improved, or RFC2119 language could be more explicit 
>> about the subject. Many of these could be resolved by simple text 
>> changes, and where possible I have provided the text from the ID, and 
>> in most cases a suggestion remedy.
>>
>> Best wishes,
>>
>> Gorry
>>
>> P.S. When you complete all the updates after the WGLC, I suggest you 
>> post a review request to TSVWG, just to keep the process good - 
>> because this does plan to update a TSVWG spec.
>>
>> =========
>>
>> This statement causes concern:
>> AccECN is intended to be a complete replacement for classic TCP/ECN 
>> feedback, not a fork in the design of TCP.
>> Is it possible the intent is:
>> AccECN is intended to offer a complete replacement for classic 
>> TCP/ECN feedback, not a fork in the design of TCP.
>> - To me the importance being that this does not immediately obsolete 
>> current TCP, it simply provides a negotiated mechanism to replace it.
>>
>> ===================
>> Section 1
>> ===
>> Recently, proposed mechanisms like Congestion Exposure (ConEx 
>> [RFC7713]), DCTCP [RFC8257] or L4S [RFC9330] need to know when more 
>> than one marking is received in one RTT which is information that 
>> cannot be provided by the feedback scheme as specified in [RFC3168].
>> - Add comma, so that:
>> Recently, proposed mechanisms like Congestion Exposure (ConEx 
>> [RFC7713]), DCTCP [RFC8257] or L4S [RFC9330] need to know when more 
>> than one marking is received in one RTT, which is information that 
>> cannot be provided by the feedback scheme as specified in [RFC3168].
>> ===
>> This document specifies an update to the ECN feedback scheme of RFC 
>> 3168 that provides more accurate information and could be used by 
>> these and potentially other future TCP extensions.
>> - could we note this is an alternative rather than replacement:
>> This document specifies an update to the ECN feedback scheme of RFC 
>> 3168 that provides an alternative more accurate information and could 
>> be used by these and potentially other future TCP extensions.
>> ===
>> This documents specifies a standards track scheme
>> - remove s
>> This document specifies a standards track scheme
>> ===
>> Then terminology is defined
>> - do we need a comma:
>> Then, terminology is defined
>> ===
>> Section 1.2
>> ===
>> Therefore an AccECN receiver aims to act as a generic (dumb) 
>> reflector of congestion information so that in future new sender 
>> behaviours can be deployed unilaterally.
>> - why /aims to/ ... why does this not specify this?
>> - is /dumb/ necessary, it's an unfortunate word choice - simple could 
>> be alternative?
>
> [BB] Yes, 'aims to' should be moved to the later part of the sentence.
> The terminology 'dumb' was borrowed from the "intelligent vs dumb 
> network" phrasing of the e2e argument. 'Simple' would lose the 
> intended nuance of 'mechanistic' or 'robotic', and the former is least 
> overloaded with other meanings these days (robotic is starting to mean 
> the opposite!). So how about:
>
>     Therefore an AccECN receiver acts as a generic (mechanistic) 
> reflector of congestion information with the aim that in future new 
> sender behaviours can be deployed unilaterally.
>
> There are other occurrences of 'dumb reflector' - so I'll change them all.
>
>
[GF2] Seems OK to me.
>> ===
>> Classic ECN: the ECN protocol specified in [RFC3168].
>> - Upper case /The/ to be consistent?
>> ===
>> Classic ECN feedback: the
>> - Upper case /The/ to be consistent?
>> ===
>> which ensures the signal is received reliably even if ACKs are lost.
>> - well not if all are lost! Could this be:
>> which provides resilience of the signal to loss or reordering of ACKs.
>> ====
>> Para after table talks of a TCP sender, but the terminology talks of 
>> a data sender and a TCP server; but not a TCP sender. Presumably this 
>> is a data sender.
>> ====
>> - It might be useful to say that a TCP connection between a TCP 
>> client and a TCP server provides bidirectional communication between 
>> two pairs of TCP senders and TCP receivers, forming two independent 
>> half connections/.
>
> [BB] Yes, we use half connection a number of times later, and this 
> would be a good place to introduce it.
>
>> ====
>> I think the whole of section 2 could be informative? ~If it is, can 
>> we say this?
>
> [BB] We do already; in the first para of §2:
> "This section provides an informative overview of the AccECN protocol 
> that will be normatively specified in Section 3 
> <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-23#accecn_Spec>"
>
[GF2] Oh, fine.
>> ====
>> The RFC 2119 conformance terms are defined in 1.3, but then are 
>> followed by an informative set of sections. I for one, would much 
>> prefer that definition block moved to the start of section 3.
>
> [BB] Much of the other terminology defined in §1.3 is needed sooner. 
> So, to do as you ask, we'd have to to split up the terminology 
> section, which I don't want to do. It's not incorrect as it is, so 
> let's leave it.
>
[GF2] I still think splitting the terminology would be better to include 
the RFC declarations at the start of the section that uses them.
>> ===
>> but it is more likely to suffer from middlebox interference.
>> - I think we could be more future-looking, and propose:
>> but is currently more likely to suffer from middlebox interference.
>> ===
>> However cycling is still a possibility at the Data Sender because a 
>> whole sequence of ACKs carrying intervening values of the field might 
>> all be lost or delayed in transit.
>> - This is about reception of ACKs? I wasn't sure first read, could we 
>> write:
>> However ACKs received at the Data Sender could still cycle because a 
>> whole sequence of ACKs carrying intervening values of the field might 
>> all be lost or delayed in transit.
>> ===
>> had been an infeasibly long sequence of ACK losses.
>> - what does infeasible mean here, it seems very imprecise. It would 
>> have been easier not to judge the feasibility and simply said /a 
>> long/...
>
> [BB] 'long' is less precise than 'infeasibly long', which conveys the 
> meaning of 'not just long, but so long that we do not need to consider 
> the possibility of it occurring in practice'.
> The main meaning of Infeasible is impracticable, so I would like to 
> leave this as-is.
>
[GF2] Not a huge issue to me.
>> ===
>> Therefore, as long as the AccECN Option is available
>> - unfortunate use of /as long/ just after discussing length of ACK 
>> drops?
>> Providing that the AccECN Option is available
>> ===
>> is provided in order to protect
>> - ordering is not implied, this could be:
>> is provided to protect
>
> [BB] The intended meaning was not "provided in order, to protect." 
> Rather it was "provided in_order_to protect." So your suggestion does 
> indeed remove the ambiguity.
>
>> ===
>> The TCP server MUST NOT set one of these 4 combination of flags on 
>> the SYN/ACK unless the preceding SYN requested support for AccECN as 
>> above.
>> - What is the client response if this does happen?
>
> [BB] To rephrase your question into a stand-alone question, "What does 
> a non-AccECN client do if it somehow receives a SYN-ACK that implies 
> the server supports AccECN?
>
> This draft doesn't specify what non-AccECN clients do, and it would be 
> pointless to try, because they are already out there, and their specs 
> already say what they do.
> Nonetheless, FYI, clients implementing either of the two non-AccECN 
> specs currently out there would both handle a randomly appearing 
> AccECN SYN-ACK correctly. Specifically:
>
> 1) RFC3168, §6.1.1:
>          If a host has received at least one [...]
>          non-ECN-setup SYN-ACK packet, then it SHOULD NOT set ECT on
>          data packets.
>          ...
>          Receivers MUST correctly handle all forms of the non-ECN-setup
>          [...] SYN-ACK packets.
>
> Even if an RFC3168 client only checked the last 2 flags of the 3 
> AccECN flags, none of the three 2-bit combinations that imply AccECN 
> support would be expected by an RFC3168 client (by design of AccECN).
>
> Therefore:
>
>   * An RFC3168 client would handle a randomly appearing AccECN SYN-ACK
>     correctly,
>       o whether it checked all the TCP flags (as required by the
>         second sentence quoted from RFC3168 above)
>       o or even if it only checked CWR and ECE.
>
>
> 2) RFC9293:
> Altho this recommends TCP implementations support RFC3168, there are 
> many RFC793 TCP implementations that do not. Such a non-ECN TCP client 
> will have no logic to check the ECN flags during the 3WHS and 
> therefore it will not notice that a SYN-ACK is (incorrectly) 
> responding as if it was asked to support AccECN.
>
> Therefore, again,
>
>   * a non-ECN TCP client will handle a randomly appearing AccECN
>     SYN-ACK correctly, just continuing to not support any form of ECN.
>
>
[GF2] Thanks for this explantion, I think setting these bits defines the 
mode - so it's a little weird as a requirement, but I get it.
>
>> ==
>> interference than other intermittent causes of loss, e.g. congestion, 
>> wireless interference, etc.¶
>> - two uses of the word /interference/ with different meaning, could 
>> the second be replaced by /loss/?
>
> [BB] Good point. I think "transmission loss" would be better.
>
>> ==
>> It MUST NOT send an ECN-setup SYN [RFC3168] within the same 
>> connection as it has sent a SYN requesting AccECN feedback.
>> /as/when/ or something else?
>> It MUST NOT send an ECN-setup SYN [RFC3168] within the same 
>> connection when it has sent a SYN requesting AccECN feedback.
>
> [BB] I don't see the problem here. "The same ... as" is correct 
> English, whereas "the same ... when" is not.
> A similar sentence proves this is a correct construction: "It MUST NOT 
> send a message within the same millisecond as it has sent another message"
>
> If this makes the reader stumble, we could rephrase entirely:
>     It MUST NOT send an ECN-setup SYN [RFC3168] during a connection in 
> which it has also sent a SYN requesting AccECN feedback.
> However, I don't think that's any better, so I'd prefer to leave it as-is.
>
[GF2] That proposal seems much clearer to me, I understand now.
>> ===
>> It MUST NOT send an ECN-setup SYN/ACK [RFC3168] within the same 
>> connection as it has sent a SYN/ACK agreeing to use AccECN feedback.
>> /as/when/ or something else?
>> It MUST NOT send an ECN-setup SYN/ACK [RFC3168] within the same 
>> connection when it has sent a SYN/ACK agreeing to use AccECN feedback.
>
> [BB] Ditto.
>
[G2] I'd prefer to find a way to make this one more clear.
>> ===
>> It is still obliged to respond appropriately to AccECN feedback that 
>> indicates there were ECN marks on packets it had previously sent
>> -Please explicitly say what is "it"?
>> The Data Sender is still obliged to respond appropriately to AccECN 
>> feedback that indicates there were ECN marks on packets it had 
>> previously sent
>> ==
>> In general, it is obliged to respond to congestion feedback even when
>> - what is it? Receiver/Sender?
>> In general, the Data Sender is obliged to respond to congestion 
>> feedback even when
>> ===
>> Unlike an RFC 3168 data sender, it MUST NOT set CWR to indicate it 
>> has received and responded
>> - what is it? Receiver/Sender?
>> Unlike an RFC 3168 data sender, a Data Receiver MUST NOT set CWR to 
>> indicate it has received and responded
>
> [BB] The problem you've identified occurs because there is an aside 
> about all the 'switching feedback negotiation' bullets, just before 
> all the 'congestion response' bullets.
> This tends to lose the thread of the Data-Sender context.
>
> Nonetheless, I've also noticed that:
> * some of the bullets under "As a Data Sender" are about sending 
> control packets, not data.
> * some of the bullets under "As a Data Receiver" are about receiving 
> control packets, not data.
>
> So I shall have to restructure the whole section.
> Let me do that, then give it to you to review again.
[GF2] OK
>
> ==
>> if it receives an ECN-setup SYN or ECN-setup SYN/ACK [RFC3168] during 
>> the same
>> - to avoid doubt in citing this RFC2119 requirement, can we write:
>> if a Data Receiver receives an ECN-setup SYN or ECN-setup SYN/ACK 
>> [RFC3168] during the same
>> ===
>> If for any reason it is not willing to
>> - to avoid doubt in citing this RFC2119 requirement, can we write:
>> If for any reason the Data Receiver is not willing to
>> ===
>> it MUST NOT use reception
>>
>> - to avoid doubt in citing this RFC2119 requirement, can we write:
>>
>> A Data Receiver MUST NOT use reception
>
> [BB] As above, I'll restructure the whole of this section before 
> giving to you to review again.
> Sorry about this.
>
>
>> ===
>> correct ECN feedback logic, as the packets
>> - could we replace /as/?
>> correct ECN feedback logic, because packets
>> ===
>> Whenever a host feeds back the value
>> - Is that the Data Receiver? Sender? or both? - it would better to 
>> match the clauses above.
>
> [BB] Yes, this could s/host/Data Receiver/
> But I will also make it clear that the stuff about the feedback being 
> on a retransmission is about a retransmission for the other half 
> connection.
>
[GF2] I see.
>> ===
>> When a host first enters AccECN mode, in its role as a Data Receiver
>> - could this be more simply said:
>> When Data Receiver first enters AccECN mode,
>
> [BB] Not really, because a host enters AccECN mode as a whole (not 
> separately per half connection).
[GF2] That was not clear, ... I think this applies to several places, so 
maybe a clear statement that explains this in th eirght place would be best?
>
>> ==
>> When a host enters AccECN mode, in its role as a Data Sender
>> - could this be more simply:
>> When Data Sender first enters AccECN mode,
>
> [BB] Ditto.
>
>> ==
>> A host MUST NOT interpret the 3
>> - Is that the Data Receiver? Sender? or both? Client? Server?
>> A Data Receiver MUST NOT interpret the 3
>
> [BB] This does really mean "a host".
> It applies across the 3WHS and data transfer, so it's not just about 
> Data receiver or Data sender. And it applies to client and server. So 
> it is a general statement about what hosts interpret.
>
>> ==
>> An implementer MAY develop an alternative algorithm as long as it 
>> satisfies these requirements.¶
>> - doesn't seem like a /MAY/ can this be /may/ or /is permitted to/?
>
> [BB] I've checked the definition of 'MAY' in RFC2119, and I'm pretty 
> sure we really do mean 'MAY', but I'm always willing to be corrected.
> Can you try to explain why your understanding of 'MAY' would be 
> inappropriate here?
>
> I'm guessing that perhaps "an implementation MAY do x", but "an 
> implementer may do y"?
> If that's your concern, I would say in this case, "an implementer may 
> produce code that MAY do y," so I don't see a great difference.
>
[GF2] I do not mind on this one at all, check and see what works.
>> ==
>> the Data Sender SHOULD believe the ACE counter, unless it can be sure 
>> that it is counting all control packets correctly.¶
>> - what does believe mean? use?
>
>     '...SHOULD consider the ACE counter to be correct (and its count 
> of control packets to be incomplete)'
>
>> ===
>> Section 3.2.3. The AccECN Option
>> - Could we please add RFC-Ed instructions to ensure replacement of 
>> TBD1 and TBD2 during final edit?
>
> [BB] In my local editor's copy, we already have the early assignment 
> numbers now.
>
>> ===
>> All implementations of a Data Sender that read any AccECN Option MUST 
>> be able to read in AccECN Options of any of the above lengths.
>> - I think /in/ seems odd.
>> - This makes me wonder what happens if they do not?
>> - REQUIRED to implement?
>
> [BB] I can s/read in/read/
> For any spec (not only AccECN), if an implementation does not 
> implement a 'MUST', it is not an implementation of the spec. So all 
> bets would be off.
>
> All this is clarifying is that, altho' it is optional to implement the 
> AccECN Option, if you do implement it, you have to be able to read all 
> the valid option lengths.
>
>> ==
>> Whenever the Data Sender receives ACK carrying
>> - add /an/
>> Whenever the Data Sender receives an ACK carrying
>> ===
>> If the ACK has not been superseded,
>> - please explain superseded!
>
> [BB] I'll add '(see Appendix A.1)', which defines superseded precisely.
>
>> ===
>> AccECN is for congestion control, which SHOULD generally be 
>> considered important relative to other TCP options.
>> - what does important mean here?
>> - Does this have to be a /SHOULD/ could it be a should and explain?
>
> I'll reword as
>     "which implementations SHOULD generally prioritize over other TCP 
> options when there is insufficient space for all the options in use."
> Then I think 'SHOULD' becomes appropriate.
>
[GF2] - yes.
>> ===
>> it (the server) has to have confirmed
>> - why not just say the server?
>> the server has to have confirmed
>
> [BB] In a SYN cookie scenario, the server deliberately forgets what it 
> would normally remember. So I had to use 'it', because the server is 
> reverse engineering what 'it' did earlier, where 'it' is another 
> incarnation of itself that it doesn't remember. Otherwise, just saying 
> "it can assume that the server has to have confirmed..." makes it 
> sound like some other server.
>
> A little rephrasing is needed here anyway, so I've tried to say this 
> another way:
>
> ORIGINAL:
>
> If a TCP server using SYN Cookies supports AccECN and if ..., and if ...,
> it can assume that:
> *  the TCP client has to have requested AccECN support on the SYN
> *  it (the server) has to have confirmed that it supported AccECN
>
> PROPOSED:
>
> If a TCP server using SYN Cookies supports AccECN and if ..., and if ...,
> the server can infer the first two stages of the handshake:
> *  the TCP client has to have requested AccECN support on the SYN
> *  then, even though the server kept no state, it has to have confirmed
>    that it supported AccECN
>
>
>> ===
>> Each host needs to maintain eight additional counters. The hosts have 
>> to apply some additional tests to detect tampering by middleboxes,
>> - Is this sender or receiver code?
>
> [BB] Here, 'hosts' is appropriate because this is a general statement 
> about all the additional tests; some are during the 3WHS (when either 
> client or server is comparing what it received against what it sent); 
> while other tests are conducted by the Data Receiver part of a 
> half-connection.
>
>> ===
>> I would suggest section 3.3 is made a separate top level section 
>> (section 4), to really call-out that this is not the TCP sender and 
>> receiver but the part that other people need to read.
>
> [BB] That would have been a really good idea earlier. I'd like to do 
> this. But I'm going to think long and hard before doing it now though. 
> I'm concerned that there might be cross refs within this or other 
> documents to §3.3 or §4 -- §8 that might break.
>
> Whether we do this or not, an additional piece in the doc roadmap 
> about this section would be useful.
>
>> ---
>> With the importance of section 3.3 in mind, I was surprised to see 
>> the top-level section had no text. Surely some text belongs here to 
>> say that deployment of AccECN places new requirements on networking 
>> equipment that forwards TCP, and inspects the TCP header information!
>
> [BB] I believe that section headings are powerful ways to say exactly 
> this, without having to repeat the same words with a few gratuitous 
> little words added to make sentences. Specifically:
>
> 4. AccECN Compliance Requirements for TCP Proxies, Offload Engines and 
> other Middleboxes
>  4.1. Requirements for TCP Proxies
>  ...
>  4.2. Requirements for Transparent Middleboxes and TCP Normalizers
>  ...
>  4.3. Requirements for TCP ACK Filtering
>  ...
>  4.4. Requirements for TCP Segmentation Offload
>  ...
>
> Again, if this idea had come earlier, I might have been more inclined 
> to add a sentence or two. But I'm not a fan of adding significant 
> amounts of text at this stage. Will probably add a little tho.
[GF2] Little - even one sentence was what I had in mind. :-)
>
>> ===
>> End of 3.3.2:
>> However, to comply with
>> - I could live without the "However", which to me at least gives more 
>> weight - could even be a separate para to help this stand out!
>
> [BB] I can see the need to emphasize, but a new para wouldn't work - 
> the 'However' sentence is very much a continuation of the same flow of 
> thought.
>
[GF2] The problem with some of these /However/ is that they follow-on a 
sentence which then becomes normative. I'm happiest when the normative 
statements are single sentences that can be read on their own...
>> ===
>> I think 3.3.4 is different to 3.3.1 and 3.3.2, and really belongs 
>> with the rest or directly following 3.2. The reason is this, is about 
>> implementing TCP.
>
> [BB]
> This is a question of whether separate processors are independent even 
> if they run off the same power supply, or they're in the same housing. 
> Similar questions apply on where you draw the line between an API and 
> a protocol.
>
> In a similar vein, TCP Proxies /are/ TCP implementations. So they 
> ought not to be outside §3.
> So it seems we're now tending towards the idea that §3.3.1 and § 3.3.4 
> ought to remain in §3.3, while §3.2 & §3.3 would be moved to a new §4.
>
> But then we return to your original point that the types of company 
> that build all the functions of all four sections (§3.3.1-§3.3.4) tend 
> to be "networking equipment vendors" rather than end-system 
> developers. So we want to direct their eyes to their own section. But 
> then NFV blurs the distinction. This is not clear-cut. As I said, I 
> will think on it.
>
[GF2] OK, I think the text works best when grouped into topics that a 
specific group of readers would look at.
>> ===
>> Section 4
>> For the avoidance of doubt it would have been nice to have one 
>> starting sentence that explains if these statements in section 4 are 
>> *additional* requirements to section 3, which I think they are likely 
>> not, or a list of the impact of the changes in section 3 on RFC3168?
>
> [BB] Good point. You're right. They are not additional, which needs 
> saying.
>
>> ===
>> I don't have a strong opinion, but I think section 6 is much easier 
>> to read than some of the other materials, and seeing this after 
>> section 2 mighty have been good. Equally could it be moved to join 
>> other material before annexe B?
>
> [BB] It is intended to be like a 'Conclusions' section. The idea is 
> that it comes after the detailed spec, to sum up whether it meets the 
> original requirements. On that basis, it can't really come before the 
> detailed spec. I'd like to leave it where it is. But would it help to 
> change the title to 'Summary: Protocol Properties'?
[GF2] That would help, or just a couple of extra words.
>
>> ===
>> Section 10 looks like it should have an RFC-Ed comment to request 
>> removal before publication.
>> ===
>> Of course, other ways could be resorted to in order to extend AccECN 
>> or ECN in future
>> - could we be simpler and just say:
>> Other ways could have been used to extend ECN
>
> [BB] Certainly, we can remove 'Of course'.
>
> But you're suggesting we change the sense from 'could be in future' to 
> 'could have been in this spec'. That would be contrary to the purpose 
> (and headings) of this appendix. So we'd rather not do that part of 
> your suggestion.
>
[GF2] I'm less sure of the future, but do what people agree.
>> ===
>> In section 2:
>> could give conflicting feedback on the same segment
>> - which segment - the received segment or the segment that carries 
>> the feedback?
>
> [BB] We'll s/on/about/ which is what the conflict concern is.
>
>
>> ===
>> In section 2.4:
>> If the option is stripped,
>> - Maybe worth explaining in parenthesis
>> If a middlebox strips the option (removes the option from a TCP 
>> segment),
>> ===
>> In section 2.4:
>> Feedback in bytes is provided in order to protect
>> - seems one to confusion, can we remove /in order/ to avoid potential 
>> misunderstanding of ordering
>> Feedback in bytes provides protection against a receiver using 
>> attacks similar to 'ACK-Division' to artificially inflate the 
>> congestion window, which is why [RFC5681] now recommends that TCP 
>> counts acknowledged bytes not packets.
>> ===
>> Can the last para of section 2.4, be modified like this to allow it 
>> to appear in the guidance on middle boxes? :
>> Feedback in bytes protects from middlebox attacks similar to 
>> 'ACK-Division' that seek to artificially inflate the congestion 
>> window, which is why [RFC5681] now recommends that TCP counts 
>> acknowledged bytes not packets.
>
> [BB] A sentence directed at bad actors, boasting that we've designed 
> this so they cannot build middleboxes that mount this particular 
> attack wouldn't fit in the middlebox section.
> The section on middleboxes is guidance for /willing/ middlebox 
> implementers.
>
> The original sentence justifies a design choice of the AccECN 
> protocol. So it would be appropriate to just
>     s/against a receiver/against a receiver or middlebox/
>
>
>> ===
>> If it has set itself into classic ECN feedback mode it MUST then 
>> comply with [RFC3168].
>> - Can we replace the /it/s:
>> If a Data Receiver has set itself into classic ECN feedback mode it 
>> MUST then comply with [RFC3168].
>
> [BB] Yes, except s/Data Receiver/TCP client/, as per the preceding 
> sentence in the draft.
>
>> =================
>> Terms:
>>
>> /ECN bits/ is mentioned 3 times, but should always be /ECN Field/
>
> Agree on occurrences 1 & 2.
>
> Occurrence 3 is about the TCP flags - that needs
>     s/using the ECN bits in the TCP header/using three flags in the 
> TCP header already assigned to earlier ECN schemes/
>
>>
>> /ECN Field/ is not defined, should be as per [RFC3168] and define the 
>> ECN codepoint.
>
> [BB] OK. This just needs a change to the first sentence in §1.4 Recap 
> of Existing ECN Feedback in IP/TCP.
>
> OLD:
>     ECN [RFC3168 <https://www.rfc-editor.org/info/rfc3168>] uses two 
> bits in the IP header.
> PROPOSED:
> [RFC3168 <https://www.rfc-editor.org/info/rfc3168>] defines the 
> two-bit ECN Field in the IP header, consisting of four codepoints.
>
> Table 1. is titled "The ECN Field in the IP header" and recaps the 
> definitions of the ECN codepoints, and the para before it describes 
> the semantics.
>
>>
>> endpoint and end-point are variously used, please be consistent.
>>
>> /Data Sender/ is defined and then /ECN sender/ used in the text. Do 
>> we need to define both terms? or are they the same?
>
> [BB] For the one occurrence, we'll
>     s/an ECN sender/a sender that supports ECN/
> and
>     s/in the IP header/in the IP header of a data packet/
>
>>
>> /TCP client/ and /TCP server/ use lower case, whereas Data Receiver 
>> and Data Sender use upper case. the text later about a /TCP sender/ 
>> lower case. These ought to be consistent?
>
> [BB] OK. I'll upper case TCP client and server.
>
> Of the 5 occurrences of 'TCP Sender', the middle 3 are within quotes 
> from RFC3168 (where it is defined) and the 1st & 5th ought to be in 
> quotes and upper-cased, because they are describing what RFC3168 says.
>
> That's it.
> As I said, I'll just do everything with no comment.
>
> Thank you v much again.
>
> Cheers
>
>
>
> Bob
>
>
It looks like the next revision will address these issues,

Gorry

>>
>>
>> _______________________________________________
>> tcpm mailing list
>> tcpm@ietf.org
>> https://www.ietf.org/mailman/listinfo/tcpm
>
> -- 
> ________________________________________________________________
> Bob Briscoehttp://bobbriscoe.net/