Subject: Re: [tcpm] WGLC for draft-ietf-tcpm-alternativebackoff-ecn-06
From: Michael Welzl <[email-masked]>
To: Markku Kojo <[email-masked]>
Cc: Michael Tuexen <[email-masked]>, [email-masked]
Date: Sat, 17 Mar 2018 08:17:12 +0100
List-Id: TCP Maintenance and Minor Extensions Working Group <tcpm.ietf.org>

Dear Markku,

Thank you very much for reading this!
Answers in line below:

> On Mar 17, 2018, at 1:52 AM, Markku Kojo <[email-masked]> wrote:
> 
> Hi all,
> 
> Maybe slightly late wg last call review of draft-ietf-tcpm-alternativebackoff-ecn-06.
> 
> The draft is mostly well written and clear, and it is also improved since version -00 or -01 I last read it.
> 
> I believe the intention of the draft is just to allow and propose experimentation using a larger multiplicative decrease factor when responding to ECN indication than when responding to packet loss.
> 
> There is, however, a significant change that seem to have occured from version -04 to -05 that does much more than this and is therefore crucial to address properly. The text and reads now in -06 as follows:
> 
> 
>  It updates the following text in section 6.1.2 of the ECN
>  specification [RFC3168] :
> 
>     The indication of congestion should be treated just as a
>     congestion loss in non-ECN-Capable TCP.  That is, the TCP source
>     halves the congestion window "cwnd" and reduces the slow start
>     threshold "ssthresh".
> 
>  Replacing this with:
> 
>     Receipt of a packet with the ECN-Echo flag SHOULD trigger the TCP
>     source to set the slow start threshold (ssthresh) to 0.8 times the
>     FlightSize, with a lower bound of 2 * SMSS applied to the result.
>     As in [RFC5681], the TCP sender also reduces the cwnd value to no
>     more than the new ssthresh value.
> 
> 
> 
> And is further clarified in Section 4.3:
> 
> 
> and in response to an indication of an ECN-signalled congestion:
> 
>       ssthresh = max (FlightSize * beta_{ecn}, 2 * SMSS)
> 
>       and
> 
>       cwnd = ssthresh
> 
> 
> 
> In my reading this text above sets a lower bound of 2 MSS for cwnd which is incorrect and dangerous modification and maybe not intended to modify RFC 3168.

Just to be clear, indeed we absolutely did not intend to change the spec here - however we thought we need to be clearer.

Consider the original text:
***
    The indication of congestion should be treated just as a
    congestion loss in non-ECN-Capable TCP.  That is, the TCP source
    halves the congestion window "cwnd" and reduces the slow start
    threshold "ssthresh”. 
***

Because this simply says “reduces” about ssthresh without specifying in detail how to reduce it, it would seem obvious (due to the first sentence) that this refers to eqn. 4 in RFC 5681. However, if we say “...set the slow start threshold (ssthresh) to 0.8 times the FlightSize”, then we lose the lower bound. This is why we introduced it for ssthresh.

As for cwnd, I find the original text to be contradictory in this respect (unless I’m missing something). “The indication of congestion should be treated just as a congestion loss in non-ECN-Capable TCP. “ => this, to me, means doing the same as specified in RFC 5681 - but in RFC 5681, I couldn’t see any text that allows cwnd to go below 2 MSS upon a “congestion loss” (assuming this is not a timeout - where you’re right, our text wrongly quoted RFC 5681 there, but that’s a separate issue further below).

( Maybe you mean *during*, not after fast recovery? But that would be confusing, as cwnd is also “inflated” in this phase in RFC 5681, which I hope we don’t do in response to ECE. )

However, RFC 3168, in the second sentence, beginning with “That is, “ (indicating that this is just a clarification), states that the TCP sources halves cwnd.

