I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
Please resolve these comments along with any other Last Call comments you
may receive.
Document: review of draft-ietf-soc-overload-rate-control-09
Reviewer: Peter Yee
Review Date: August-22-2014
IETF LC End Date: August-22-2014
IESG Telechat date: TBD
Summary: This draft is on the right track but has open issues, described in
the review. [Ready with issues]
The draft describes an alternate SIP overload control mechanism. Instead of
the default, loss-based scheme, it proposes an optional, rate-based scheme.
The draft is mostly straightforward although the language is labored in some
places. Coordination with draft-ietf-soc-overload-control should take place
before this draft advances to ensure proper alignment with the final form of
that draft.
Issues:
Page 4, 4th paragraph: draft-ietf-soc-overload-control tends to speak of
parameters in the singular since they are being dealt with in a single Via
header field between a client and server. In this particular case, the
wording ends up sounding like a single Via header field might have more than
one oc parameter. Perhaps aligning with the base document would be best?
Page 6, 3rd paragraph: this whole paragraph is presumptive of all clients
performing rate-based overload control, but that presumption isn't stated.
Given that this document is introducing a second overload control algorithm,
it might not be realistic to assume that all clients implement or select the
same algorithm let alone any algorithm at all. Also, there's an unspoken
assumption in the scenario that clients are not initiating any traffic
themselves. Is that always going to be the case?
Page 6, 5th paragraph: this paragraph is supposing that all clients of the
server support overload control and furthermore support rate-based control,
but these suppositions aren't mentioned. The oc parameter should only be
returned to clients that have indicated they support overload control. I
know you know this, but the wording isn't clear on that point.
Page 7, 1st full paragraph: what's being called "oc" in the formula isn't
really "oc" but rather "oc value". While it's probably clear what you
meant, the base spec talks about the server inserting a value into this
parameter.
Page 14, Section 5, "algo-value" extension: This is currently called
algo-list in draft-ietf-soc-overload-control-15, the last version of that
spec. RFC 3261 doesn't have an algo-value definition.
Nits:
Page 1, author block: far be it for me to tell an author what his name is,
but I would suggesting changing " Philip M Williams" to " Philip M.
Williams" unless M is that author's middle name.
Page 2, Introduction, 1st paragraph, 1st sentence: change "large scale" to
"large-scale".
Page 2, Introduction, 1st paragraph, 1st sentence: change "SIP based" to
"SIP-based". Do this globally in the document.
Page 3, 1st paragraph, 1st sentence: insert "a" before "loss".
Page 3, 1st paragraph, 2nd sentence: append "scheme" after "control".
Insert "that" before "clients" and delete the subsequent "to".
Page 3, 2nd paragraph, 1st sentence: insert "scheme" after "control".
Page 3, 2nd paragraph, last sentence: delete the comma after "client".
Page 3, 3rd paragraph: change "a rate based" to "a rate-based". Append a
comma after "default". Insert "scheme" between "control" and "in".
Page 4, 3rd paragraph: append a comma after "e.g.". Do this globally and
also for "i.e.".
Page 4, 3rd paragraph: insert "or" between "utilization" and "queueing".
Delete the comma after "utilization". Delete the ellipsis.
Page 4, Section 3.2, 1st paragraph, 1st sentence: capitalize "via". Append
"field" after "header".
Page 5, 1st paragraph, 2nd sentence: insert "the" before "server".
Page 5, 1st paragraph, 3rd sentence: delete "conjunction for".
Page 5, Section 3.3, 1st paragraph, 1st sentence: insert "the" before "Via"
and append "field" after "header".
Page 5, Section 3.3, 1st paragraph, 3rd sentence: append "field" after
"header".
Page 5, Section 3.3, 1st paragraph, last sentence: change "rate based" to
"rate-based".
Page 5, Section 3.4, 3rd paragraph, 1st sentence: replace "max" with
"maximum".
Page 6, 1st partial paragraph: replace "the" with "a".
Page 6, 1st full paragraph: replace "cpu" with "CPU" in both uses.
Page 6, 2nd paragraph: delete the comma.
Page 6, 4th paragraph: delete the first comma.
Page 6, 4th paragraph: append to the paragraph something like " and that
rate-based control is in effect". The server needs to indicate the rate (in
the oc parameter), but it must also inform the client which algorithm has
been selected.
Page 6, 5th paragraph: append "the" after "use".
Page 6, section 3.5.1: move the definition of "T" to this paragraph from the
following one. Or consider just saying "oc value" since that's what 1/T
(1/1/oc value) works out to.
Page 7, 1st partial paragraph: insert a space into "RFC5390". Or place it
in square brackets as a reference.
Page 7, 2nd full paragraph: replace "out of scope" with "out-of-scope".
Page 7, 4th paragraph: replace "Leaky Bucket" with "leaky bucket" and use
the lower case version throughout the draft.
Page 8, 2nd paragraph: there's a weird break in the paragraph that should be
fixed.
Page 8, 2nd paragraph: last sentence: replace "burstyness" with
"burstiness". This spelling seems to be more prevalent.
Page 9, 3rd paragraph: change "rate" to "rates".
Page 9, 4th paragraph: delete the commas. Change "is" to "are".
Page 9, No priority case: TargetRate is not defined. It should be noted
that it is the value of the oc parameter, as received from the server. ta
might be more clearly defined as "arrival time of the most recent SIP
request received by the client". The same applies to the prioritized case
further down.
Page 10, 1st paragraph, phrase after the colon: rewrite the first part as
"Requests that are candidates for reduction and requests not subject to
reduction"
Page 10, 2nd paragraph: replace the first "Leaky" with "leaky" in all uses
except when used in the term "Leaky Bucket algorithm".
Page 10, 2nd paragraph first sentence: replace "requests candidate" with
"request candidates".
Page 10, 2nd paragraph, 3rd bullet item: insert "at or" before "above".
Page 10, 3rd paragraph, 1st sentence: the use of "<=" contradicts the
requirement for the Leaky Bucket algorithm as given
Page 10, 4th paragraph: change "is" to "are".
Page 10, 5th paragraph: replace the "&" with "and".
Page 10, 6th paragraph: delete the commas and change "is" to "are".
Page 11, priority case, TAU1 definition: hyphenate "no priority".
Page 11, Section 3.5.3, second sentence: I understand what's trying to be
said, but the wording is difficult. Consider rewording.
Page 12, 1st partial paragraph: delete the first comma.
Page 12, 1st full paragraph, else clause: insert " let u be set to a random
value" before "uniformly".
Page 12, 4th paragraph, 4th bullet item: replace "At" with "As".
Page 12, Section 4: insert "the" before the reference.
Page 13, 1st full paragraph: append "field" after "header".
Page 13, 2nd paragraph, 1st sentence: replace both "---" with commas and
space appropriately. Append "field" after "header".
Page 13, 2nd paragraph, 2nd sentence: drop all of the quotation marks. They
aren't even used consistently in the sentence and don't add much. Change
"rate based" to "rate-based".
Page 13, 3rd paragraph, 2nd sentence: change "seconds" to "second".
Page 16, Appendix B: insert "and" before "James".
Page 16, Eric Noel's address: change "200s Laurel Avenue" to "200 S Laurel
Avenue". Change "Middletown, NJ, 07747" to "Middletown, NJ 07747". Both
changes are made to conform to the USPS standard for address formats.
Aren't you sorry you asked? ;-)
Page 16, Philip M Williams' name. See change requested on page 1.