Re: [Dime] WGLC #1 draft-ietf-dime-doic-rate-control-03

Jean,

Again, thanks for the detailed review.

See my comments inline.

Regards,

Steve

On 6/8/16 4:04 PM, A. Jean Mahoney wrote:
> Hi Steve,
>
> Here's my feedback. I took a look at the errata (none) and Doc 
> Shepherd write-up for RFC 7415 to evaluate the rate control algorithm. 
> The write-up for RFC 7415 says that it has been incorporated into 
> several simulators, so I think that it should be ok here, but I did 
> not implement/test it myself.
SRD> Yes, we are riding on the coattails of the SIP work in this area. :-)
>
> Minor Issues:
>
> Section 5.1 para 5. The following sentence isn't clear to me:
>
>    A reporting node that supports the rate abatement algorithm MUST
>    include the specified rate in the abatement algorithm specific
>    portion of the reporting node rate OCS when sending a rate OLR.
>
> Perhaps update it to the following:
>
>    A reporting node that supports the rate abatement algorithm MUST
>    include the rate of its abatement algorithm in the OC-Maximum-Rate
>    AVP when sending a rate OLR.
>
SRD> Okay, change made.
>
> Section 5.4 para 1. Current:
>
>    When receiving an answer message indicating that the reacting node
>    has selected the rate algorithm, a reaction node MUST indicate the
>    rate abatement algorithm in the reacting node OCS entry for the
>    reporting node.
>
> Suggested:
>
>    When receiving an answer message indicating that the *reporting* node
>    has selected the rate algorithm, a *reacting* node MUST indicate the
>    rate abatement algorithm in the reacting node OCS entry for the
>    reporting node.
SRD> Good catch.  Change made.
>
>
> Section 6.2. The CCF for the OC-OLR AVP shows an 
> OC-Abatement-Algorithm AVP, which is not defined or used anywhere.  
> The CCF also has OC-Source-ID, which should be SourceID.
SRD> Again, good catch.  OC-Abatement-Algorithm has been removed.  
SourceID changes have also been made.
>
>
> Section 6.2.1 says that OC-Maximum-Rate is type Unsigned32 but Section 
> 6.3 says that it's Unsigned64.
SRD> I've made it Unsigned32
>
>
> Section 8, IANA Considerations, needs to be filled in.
SRD> Oops.  Done as follows:

8.  IANA Consideration

8.1.  AVP codes

    New AVPs defined by this specification are listed in Section 6. All
    AVP codes are allocated from the 'Authentication, Authorization, and
    Accounting (AAA) Parameters' AVP Codes registry.

8.2.  New registries

    There are no new IANA registries introduced by this document.
>
>
>
> Nits:
>
> Section 1 para 2.    s/increase quickly/increases quickly
>
> Section 1 para 6.    s/rate based request/rate-based request
>
> Section 1 para 8.    s/RIA/RAI or just remove it since the area
>                      has been renamed
SRD> Removed
>
> Section 4 para 5.    s/OC-Selected-Features/OC-Supported-Features
>
> Section 5.1 para 1.  Expand the first use of OCS and OLR.
>
> Section 5.1 para 2.  s/define/defined
>
> Section 6.3.         s/x.x/6.2
>
> Section 7.2 para 4.  s/cpu/CPU (2 instances)
>
> Section 7.2 para 7.  s/[draft-ietf-dime-ovli]/[RFC7683]
>
> Section 9 para 1.    s/based/base
>
> Section 11.2.        add the [Erramilli] reference
SRD> The above changes have been made.
>
>
> Thanks!
>
> Jean
>
>
> On 5/25/16 12:43 PM, Jouni Korhonen wrote:
>> Folks,
>>
>> This email starts the WGLC #1 for draft-ietf-dime-doic-rate-control-03.
>> Please, review the document, post your comments to the mailing list and
>> also insert them into the Issue Tracker with your proposed resolution.
>>
>> WGLC starts: 5/25/2016
>>        ends: 6/8/2016 EOB PDT
>>
>> - Jouni & Lionel
>>
>> _______________________________________________
>> DiME mailing list
>> DiME@ietf.org
>> https://www.ietf.org/mailman/listinfo/dime