Re: [rmcat] I-D Action: draft-ietf-rmcat-coupled-cc-05.txt

[Colin Perkins <[e-mail]>]

Looks good, thanks! (although maybe call out explicitly that the priority has to be greater than zero?)

Cheers,
Colin




> On 23 Mar 2017, at 13:47, Michael Welzl <[e-mail]> wrote:
> 
> Dear Colin,
> 
> Thanks a lot, this was very helpful !    We incorporated your comments and attach the updated draft to this email, as right now submission is closed.
> Some answers in line:
> 
> 
>> On 23 Mar 2017, at 00:33, Colin Perkins <[e-mail]> wrote:
>> 
>> Authors,
>> 
>> I’ve reviewed this draft while preparing the IESG write-up and request for publication. I have a number of minor questions and comments:
>> 
>> - Section 2 defines a Flow and gives an example as “an RTP session, or a sub-session that is multiplexed onto a single RTP session”. I believe this example is incorrect, and what is intended is “an RTP Stream [RFC7656], or a group of RTP Streams multiplexed onto a single RTP session” (Section 2.1.10 of RFC 7656 defines the term RTP Stream carefully, so I think a reference is useful; it also defines the term RTP Session in Section 2.2.2). Can you please check and update are necessary.
> 
> Thanks a lot - of course you're right! We changed the definition of a flow as follows:
> "A flow is the entity that congestion control is operating on. It could, for example, be a transport layer connection, or an RTP  stream <xref target="RFC7656"/>, whether or not this RTP stream is multiplexed onto an RTP session with other RTP streams."
> 
> 
>> - Section 3 discusses limitations of the proposed algorithms. Would it be appropriate to add a brief applicability statement either here, or in the Introduction, stating that the described mechanisms are believed safe to use, but are experimental and are presented for wider review and operational evaluation?
> 
> OK, done, at the end of the introduction.
> 
> 
>> - Section 5.1, bullet 1, notes that flows share a bottleneck if they’re sent on the same 5-tuple and have the same DSCP value. Should this also note that they need the same ECT mark? With the proposed L4S ECN experiments, we’re likely to see quite different behaviour between flows with NotECT/ECT(0) and those with ECT(1) marking. 
> 
> Good catch! Done
> 
> 
>> - Section 5.2 defines the priority in the range 0.1 to 1. Assuming 0.1 is correct, it might be worth adding a brief note to explain this choice, since readers might think it a typo for the more typical range of 0 to 1. 
>> 
>> - Section 5.2, in the paragraph following the bullet points, the draft suggests mapping priority levels from RTCWEB using the values 1, 2, 4, and 8 – how do these relate to the 0.1 to 1 range?
> 
> Another good catch! The value range really doesn't matter for our algorithm, but 0 is not allowed (it might lead to a division by 0).
> 
> To clarify all of this, we have removed the "0.1 to 1" example in the priority definition, and changed the text talking about 1, 2, 4 and 8 levels to the following:
> 
> ***
>   Note that the absolute range of priorities does not matter: the
>   algorithm works with a flow's priority portion of the sum of all
>   priority values.  For example, if there are two flows, flow 1 with
>   priority 1 and flow 2 with priority 2, the sum of the priorities is
>   3.  Then, flow 1 will be assigned 1/3 of the aggregate sending rate
>   and flow 2 will be assigned 2/3 of the aggregate sending rate.
>   Priorities can be mapped to the "very-low", "low", "medium" or "high"
>   priority levels described in [I-D.ietf-rtcweb-transports] by simply
>   using the values 1, 2, 4 and 8, respectively.
> ***
> 
> 
>> - Section 5.3 defines terminology and describes the two algorithms. The terminology used is inconsistent, sometimes using FSE_R and sometimes FSE_R(f). If the intent is that these are the same variable, the terminology should be aligned (I assume to FSE_R(f) since they’re per-flow values, but the important point is that it’s consistent). If there’s intended to be a difference between the overall and per-flow values, can you please check that they’re used consistently, and add a sentence to clarify the meanings of the terms. 
>> 
>> - Section 5.3 also uses P and P(i) in a manner that suggests they’re referring to the same parameter. Similarly for CC_R – should it be CC_R(i)?
> 
> Fixed everywhere.
> 
> 
>> - Appendix C: similar terminology comments apply.
> 
> Fixed everywhere (here it also applies to the variables DR, new_DR and Rate).
> 
> 
>> - Appendix C: “highly experimental” – It might be useful to add a brief applicability statement for implementors so they know whether this is safe to deploy outside of testbed environments.
> 
> done  (we added "and not safe to deploy outside of testbed environments" after "highly experimental").
> 
> Cheers,
> Michael
> 
> <draft-ietf-rmcat-coupled-cc-06.txt>
> 



-- 
Colin Perkins
https://csperkins.org/