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

From: Bob Briscoe <[email-masked]>
Date: Wed, 3 Nov 2021 18:09:04 +0000

TCPM chairs,

The status of this erratum is 'Reported'. I think some consensus was 
reached on the list. What happens now? Who is meant to propose updated 
text for the erratum based on the discussion?

Bob

On 01/10/2021 14:12, Bob Briscoe wrote:
> 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 <[email-masked] 
>> <mailto:[email-masked]>> 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<[email-masked]>  <mailto:[email-masked]>
>>>
>>>     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
>>>     [email-masked]  <mailto:[email-masked]>
>>>     https://www.ietf.org/mailman/listinfo/tcpm  <https://www.ietf.org/mailman/listinfo/tcpm>
>>
>>     -- 
>>     ________________________________________________________________
>>     Bob Briscoehttp://bobbriscoe.net/  <http://bobbriscoe.net/>
>>
>>     _______________________________________________
>>     tcpm mailing list
>>     [email-masked] <mailto:[email-masked]>
>>     https://www.ietf.org/mailman/listinfo/tcpm
>>     <https://www.ietf.org/mailman/listinfo/tcpm>
>>
>
> -- 
> ________________________________________________________________
> Bob Briscoehttp://bobbriscoe.net/

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

---

From: [email-masked]
Date: Wed, 3 Nov 2021 20:02:28 +0100

> On 3. Nov 2021, at 19:09, Bob Briscoe <[email-masked]> wrote:
> 
> TCPM chairs,
> 
> The status of this erratum is 'Reported'. I think some consensus was reached on the list. What happens now? Who is meant to propose updated text for the erratum based on the discussion?
Hi Bob,

as far as I know, the WG chairs can't change erratas. That might be possible for AD, I think,
but I'm not sure.

Best regards
Michael
>>> 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/.
>> 
>> 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:
>> 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():
>>> 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
>>>    [RFC3168]
>>> , on the arrival of ECN congestion feedback,
>>>  the sender SHOULD 
>>>    update cwnd as follows:
>>> 
>>>    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.NXT
>>>  and suppresses further
>>>    
>>> ECN-triggered
>>>  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.
>>> 
>>> 
>>> 3.
>>> 5.  Handling of Congestion Window Growth...
>>> 
>>> neal
>>> 
>>> 
>>> On Thu, Sep 30, 2021 at 11:22 AM Bob Briscoe <[email-masked]> 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:
>>> ========
>>> 3.4
>>> .  Handling of Congestion Window Growth...
>>> 
>>> 
>>> SUGGESTED:
>>> ==========
>>> 
>>> 3.4. Congestion Window Reduction
>>>    [RFC3168]
>>> , on the arrival of congestion feedback,
>>>  the sender SHOULD 
>>>    update cwnd as follows:
>>> 
>>>    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.
>>> 
>>> 
>>> 3.5.  Handling of Congestion Window Growth...
>>> 
>>> Then the of numbering all subsequent subsections of section 3. will increment by 0.1.
>>> 
>>> 
>>> 
>>> Bob
>>> 
>>>> https://www.rfc-editor.org/errata/eid6697
>>>> 
>>>> 
>>>> Reported by: Vidhi Goel 
>>>> <[email-masked]>
>>>> [email-masked]
>>> [email-masked]

---

From: Vidhi Goel <[email-masked]>
Date: Wed, 03 Nov 2021 17:37:01 -0700

> as far as I know, the WG chairs can't change erratas. That might be possible for AD, I think,

I am a bit new to the errata process and would like to understand a bit more about next steps. Based on Bob’s suggestion about updated text and feedback from others, how do we proceed to make the necessary change to the RFC 8257?
Is the only thing we can do now is somehow update the Suggestion in errata itself? When would the update me applied to the RFC?

Thanks,
Vidhi

> On Nov 3, 2021, at 12:02 PM, [email-masked] wrote:
>

---

From: [email-masked]
Date: Thu, 4 Nov 2021 09:26:14 +0100

> On 4. Nov 2021, at 01:37, Vidhi Goel <[email-masked]> wrote:
> 
Hi Vidhi,

an RFC can be changed when it is published. Erratas can be filed, but can't be changed by
the submitter. I think ADs can change Erratas. So if a group agrees on text changes for
an RFC, I think ADs have the power to change Erratas and therefore the text change can
go in via this method.
If you actually want to change an RFC, a bis document needs to be published.

Best regards
Michael

---

From: t petch <[email-masked]>
Date: Thu, 4 Nov 2021 16:57:21 +0000

