Re: [OPS-DIR] Opsdir last call review of draft-ietf-tcpm-dctcp-07

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 8, 2017 at 11:53 AM, Joe Clarke <jclarke@xxxxxxxxx> wrote:
> Reviewer: Joe Clarke
> Review result: Has Nits
>
> Hello, WG and authors.  I have reviewed rev -07 of the draft-ietf-tcpm-dctcp as
> requested by the OPS-DIR.  This review focuses on improving operational aspects
> as well as any nits found in the text.

... and I'd quickly like to thank Joe for his review. Fixing issues
like this make the documents progress through the IESG with much less
drama...

W


>
> This document is an informational draft that describes Data Center TCP (DCTCP),
> a congestion control mechanism for TCP in Data Center environments.
>
> Overall, I believe this document to be ready, with some nits and perhaps small
> areas for improved clarity and readability.  First, I'd like to say that I
> appreciate the fact that this has been implemented on a number of kernels, and
> the authors included real-world implementation results and thoughts.  From an
> operational perspective, that is very helpful.  I also appreciated the fact
> that there are interoperability challenges, and those were called out in the
> document.  My specific comments are below.
>
> There are a lot of abbreviations, variables and other terminology used
> throughout this document.  It might be helpful for the reader to have an
> expanded terminology section at the top that one can refer to for all of these
> things.  Some of the abbreviations are called out in the description of the
> algorithm, but not all (e.g., DCTCP.Alpha, CWR, RTT, etc.).
>
> ===
>
> Section 3.2:
>
> You refer to DCTCP.Alpha before defining it.  While you refer to Section 3.3
> here, the impact of an incorrect Alpha value is not fully appreciated in this
> text.  Perhaps this could be changed to reflect the impact the incorrect Alpha
> value would have?
>
> ===
>
> Section 3.2:
>
> My abbreviating DCTCP.CE as CE in your state machine diagram, it is a bit
> confusing as to the difference between CE and DCTCP.CE.  The description of the
> state machine above requires the CE codepoint to have a certain value in order
> for DCTCP.CE to change.  Perhaps you can use D.CE as an abbreviation to be a
> bit clearer here.
>
> ===
>
> Section 3.3:
>
> It is not clear if 'g' can be inclusive of 0 and 1.
>
> ===
>
> Section 3.3:
>
> You define DCTCP.WindowEnd as the threshold for beginning a new observation
> window, but maybe to complement the state variable name, you should define it
> as the following:
>
> The TCP sequence number threshold when one observation window ends and other is
> to begin; initialized to SND.UNA.
>
> ===
>
> Section 3.3:
>
> You state:
>
> Thus, when no bytes sent experienced congestion, DCTCP.Alpha equals
> zero, and cwnd is left unchanged
>
> But if I use a value of 1/16 for g, with DCTCP.Alpha initialized to 1 as you
> say, I get a value of DCTCP.Alpha == 15/16 when there is no congestion (i.e., M
> == 0).
>
> ===
>
> Section 3.5:
>
> You have an extra space here before the comma:
>
> If SYN , SYN-ACK and RST packets for DCTCP connections have ECT set
>
> This should be:
>
> If SYN, SYN-ACK and RST packets for DCTCP connections have ECT set
>
> ===
>
> Section 3.5:
>
> You do not define ECT before using it.
>
> ===
>
> Section 4.1:
>
> Can you provide a reference for NewReno?
>
> ===
>
> Section 5:
>
> Can you reference or define AQM and RED?
>
>
> _______________________________________________
> OPS-DIR mailing list
> OPS-DIR@xxxxxxxx
> https://www.ietf.org/mailman/listinfo/ops-dir



-- 
I don't think the execution is relevant when it was obviously a bad
idea in the first place.
This is like putting rabid weasels in your pants, and later expressing
regret at having chosen those particular rabid weasels and that pair
of pants.
   ---maf




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Fedora Users]