Re: [tcpm] [Technical Errata Reported] RFC8257 (6697)

[Bob Briscoe <[e-mail]>]

Neal,

On 30/09/2021 16:43, Neal Cardwell wrote:
> I agree with the points made by Vidhi and Bob, and really like Bob's text.
>
> In the suggested text there may be a typo; I believe we want 
> s/SND.UNA/SND.NXT/.

[BB] Agree (and your next point about solely ECN indications).

I only said SND.UNA 'cos I was looking back at this earlier sentence in 
the RFC, and I copied the idea without engaging brain:

    o  DCTCP.WindowEnd: the TCP sequence number threshold when one
       observation window ends and another is to begin; initialized to
       SND.UNA.

Why does this say SND.UNA? Is this another erratum? I believe the Linux 
code initializes to SND.NXT in dctcp_reset(), which is called from 
dctcp_init():
https://elixir.bootlin.com/linux/v4.7/source/net/ipv4/tcp_dctcp.c#L78

BTW, step 7 correctly says SND.NXT:

    7.  Determine the end of the next observation window:

           DCTCP.WindowEnd = SND.NXT




Bob

> And probably we want to be more specific about only suppressing 
> further ECN-based reductions (further loss-triggered reductions would 
> be good to allow). I'm posting my suggested tweaks in blue, starting 
> from Bob's nice green text:
>
> SUGGESTED:
> ==========
>
> *3.4. Congestion Window Reduction*
>     Rather than always halving the congestion window as described in
>     [RFC3168]*, on the arrival of ECNcongestion feedback,*  the sender SHOULD
>     update cwnd as follows:
>
>        cwnd = cwnd * (1 - DCTCP.Alpha / 2)
>     Just as specified in [RFC3168], DCTCP does not react to congestion
>     indications more than once for every window of data.*Therefore, as for RFC3168 ECN, it sets the variable for the end of 
> congestion  window reduced (CWR) state to SND.NXTand suppresses 
> further ECN-triggeredreductions until this TCP sequence number is 
> acknowledged. Periods  of CWR state are triggered by congestion 
> feedback, and therefore  occur at times unrelated to the continuous 
> cycle of observation  windows used to update DCTCP.Alpha in Section 3.3.*
>
>     The setting of the CWR bit is also as per [RFC3168].  This is
>     required for interoperation with classic ECN receivers due to
>     potential misconfigurations.
>
> 3.*5*.  Handling of Congestion Window Growth...
>
> neal
>
>
> On Thu, Sep 30, 2021 at 11:22 AM Bob Briscoe <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>     Vidhi,
>
>     You're right. It's incorrect to have the window reduction hanging
>     off the end of the list of steps for updating the EWMA.
>
>     To make this concrete, here's some specific additional text (in
>     green for those with HTML mail readers). Also, rather than
>     splitting into sub-subsections, I have suggested that Item 9. of
>     the list in subsection 3.3 is moved out of the list, and instead
>     forms the basis of a new subsection 3.4. entitled "Congestion
>     Window Reduction".
>
>     CURRENT:
>     ========
>
>         9.  Rather than always halving the congestion window as described in
>             [RFC3168], the sender SHOULD update cwnd as follows:
>
>                cwnd = cwnd * (1 - DCTCP.Alpha / 2)
>
>         Just as specified in [RFC3168], DCTCP does not react to congestion
>         indications more than once for every window of data.  The setting of
>         the CWR bit is also as per [RFC3168].  This is required for
>         interoperation with classic ECN receivers due to potential
>         misconfigurations.
>
>     3.4. Handling of Congestion Window Growth...
>
>     SUGGESTED:
>     ==========
>
>     *3.4. Congestion Window Reduction*
>
>         Rather than always halving the congestion window as described in
>         [RFC3168]*, on the arrival of congestion feedback,*  the sender SHOULD
>         update cwnd as follows:
>
>            cwnd = cwnd * (1 - DCTCP.Alpha / 2)
>
>         Just as specified in [RFC3168], DCTCP does not react to congestion
>         indications more than once for every window of data.*Therefore, as for RFC3168 ECN, it sets the variable for the end
>     of congestion  window reduced (CWR) state to SND.UNA and
>     suppresses further  reductions until this TCP sequence number is
>     acknowledged. Periods  of CWR state are triggered by congestion
>     feedback, and therefore  occur at times unrelated to the
>     continuous cycle of observation  windows used to update
>     DCTCP.Alpha in Section 3.3.*
>
>         The setting of the CWR bit is also as per [RFC3168].  This is
>         required for interoperation with classic ECN receivers due to
>         potential misconfigurations.
>
>     3.*5*. Handling of Congestion Window Growth...
>
>
>     Then the of numbering all subsequent subsections of section 3.
>     will increment by 0.1.
>
>
>
>     Bob
>
>     On 28/09/2021 08:18, RFC Errata System wrote:
>>     The following errata report has been submitted for RFC8257,
>>     "Data Center TCP (DCTCP): TCP Congestion Control for Data Centers".
>>
>>     --------------------------------------
>>     You may review the report below and at:
>>     https://www.rfc-editor.org/errata/eid6697  <https://www.rfc-editor.org/errata/eid6697>
>>
>>     --------------------------------------
>>     Type: Technical
>>     Reported by: Vidhi Goel<[e-mail]>  <mailto:[e-mail]>
>>
>>     Section: 3.3
>>
>>     Original Text
>>     -------------
>>     The below pseudocode follows after DCTCP.Alpha is updated on ACK processing. This is wrong as cwnd should only be reduced using DCTCP.Alpha when ECE is received.
>>
>>     9. Rather than always halving the congestion window as described in
>>             [RFC3168], the sender SHOULD update cwnd as follows:
>>
>>                cwnd = cwnd * (1 - DCTCP.Alpha / 2)
>>
>>     Corrected Text
>>     --------------
>>     Instead, a new paragraph for Congestion Response to ECN feedback would be much clearer. First start with RFC 3168's response to ECE and then provide DCTCP's response to ECE.
>>
>>     I am thinking splitting section 3.3 into two sub-sections -
>>     3.3.1 Computation of DCTCP.Alpha
>>     3.3.2 Congestion Response to ECE at sender
>>
>>
>>
>>     Notes
>>     -----
>>     Although RFC 8257 refers to RFC 3168 congestion window halving at step 9, but it is confusing to put it right after step 8.
>>
>>     Instructions:
>>     -------------
>>     This erratum is currently posted as "Reported". If necessary, please
>>     use "Reply All" to discuss whether it should be verified or
>>     rejected. When a decision is reached, the verifying party
>>     can log in to change the status and edit the report, if necessary.
>>
>>     --------------------------------------
>>     RFC8257 (draft-ietf-tcpm-dctcp-10)
>>     --------------------------------------
>>     Title               : Data Center TCP (DCTCP): TCP Congestion Control for Data Centers
>>     Publication Date    : October 2017
>>     Author(s)           : S. Bensley, D. Thaler, P. Balasubramanian, L. Eggert, G. Judd
>>     Category            : INFORMATIONAL
>>     Source              : TCP Maintenance and Minor Extensions
>>     Area                : Transport
>>     Stream              : IETF
>>     Verifying Party     : IESG
>>
>>     _______________________________________________
>>     tcpm mailing list
>>     [e-mail]  <mailto:[e-mail]>
>>     https://www.ietf.org/mailman/listinfo/tcpm  <https://www.ietf.org/mailman/listinfo/tcpm>
>
>     -- 
>     ________________________________________________________________
>     Bob Briscoehttp://bobbriscoe.net/  <http://bobbriscoe.net/>
>
>     _______________________________________________
>     tcpm mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/tcpm
>     <https://www.ietf.org/mailman/listinfo/tcpm>
>

-- 
________________________________________________________________
Bob Briscoe                               http://bobbriscoe.net/