> Is the only thing we can do now is somehow update the Suggestion in errata itself? When would the update me applied to the RFC?

The usual form of an Erratum is along the lines of
OLD
<current text>
NEW
<proposed text>
Note
Section 31.102 is ambiguous and could be interpreted as ... or .... 
This erratum .....

Discussion may result in a better version of <proposed text> in which 
case the usual practice is to reject the Erratum and craft a new one.

ADs sometimes add notes explaining why the action that is being taken is 
being taken but I think it unusual for an AD to do more than that.

This Erratum I do not understand and so would reject.  What is the
problem?  What is the fix?

As others have said, an Erratum cannot change the consensus expressed in 
the RFC.  The RFC is immutable.  A change of meaning is a new RFC.

Perhaps discuss on the list what you perceive the problem to be and if 
there is consensus that there is a problem and that it falls within the 
limited scope of an Erratum, then craft an Erratum.

Tom Petch

---

From: Martin Duke <[email-masked]>
Date: Thu, 4 Nov 2021 13:38:40 -0700

Yes, I'm the one to apply the erratum to the RFC.

The rfc-editor page presents both the original and corrected version of the
RFC. A future RFC that obsoletes this one would be expected to incorporate
all verified errata.

On Thu, Nov 4, 2021 at 9:57 AM t petch <[email-masked]> wrote:

> reached on the list. What happens now? Who is meant to propose updated text
> for the erratum based on the discussion?
> >> Hi Bob,
> >>
> > I am a bit new to the errata process and would like to understand a bit
> more about next steps. Based on Bob’s suggestion about updated text and
> feedback from others, how do we proceed to make the necessary change to the
> RFC 8257?
> > Is the only thing we can do now is somehow update the Suggestion in
> errata itself? When would the update me applied to the RFC?
>
>
> reached on the list. What happens now? Who is meant to propose updated text
> for the erratum based on the discussion?
> >> Hi Bob,
> >>
> Linux code initializes to SND.NXT in dctcp_reset(), which is called from
> dctcp_init():
> 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:
> >>>>> ==========
> >>>>>
> 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:
> >>>>> ========
> >>>>>> 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.
> >>>>>>
> >>>>>> 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.
> >>>>>>
> >>>>>> 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:
> >>>>>> -------------

---

From: Martin Duke <[email-masked]>
Date: Thu, 4 Nov 2021 14:54:00 -0700

As someone who hasn't implemented DCTCP, I'd like to understand the
proposed changes. While there's some editorial rearranging, which is
usually not good grounds for a verified errata, there are three concrete
concerns I see in the thread.

1) WindowEnd should be initialized to SND.NXT, not SND.UNA.

These are the same at the time of initialization (IIUC, entering
ESTABLISHED), no? The very first ACK (if not ECE) will set WindowEnd to
SND.NXT = (end of the first flight). If the first ack is ECE, then we'll
run through step 9 and have an immediate cwnd adjustment with Alpha=1. ISTM
there is an issue with the initial conditions in the latter case, but not
one fixed by initializing to SND.NXT.

2) If SEG.ACK < DCTCP.WindowEnd, it skips steps 5 through 9. It should skip
5-8 and still execute step 9.

My understanding from the paragraph after step 9 is that cwnd reduction in
step 9 should be done only once per RTT, so the original text seems
correct?

3) Step 9 should only apply when the ECE flag is set.

As this suggests the rule would only apply if the last ack in the window
was ECE, I think the correct condition would be M > 0 (i.e. something was
marked in the past window)

If I've missed a concern or mangled one of the three listed, don't hesitate
to speak up!

Martin

On Thu, Nov 4, 2021 at 1:38 PM Martin Duke <[email-masked]> wrote:

> The rfc-editor page presents both the original and corrected version of
> the RFC. A future RFC that obsoletes this one would be expected to
> incorporate all verified errata.
>

---

From: Vidhi Goel <[email-masked]>
Date: Thu, 04 Nov 2021 15:37:21 -0700

> These are the same at the time of initialization (IIUC, entering ESTABLISHED), no? The very first ACK (if not ECE) will set WindowEnd to SND.NXT = (end of the first flight). If the first ack is ECE, then we'll run through step 9 and have an immediate cwnd adjustment with Alpha=1. ISTM there is an issue with the initial conditions in the latter case, but not one fixed by initializing to SND.NXT.

I think it is consistent to simply initialize it WindowEnd = SND.NXT, that way it is semantically correct and consistent with the future assignment in step 7.

