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

["Scharf, Michael" <[e-mail]>]

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.

* 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.

* 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..

* 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.

* 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.

* 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.

* Section 3.2.3 (and elsewhere)

  As far as I understand, some of the heuristics may not work well if there is reordering, e.g., between the ACK completing the 3way handshake and the first data packet. The document explains well that loss could be an issue, but I think reordering should be mentioned, too. An example is Section 3.2.3. Usually, I would flag this comment as a minor editorial. However, given that it is currently proposed to allow more reordering in L4S, and given that this document repeatedly mentions L4S, I think the downsides of reordering have to be emphasized in this document.

* Section 3.2.7.3

   "While a host is in this mode that assumes incoming AccECN Options are
   not available, it MUST adopt the conservative interpretation of the
   ACE field discussed in Section 3.2.5."

 If sending AccECN options is not mandatory-to-implement, this may be the norm. But what does this MUST precisely ask for? As far as I can see, Section 3.2.5 mostly has a SHOULD on counter cycling and an example algorithm, but explicitly also allows other algorithms. So, it is not clear to me what this MUST ask for precisely. Is the MUST about the example algorithm? The same question also arises in Section 3.2.7.4. What do I miss?

* Section 3.2.8

  The question whether a change-triggered ACK indeed has to include an AccECN option has already mentioned above. Personally, I doubt that the option is needed, but implementers may be in a better position to comment.

  I think I have said this in the past already: To me, if it is not known if hardware can implement this, it is a perfect reason for a SHOULD. To me, this can only be a MUST if an implementation proves that it is doable with _current_ offloading technology. I don't think we should run an experiment to learn whether future hardware can indeed implement our specification. For a MUST, there should at least be some level of confidence at the time of writing.   Otherwise, a general MUST on immediately sending an ACK (even without the option) may sound like an example for Section 1 of RFC 6919 (be aware of the date!).

* Also Section 3.2.8

  In particular if the sending the TCP option is not optional in change-triggered ACKs, I believe the discussion on overhead should not only mention additional ACKs but also their size (number of bytes). Nobody would care about that in a data center at 100 Gbit/s. But think of the option byte overhead in very narrowband IoT links...

* Section 3.3

   "In data centres it has been fortunate for offload hardware that
   DCTCP-style feedback changes less often when there are long sequences
   of CE marks, which is more common with a step marking threshold.  In
   order to enable DCTCP to improve its responsiveness, DCs will need to
   move beyond step marking.  Before this can happen, offload hardware
   will have to explicitly address the variability of ECN feedback.

   ECN encodes a varying signal in the ACK stream, so it is inevitable
   that offload hardware will ultimately need to handle any form of ECN
   feedback exceptionally.  The purpose of working towards standardized
   TCP ECN feedback is to reduce the risk for hardware developers, who
   will have to choose which scheme is likely to become dominant."

  I don't understand why these two sections are needed in this document. If they stay in there, I believe this would require more explanation and backing e.g. by references, most notably the statements on "step marking". From this document, it is unclear what this is all about. But I don't think this discussion is needed here at all.

* Section 5

  My own implementation experience with using TCP options is not recent. But when I worked with new TCP options in the times of Linux kernel 2.6, there have been a bunch of implementation challenges. It is surprising that the section on "complexity" only mentions counters, not option processing. And what about offloading hardware complexity?

* Section 6

 As explained, this section is not covered by this review

* Section 7

  I don't know where this fits best in this document, but to somebody who looks at Wireshark traces from time to time, the AccECN protocol has a one disadvantage: If the three-way handshake is not captured in a packet trace, I believe it is not trivial to correctly render the fixed TCP header in a debugging tool, as a tool like Wireshark cannot easily know whether AccECN has been negotiated or not without seeing the SYNs. This inconvenience is similar to the 'window' field, which also cannot always be displayed correctly in a tool without having seen the SYN and SYN, ACK and knowing window scale factor. But AccECN is actually somewhat worse, because now even the borders between TCP header fields are unknown (2 single bits vs. a 3 bit counter). Heuristics seem possible that can guess enabling of AccECN from an incomplete packet capture trace, but there is still a risk of a wrong outcome, most notably if the connection is short and no TCP options are used. Then the TCP header is simply displayed wrongly in a tool GUI. This may be more a manageability than a security issue, but IMHO it can be inconvenient during TCP debugging (or, well, teaching students...). I believe this very small (but real-world) downside of the protocol should be mentioned somewhere - albeit this issue clearly may not be significant enough to change the protocol. The challenge also exists for other passive monitoring solutions possibly using sampling or TCP middleboxes. Needless to say that I don't care here about middleboxes.


Minor comments:

* Section 1.4.  Terminology

  I believe it would make sense to introduce "ACE field".

* Section 2.

  " o  a supplementary part using a new AccECN TCP Option..."

  Please make clear what "supplementary" means with respect to mandatory-to-implement guidance.

* Section 2.3.  Delayed ACKs and Resilience Against ACK Loss

   "With both the ACE and the AccECN Option mechanisms, the Data Receiver..."

  I think "ACE field" would be more consistent.

* Section 2.4

   "... which is why [RFC5681] now recommends
   that TCP counts acknowledged bytes not packets."

  I think that sentence works well without "now".

* Section 2.5

   "private networks (e.g. data centres)"

  Spelling will be sorted out by the RFC editor. But, as a non-native speaker, I find the spelling "data centre" really confusing, specifically since the document also uses "data center" when expanding DCTCP. At least for this term I think it would be reasonable to always use the term well-known to industry in most parts of the world.

* Section 3.2.6.  The AccECN Option

  The statement "Length = 11" in Figure 3 is misleading, as the length of the option is actually variable, as explained later in the same section.

* Serction 4.2

   "AccECN is compatible (at least on paper) with the most commonly used"

  I think we should avoid paper-only work in TCPM and thus I don't like this wording. If this compatibility has not (entirely) been verified by running code, this should be mentioned in the shepherd write-up that has to comment on running code.

* Section 5

  The subsection "Backward Compatibility" is repeated twice.