Re: [tcpm] Review of draft-ietf-tcpm-accurate-ecn-09

[Mirja Kuehlewind <[e-mail]>]

Hi Michael,

Thanks for your feedback and an another review. Having considered it carefully I believe your comment below are actually all editorial, even inducing the first one (see below for further explanation). See proposed changes here:

https://github.com/bbriscoe/accecn/pull/1/files

Please see below for replies!

Thanks!
Mirja



> On 10. Sep 2019, at 12:51, Scharf, Michael <[e-mail]> wrote:
> 
> Bob, Mirja, Richard,
> 
> I have read draft-ietf-tcpm-accurate-ecn (well, again). Please find below my comments as individual contributor to TCPM. Some may have been discussed already; I have re-read the document from scratch. I apologize for any repetitions.
> 
> I have sent out a separate e-mail on whether the IANA considerations in this document are formally correct. That discussion belongs into a separate thread.
> 
> Major issues (not ordered by priority):
> 
> * Overall document
> 
>  I am confused about whether it is required to _send_ TCP options. As far as I recall past discussions, I thought that implementers asked to make _sending_ TCP options optional. Or do I miss something? I also think that there was less pushback on having a mandatory-to-implement guidance regarding _receiving_ TCP options. As somebody who has worked on sending sporadic TCP options a long time ago, I am concerned about the complexity of any MUST statement that mandates sending TCP options. The document seems inconsistent on that. For instance, Section 3.2.6 states: " It is RECOMMENDED to implement both sending and receiving of the AccECN Option." However, Section 3.2.8 mandates: "If an arriving packet increments a different byte counter to that incremented by the previous packet, the Data Receiver MUST immediately send an ACK with an AccECN Option, ..." There is other unclear wording e.g. in Sections 2.1, 2.3, and 5. Particularly confusing is also Section 7, which discusses omitting the option as a security vulnerability. What is needed is a clear statement on what is required regarding options at a prominent place (e.g. also in the overview in Section 2). Then the whole document has to be consistent to that. I personally believe that we should keep the bar for implementers low.

Sorry for the confusion here and thanks for flagging the issue in section 3.2.8. However, this is really a wording issue. The MUST is supposed to relate to the “send an ACK immediately” and was not supported to meant to say “MUST send an option”. However, the wording is clearly confusing and we will fix that!

The conclusion we arrived at of this general issue is that we RECOMMEND to implement but there has never been any obligation to use the option. I believe that it is clear throughout the document that the option is and always has been actually optional (also because in some cases TCP options might simply not useable) and we will make sure that we resolve any confusion that the sentence in section 3.2.8 could otherwise cause.

> 
> * Overall document
> 
>  This document contains some specification on caching. Given that we now have a WG document (2140bis), I think the information therein could be taken into account. For instance, a reference could be added and the terminology could be reused (most of that terminology is established in RFC 2140 already). I am a big fan of self-contained documents and therefore the specific guidance on caching for AccECN belongs into draft-ietf-tcpm-accurate-ecn. But that does not prevent referencing and leveraging RFC 2140 and/or 2140bis terminology.

Sure we can refer 2140bis informationally.
> 
> * Abstract/Introduction
> 
>  draft-ietf-tcpm-accurate-ecn currently has at least two roles: It allows echoing CE marks in SYN segments and therefore, in combination with draft-ietf-tcpm-generalized-ecn, allows to set ECT on SYNs, and it provides feedback more than once per RTT. I can think of scenarios in which the former is more beneficial than the latter, i.e., in which there is benefit of enabling ECT in SYNs, but feedback once per RTT is enough.
> 
>  One specific example (albeit it may not be the only one) is described in draft-ietf-lwig-tcp-constrained-node-networks: A sender on embedded devices with very small window (say, initially only one MSS) could probably benefit from having CE marks instead of loss for SYNs, but inherently there could only be one CE mark per RTT if there is at most one segment outstanding, so RFC3168-style feedback seems good enough. Being able to reflect more CE marks per RTT is not needed in that case and the implementation overhead of doing this on an IoT device may be questionable. This is an area very different e.g. from DCTCP, but TCP is a general purpose protocol that also matters in IoT. I am not aware of much current use of ECN in the IoT area, but there can be controlled environments in this space as well, which, theoretically, may be able to roll out ECN earlier than the public Internet. While I believe that the handshake and the more-than-once-per-RTT feedback during a connection are two separate items that could even be specified separately, I cannot ask for such separation as I do not own running code.
> 
>  But I believe it would at least make sense to better stress in the abstract and introduction that more accurate ECN information is provided during the handshake and for control segments. As I wrote, in some use cases this may be the (only) relevant part of the protocol..