> 2) If SEG.ACK < DCTCP.WindowEnd, it skips steps 5 through 9. It should skip 5-8 and still execute step 9.
> 
> My understanding from the paragraph after step 9 is that cwnd reduction in step 9 should be done only once per RTT, so the original text seems correct? 

Yes, the cwnd reduction is done once per RTT but there is no synchronization between update of alpha and cwnd reduction (and subsequent CWR). Meaning, we update alpha regardless of whether CE mark was received or not but we only do cwnd reduction if CE is received. So, these two independent sub-modules could have a completely different RTT cycle.

We still need to do what Bob said,
So, steps 5-8 really needed to be sub-bullets of step 4. And step 9 would be renumbered step 5.

> As this suggests the rule would only apply if the last ack in the window was ECE, I think the correct condition would be M > 0 (i.e. something was marked in the past window)
No. As I said above, the update of alpha is separate from cwnd reduction due to ECE. So, we still need to say in step 9, if ECE was received…

Combining all the suggestions from the email, below is how I put it together,

CURRENT:
========

 
 
SUGGESTED:
==========
Steps 5-8 are sub-parts of step 4, i.e. steps 5, 6, 7 and 8 are 4.1, 4.2, 4.3 and 4.4 respectively and these should be followed if condition specified in step 4 is true. Step 9 is renumbered as step 5 which is followed independent of condition in step 4.
5. 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.NXT 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 step 4.

——————————

Looking forward to hear from others to refine the text.

Thanks,
Vidhi

> On Nov 4, 2021, at 2:54 PM, Martin Duke <[email-masked]> wrote:
> 
> As someone who hasn't implemented DCTCP, I'd like to understand the proposed changes. While there's some editorial rearranging, which is usually not good grounds for a verified errata, there are three concrete concerns I see in the thread.
> 
> These are the same at the time of initialization (IIUC, entering ESTABLISHED), no? The very first ACK (if not ECE) will set WindowEnd to SND.NXT = (end of the first flight). If the first ack is ECE, then we'll run through step 9 and have an immediate cwnd adjustment with Alpha=1. ISTM there is an issue with the initial conditions in the latter case, but not one fixed by initializing to SND.NXT.
> 
> 2) If SEG.ACK < DCTCP.WindowEnd, it skips steps 5 through 9. It should skip 5-8 and still execute step 9.
> 
> My understanding from the paragraph after step 9 is that cwnd reduction in step 9 should be done only once per RTT, so the original text seems correct? 
> 
> As this suggests the rule would only apply if the last ack in the window was ECE, I think the correct condition would be M > 0 (i.e. something was marked in the past window)
> 
> If I've missed a concern or mangled one of the three listed, don't hesitate to speak up!
> 
> Martin
> 
> On Thu, Nov 4, 2021 at 1:38 PM Martin Duke <[email-masked] <mailto:[email-masked]>> wrote:
> The rfc-editor page presents both the original and corrected version of the RFC. A future RFC that obsoletes this one would be expected to incorporate all verified errata.
> 
> On Thu, Nov 4, 2021 at 9:57 AM t petch <[email-masked] <mailto:[email-masked]>> wrote:
> >> On Nov 3, 2021, at 12:02 PM, [email-masked] <mailto:[email-masked]> wrote:
> >>
> >>> On 3. Nov 2021, at 19:09, Bob Briscoe <[email-masked] <mailto:[email-masked]>> wrote:
> >>>
> >>> TCPM chairs,
> >>>
> >>>> https://elixir.bootlin.com/linux/v4.7/source/net/ipv4/tcp_dctcp.c#L78 <https://elixir.bootlin.com/linux/v4.7/source/net/ipv4/tcp_dctcp.c#L78>
> >>>>
> >>>>> On Thu, Sep 30, 2021 at 11:22 AM Bob Briscoe <[email-masked] <mailto:[email-masked]>> wrote:
> >>>>> Vidhi,
> >>>>>
> >>>>>> <[email-masked] <mailto:[email-masked]>>
> >>>>>>
> >>>>>>
> >>>>>> Section: 3.3
> >>>>>>
> >>>>>> Original Text
> >>>>>> -------------
> >>>>> Bob Briscoe
> >>>>> http://bobbriscoe.net/ <http://bobbriscoe.net/>
> >>>> Bob Briscoe
> >>>> http://bobbriscoe.net/ <http://bobbriscoe.net/>
> >>>
> >>> --
> >>> Bob Briscoe
> >>> http://bobbriscoe.net/ <http://bobbriscoe.net/>
> >>
> >
> >
> >