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

From: "A. Jean Mahoney" <[email-masked]>
Date: Wed, 8 Jun 2016 16:04:26 -0500

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.

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.

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.

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.

Section 6.2.1 says that OC-Maximum-Rate is type Unsigned32 but Section 
6.3 says that it's Unsigned64.

Section 8, IANA Considerations, needs to be filled in.

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

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

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
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dime

---

From: Steve Donovan <[email-masked]>
Date: Thu, 9 Jun 2016 21:05:51 -0500

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. :-)
>    AVP when sending a rate OLR.
SRD> Okay, change made.
>    rate abatement algorithm in the reacting node OCS entry for the
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.
> 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.
> Section 1 para 8.    s/RIA/RAI or just remove it since the area
SRD> Removed
> Section 11.2.        add the [Erramilli] reference
SRD> The above changes have been made.

---

From: "A. Jean Mahoney" <[email-masked]>
Date: Fri, 10 Jun 2016 10:24:13 -0500

Hi Steve,

Thanks for making the changes.

Jean

---

From: Steve Donovan <[email-masked]>
Date: Thu, 16 Jun 2016 16:04:15 -0500

Jouni,
Lionel,

I have made changes based on Jean's review.  The WGLC period has expired 
on this.  Should I submit the new document or should I wait to see if 
there are other reviews pending?

Regards,

Steve

---

From: Jouni Korhonen <[email-masked]>
Date: Fri, 17 Jun 2016 16:25:22 -0700

Just upload a new revision.

- Jouni