Re: [Dime] Last Call: <draft-ietf-dime-agent-overload-08.txt> (Diameter Agent Overload and the Peer Overload Report) to Proposed Standard

[Stephen Farrell <[e-mail]>]

Hi all,

On 20/01/17 17:16, Steve Donovan wrote:
> Misha,
> 
> Thanks for the detailed review.
> 
> In the future this type of message only needs to go to the DIME mailing
> list.

Actually sending IETF last call comments to [e-mail]
is entirely valid - that's partly how we get review from
outside the WG. But sending comments to the WG list is
also ok.

> 
> I will deal with all of your comments in one email.

Great. I'll hold off moving this forward until we've gotten
those handled. I assume that a revised I-D will be needed.

Thanks,
S.


> 
> Regards,
> 
> Steve
> 
> On 1/17/17 12:23 PM, Misha Zaytsev wrote:
>> Hi All,
>>
>> Here are my comments/questions to an agent overload draft.
>> This the first part. Later on I will complete my review and send out
>> the second portion of the comments.
>>
>> 1. section 1 (editorial) removed "is" before "feasible".
>>
>> In the base specification, the goal is to handle abatement of the
>>     overload occurrence as close to the source of the Diameter traffic as
>>     feasible.
>>
>> "scenaios" -> "scenarios"
>>
>> The Peer overload report type is
>>     defined in a generic fashion so that it can also be used for other
>>     Diameter overload*scenarios*.
>>
>> 2. section 3.1.1 (editorial) replaced "were"-> "was"
>>
>> In both of these cases, the occurrence of overload in the single
>>     agent must by handled by the client in a similar fashion as if the
>>     client*was*  handling the overload of a directly connected server.
>>
>> 3. section 3.1.1 (question)
>>
>> An appropriate error response is sent back to the originator
>>     of the request.
>> Who sends "an appropriate" error response" in this case?
>>
>> 4. section 3.1.2 (editorial) changed "to"->"the"
>>
>> 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 and
>>     route all traffic through the new active connection.
>>
>> 5. section 3.1.3 (editorial)
>>
>> An example of this type of deployment include*s*  when there are Diameter
>>     agents between administrative domains.
>> 6. section 3.1.3
>>
>> There is no section 2.2. I guess section 3.1.2 was meant here, right?
>>
>> Handling of overload of one or both of agents a11 or a12 in this case
>>     is equivalent to that discussed in section 2.2.
>>
>> 7. section 3.2.1
>>
>> It is not clear which usage scenario is meant here.
>>
>>     It is envisioned that abatement algorithms will be defined that will
>>     support the option for Diameter Endpoints to send peer reports.  For
>>     instance, it is envisioned that one usage scenario for the rate
>>     algorithm, [I-D.ietf-dime-doic-rate-control], which is being worked
>>     on by the DIME working group as this document is being written, will
>>     involve abatement being done on a hop-by-hop basis.
>> 8. section 4
>> Why is throttling to be applied and not diversion (like in case of
>> redundant agents)?
>> In this scenario the reacting node should first handle the throttling
>> of the
>>     overloaded host or realm.
>> "LOSS" Is it a new type defined in the scope of this draft?
>> Note: The goal is to avoid traffic oscillations that might result
>>        from throttling of messages for both the HOST/REALM overload
>>        reports and the PEER overload reports.  This is especially a
>>        concern if*both reports are of type LOSS*.
>> 9. section 5.1.1
>> Probably it is better to describe OC_PEER_REPORT feature in section 5.1?
>> Otherwise, it is used as a well-known one while it is the first place
>> where it is mentioned.
>> Also I think it is better to add more specific in this draft related
>> to peer report handling:
>> - define Peer Report Reacting Node and Peer Report Reporting Node
>> terms explicitly and use them through the draft and especially
>> starting from section 5.1
>> - add "Peer Report" prefix to all the described procedures
>> Example: Capability Announcement -> Peer Report Capability Announcement
>> 10. section 5.1.1/general
>> "DiameterIdentity" and "Diameter identity"
>> My proposal is to use one term through the spec.
>> Under "DOIC node", an agent is meant here?
>>   When an agent relays a request that includes a SourceID AVP in the
>>     OC-Supported-Features AVP, a DOIC node that supports the
>>     OC_PEER_REPORT feature MUST remove the received SourceID AVP and
>>     replace it with a SourceID AVP containing its own Diameter identity.
>> My proposal is to use peer report reacting node here re-phrasing this
>> statement below in the following way:
>>   When relaying a request that includes a SourceID AVP in the
>>     OC-Supported-Features AVP, a peer report reacting node MUST remove
>> the received SourceID AVP and
>>     replace it with a SourceID AVP containing its own DiameterIdentity.
>> 11. section 5.1.2
>> added the missed "to"
>> changed "PEER_REPORT"-> "PEER"
>> Note: The transaction state is used when the DOIC node is acting
>>        as a peer-report reporting node and needs*to *send OC-OLR
>> reports of
>>        type*PEER *in answer messages.  The peer overload reports
>>        are only included in answer messages being sent to peers that
>>        support the OC_PEER_REPORT feature.
>> "Diameter ID" term is not clarified anywhere.
>> Re-phrased the appropriate statement a little bit, changed "Diameter
>> ID"->"value"
>> Also there are other places in the draft where "Diameter ID" term is
>> used.
>> The peer supports the OC_PEER_REPORT feature if the received request
>>     contains an OC-Supported-Features AVP with the OC-Feature-Vector with
>>     the OC_PEER_REPORT feature bit set and with a SourceID AVP with a
>>     value that matches the DiameterIdentity of the peer from which
>>     the request was received.
>> Agent is meant under "reporting node" here?
>> Should not SourceID AVP not just stripped from the relayed answer, but
>> replaced with the SourceID AVP containing the DiameterIdentity of the
>> agent supporting OC_PEER_REPORT feature?
>> When an agent relays an answer message, a reporting node that
>>     supports the OC_PEER_REPORT feature MUST strip any SourceID AVP from
>>     the OC-Supported-Features AVP.
>> Hard to follow what was wanted to say here:
>> Corrected the statement, but this is just my best guess.
>> The OC-Peer-Algo AVP MUST indicate the overload abatement
>>     algorithm that the reporting node wants the reacting nodes to use
>>     *when *the reporting node send*s*  a peer overload report as a
>> result of
>>     becoming overloaded.
>> Should not we add a separate if- statement for the case when the peer
>> does not support OC_PEER_REPORT feature when sending an answer message?
>> 12. section 5.1.1 and 5.1.2
>> Probably it is more helpful to illustrate OC_PEER_REPORT feature CA
>> using sequence diagrams like in the load info conveyance draft.
>> 13. general.
>> What about to use the writing for the same terms through the spec?
>> Example1: "DOIC node" and "DOIC Node"
>> Example2: "peer-report reporting node" and "peer report reporting node"
>> 14. section 5.2.1.2, 5.2.2, 5.2.3 and general
>> "peer-type OCS" and "peer report OCS" define the same term?
>> Why not to use only one?
>> Another example: "peer report" and "peer report-type" and "report of
>> type PEER"
>> 15. section 5.2.3
>> Probably it is better to re-phrase this statement a little bit +
>> corrected the misprints.
>> If a*peer report*  reacting node receives an OC-OLR AVP of type peer
>> and the
>> SourceID matches the*DiameterIdentity *of the peer from which the*report*
>> was received then the report was*generated *by the peer.
>> Similar comment + corrected misprints for the next statement:
>> If a peer report reacting node receives an OC-OLR AVP of type peer and
>> the
>> SourceID does not match the*DiameterIdentity *of the peer from which the
>> *report *was received then the reacting node MUST ignore the overload
>> report.
>> Also I think it is useful to use one wording for the same term:
>> "Peer Report OLR", "OC-OLR AVP", "OLR"
>> Let's use a generic one "peer report"?
>> Just minor comment: "the existing..." and "a new overload condition"
>> for all occurrences if my English is correct.
>> 16. section 5.2.3
>> How may it happen that peer report reacting node receives a peer
>> report not from the peer that generated it?
>> Peer reports can be sent only to peer report reacting node, right? And
>> peer reports are not relayed, right?
>> Best regards,
>> /Misha
>> 2017-01-09 17:35 GMT+03:00 The IESG <[e-mail]
>> <mailto:[e-mail]>>:
>>
>>     The IESG has received a request from the Diameter Maintenance and
>>     Extensions WG (dime) to consider the following document: -
>>     'Diameter Agent Overload and the Peer Overload Report'     
>> <draft-ietf-dime-agent-overload-08.txt> as Proposed Standard The
>>     IESG plans to make a decision in the next few weeks, and solicits
>>     final comments on this action. Please send substantive comments to
>>     the [e-mail] <mailto:[e-mail]> mailing lists by
>>     2017-01-23. Exceptionally, comments may be sent to [e-mail]
>>     <mailto:[e-mail]> instead. In either case, please retain the
>>     beginning of the Subject line to allow automated sorting. Abstract
>>        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. Requirements The file can be
>>     obtained via
>>     https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/
>>     <https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/>
>>     IESG discussion can be tracked via
>>    
>> https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/ballot/
>>    
>> <https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/ballot/>
>>     No IPR declarations have been submitted directly on this I-D. The
>>     document contains these normative downward references. See RFC
>>     3967 for additional information:       
>> draft-roach-dime-overload-ctrl: A Mechanism for Diameter Overload
>>     Control (None - ) Note that some of these references may already
>>     be listed in the acceptable Downref Registry.
>>     _______________________________________________ DiME mailing list
>>     [e-mail] <mailto:[e-mail]>
>>     https://www.ietf.org/mailman/listinfo/dime
>>     <https://www.ietf.org/mailman/listinfo/dime>
> 
> 
> 
> _______________________________________________
> DiME mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/dime
>