> The lower bound for cwnd is 1 MSS. The lower bound of 2 MSS for cwnd
> 
> 1) is not specified in any (standards track) RFC,
> 2) is against the congestion control principles for avoiding
>   congestion collapse as it effectively disables a full back
>   off that is a crucial requirement for all congestion control
>   algorithms,
> 3) is in conflict with the basic principle set in Sally's ECN papers
>   and in RFC 3168 itself (the original text above from RFC 3168):
> 
>   "The indication of congestion should be treated just as a
>    congestion loss in non-ECN-Capable TCP."
> 
>   When Flightsize (and cwnd) is 2 MSS in non-ECN-Capable TCP and loss
>   signals congestion, cwnd effectively becomes 1 MSS. That is why RFC
>   3168 formulates the text: "the TCP source halves the congestion
>   window "cwnd" …"

See above, regarding this text. We certainly don’t want to change these principles but tried to be correct about what should happen, and found the quoted text here to be contradictory.

>   Note: the majority of the work and text in RFC 3168, including
>   the text in RFC 2482, predates RFC 2581 that first introduced
>   FlightSize. Prior to RFC 2581 all TCP CC text "halved cwnd"
>   which literally meant cwnd=cwnd/2.

I know that, and I thought that this is the reason for the problem with this text (due to updates of the non-ECN CC text).

> Instead of the above text in the draft, the draft should explicitly state that when FlightSize/2 would result in ssthresh below its lower bound of 2 MSS, a TCP sender must not stop decreasing cwnd. And, if more ECN-signalled congestion indications arrive, the TCP sender must continue reducing sending rate further (plus refer to RFC 3168 how to proceed reducing the sending rate, if further ECE signals arrive).

Technically this makes sense to me, but I’m not currently reading RFC 3168 this way.

> Furthermore, the draft should be very specific on how the TCP sender should act when cwnd=ssthresh and ECE arrives. Currently the draft specifies the way how the TCP sender reduces ssthresh (and cwnd) in Congestion Avoidance. According to RFC 5681, when cwnd = ssthresh, TCP is not in Slow Start nor in Congestion Avoidance. Therefore, the draft should be explicit on how cwnd is reduced, when cwnd=ssthresh.

Why? RFC 5681 states “When cwnd and ssthresh are equal, the sender may use either slow start or congestion avoidance.” - so if you're an implementer, this basically tells you that you have to make a decision on your own for the cwnd == sthresh case. We specify how to behave when you’re in congestion avoidance - thus it depends on the decision you’ve taken for this case.

> It is true that there is (at least) one (widely deployed) incorrect implementation of RFC 3168 that incorrectly sets lower bound of 2 MSS for cwnd when congestion is ECN-signaled. This, however, must not be used as any kind of justification for changíng IETF-specified congestion control principles and requirements. Instead, this draft should carefully instruct implementors and those who intend to experiment with the ECN modification set in this draft to ensure the TCP stack used for experiments correctly implements RFC 3168 in this respect. Otherwise, such experimental results are mostly void.

Maybe the person behind this implementation was as confused by the RFC 3168 as I am … clearly we didn’t intend to change the principles.

> In addition, there is an incorrect statement in Sec 4.1:
> 
> According to
> [RFC5681], when a TCP sender detects segment loss using the
> retransmission timer and the given segment has not yet been resent by
> way of the retransmission timer, the value of ssthresh must be set to
> no more of the maximum of half of the FlightSize and 2*SMSS.  The
> same equation is also used during Fast Retransmit/Fast Recovery
> [RFC5681].  As a result, the TCP congestion control only allows one
> BDP of packets in flight.  This is just sufficient to maintain 100%
> utilisation of the bottleneck on the network path.
> 
> 
> What the text says about setting ssthresh is correct. However, if the segment loss is detected using the retransmission timer, the TCP
> congestion control does not allow one BDP of packets in flight (unless BDP=1 MSS). Instead, cwnd is set to 1 MSS, allowing only 1 pkt in flight.
> 
> I suggest removing the text that discusses detecting segment loss using the retransmission timer.

Good catch, thanks!

Cheers,
Michael