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

From: Mirja Kuehlewind <[email-masked]>
Date: Tue, 5 Nov 2019 15:08:01 +0100

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 <[email-masked]> 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.

---

From: "Scharf, Michael" <[email-masked]>
Date: Tue, 12 Nov 2019 23:19:13 +0000

Inline...

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

I'd suggest to add to abstract and introduction a sentence along the lines of "The AccECN negotiation has been designed as a generic reflector that also provides feedback for ECN markings in SYN segments."

To me, this reflector property during SYNs could in certain cases be much more relevant than the feedback more than once per RTT. Therefore, I believe it should be made explicit in the abstract. As far as I understand, this objective has also significantly influenced the design of the AccECN protocol during the 3-way handshake and the functionality does not come entirely for free, e.g., as it consumes codepoint combinations.

Also, out of my head, this requirement is not clearly spelt out in RFC 7560, i.e., the protocol may actually outperform the requirements. That would be another reason why to mention 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.

I continue to believe that Figure 1 does not help and should be removed from this section.

First, given that the experiment is finished, we really should stop talking about "NS flag". People should use the new term consistently from now on. The best way to achieve this is IMHO just to show figures with the new header (i.e., according to AccECN). I believe it is not uncommon for readers just to scan through the document and look at the first ASCII art for a header they can find. In that case, Figure 1 is exactly the wrong content. Note that one way to avoid that would be to move Figure 1 to an appendix. That could work for me. The new second Figure with the AE flag could somewhere note that the AE flag is identical to the former NS flag, in order to high-light relation. Then there is no added value of the current Figure 1 in my point of view.

Second, the caption of Figure 1 "The (post-ECN Nonce) definition of the TCP header flags" entirely lacks a warning sign that this is a historic picture. I believe wording like "The historic RFC 3540 definition of TCP header flags" would be required, say, in an Appendix version of this figure if removal is not an option.

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

The specific comment in the quoted section is about the wording "the byte counter in the option". And it seems that L4S does not 'need' the option, and neither does DCTCP. I could read this sentence as a requirement that the AccECN option is mandatory to implement in L4S or DCTCP.

Maybe that is not the intention but then please rephrase it as it confuses as least me. I have provided an alternative sentence that has no ambiguity in my view.  

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

Maybe we can just agree to disagree on that.

I believe others in the working group should have a careful look at this section and its deployment implications, most notably in combination with other future TCP extensions. I also believe this section requires careful review during IETF last call as it is about a general design pattern. As long as I am the only person concerned about this section and its implications, my proposal is that the shepherd notes down that one individual contributor believes that fallback should be to RFC 3168 as standard and the contributor agreed to move on nonetheless.

But, as I said, I flag this section mainly to ensure that the forward compatibility rule is carefully reviewed by all who may be affected.

Michael