As specified in the requirements RFC the goal of accurate ECN is to provide more accurate ECN feedback than once per RTT. Having feedback on a per packet basis and well as per byte is the design the group settle on which has the nice benefit that this also reflects marking of the SYN. I think the goal of AccECN is clearly stated in the document. However, if you have a concrete proposal for additional text, please send it.

> 
> * Figure 1
> 
>  Figure 1 is outdated. I do not understand why this document draws a picture with the "NS" flag, which is historic and has probably never been used in widely deployed running code. Figure 1 should be removed. Instead, what would make a lot of sense is to add a figure later in the normative part of the document, e.g. in Section 3.1. That figure should show the TCP header with "AE" flag as being used in SYN segments.

As the section clearly states this section provides a recap of other feedback mechanism in TCP and the picture shows "The (post-ECN Nonce) definition of the TCP header flags” as the title says. This is to make clear what the relation to the NS bit is as we re-use a TCP header flag for the first time. However, we could probably add another such diagram with AE instead of NS in the next section.

> 
> * Section 2.4 (and elsewhere)
> 
>  There is an (well, IMHO) unnecessary repetition of L4S and ConEx at many places in the document. I am fine with introducing it in the introduction. However, for instance, Section 2.4 states "If both are present, the byte counter in the option will provide the more accurate information needed for modern congestion control and policing schemes, such as L4S, DCTCP or ConEx." It would be sufficient to state "If both are present, the byte counter in the option will provide the more accurate information". At least for DCTCP it is IMHO not proven that the information in the _option_ is indeed needed, as implied by this wording. As also explained already, some aspects of AccECN could also be useful in quite different environments and it is unclear to me why only L4S, DCTCP and ConEx are mentioned so repeatedly as (only) examples.

Because these are the approaches that the IETF has specified, documented, and is working on...? However, given it says “as such” it not supposed to exclude other future mechanisms. I don’t really understand the concern here and would advocate for referencing other existing work of the IETF where possible.

> 
> * Section 3.1.2.  Forward Compatibility
> 
>   "If a TCP server that implements AccECN receives a SYN with the three
>   TCP header flags (AE, CWR and ECE) set to any combination other than
>   000, 011 or 111, it MUST negotiate the use of AccECN as if they had
>   been set to 111.  This ensures that future uses of the other
>   combinations on a SYN can rely on consistent behaviour from the
>   installed base of AccECN servers."
> 
>  I have thought about the pros and cons of this forward compatibility quite a bit and I am not entirely convinced of the benefits of this rule, e.g., one could equally argue that fallback to RFC 3168 would be quite reasonable as this is a standard that is safe to use. But I will not further push back on this aspect if I am the only one person who cares about it.
> 
>  Having said this, what IMHO is missing is an addition to the first sentence along the lines of "... unless the TCP server implements a TCP extension that uses these codepoints". It is well understood that as of today no such TCP extension exists. But the text of an RFC does not change and if it contains forward-looking statements, these statements should be written it in a way that the text stays valid also in future, as far as this is possible

I think this has be discussed extensively in the past. I also don’t really see the different for the addition you proposed as of course new RFCs may always add additional mechanism and there is nothing in the text here that excludes that.