Re: [Dime] Review of draft-ietf-dime-agent-overload-08

[Steve Donovan <[e-mail]>]

Shucheng,

Thank you for your review.  See my comments inline.

Regards,

Steve

On 3/16/17 12:12 AM, Shucheng LIU wrote:
> Reviewer: Shucheng LIU
> Review result: Has Nits
>
> Hi all,
>
> (Sorry for being late that I reviewed this draft with a printed one on
> my flight to home last month, however, I left paper there…)
> I have reviewed draft-ietf-dime-agent-overload-08 as part of the
> Operational directorate's ongoing effort to review all IETF documents
> being processed by the IESG.  These comments were written with the
> intent of improving the operational aspects of the IETF drafts.
> Comments that are not addressed in last call may be included in AD
> reviews during the IESG review.  Document editors and WG chairs should
> treat these comments just like any other last call comments.
>
> “This specification documents an extension to RFC 7683 (Diameter
>     Overload Indication Conveyance (DOIC)) base solution.  The
> extension
>     defines the Peer overload report type.  The initial use case for
> the
>     Peer report is the handling of occurrences of overload of a
> Diameter
>     agent.”
>
> My overall view of the document is 'Ready with nits' for publication.
>
>
> ** Technical **
> No.
>
>
> ** Editorial **
>
> *Section 2, page 4
>>     A RFC6733 Diameter Client, an RFC6733 Diameter Server, and
> RFC6733
>>        Diameter Agent.
> s/ RFC6733/ [RFC6733]
> Similar changes should also be made in this section to get consistent
> with section 1 and the last sentence in section 2(therein you were
> using [RFC6733]).
Change made.
>
>
> * Section 3.1.1, page 4:
>
>>                               +-+    +-+    +-+
>>                                |c|----|a|----|s|
>>                               +-+    +-+    +-+
> Though I can easily guess what does “c, a, s” mean here, I still
> suggest to put full words or at least add a sentence below the figure
> to explain.
> The same issue should be fixed in all the figures below in entire
> section 3.
I've added the following in section 3.

    In the figures in this section, elements labeled "c" are Diameter 
Clients,
    elements labeled "a" are Diameter Agents and elements labeled "s" are
    Diameter Servers.
>
>   
> * Section 3.1.2, page 6:
>
>>    In the case where one of the agents in the above scenario becomes
>>    overloaded
> s/ scenario becomes/ scenarios become
>
> If I understand correctly , here you were referring to two scenarios
> above?
Change made.
>
>>    When the client has an active and a standby connection to the two
>>    agents then an alternative strategy for responding to an overload
>>    report from an agent is to change to standby connection to active
> and
>>    route all traffic through the new active connection.
> I would suggest to split this sentence in case of misunderstanding.
Changed to:

When the client has an active and a standby connection to the two agents 
then
             an alternative strategy for responding to an overload 
report from an agent is
             to change the standby connection to active.  This will 
result in all traffic
             being routed through the new
             active connection.
>
> * Section 3.1.3, page 7:
>
>>   Handling of overload of one or both of agents a11 or a12 in this
> case
>>    is equivalent to that discussed in section 2.2.
> Tried hard to find section 2.2, but there is no such section.
This was fixed in the last version of the draft.
>
>
> * Section 5.1.1, page 8:
>
>>    When sending a Diameter request a DOIC node that supports the
>>     OC_PEER_REPORT feature MUST include in the OC-Supported-Features
> AVP
>>     an OC-Feature-Vector AVP with the OC_PEER_REPORT bit set.
> Full name of AVP should be put into terminology.
Change made.
>
>
> _______________________________________________
> DiME mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/dime