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

[Misha Zaytsev <[e-mail]>]

Hi All,

Just several small comment from my side:

24. section 6.1.1. "OC-Feature-Vector" -> "OC-Feature-Vector AVP" in the
header.

25. section 6.1.2 "OC-Peer-Algo" -> "OC-Peer-Algo AVP" in the header

26. section 6.2 corrected AVP names

This extension makes no changes to the *OC-Sequence-Number* or
*OC-Validity-Duration* AVPs in the OC-OLR AVP.

27. section 6.3

Probably, it is the matter of preference, but I would still propose to
rename SourceID to Source-Identity.


Thanks in advance!

/Misha


2017-01-18 21:40 GMT+03:00 Misha Zaytsev <[e-mail]>:

> Hi All,
>
> As promised here is the second part of the comments (numbering is kept):
>
> 17. section 5.2.3, 5.2.4 (and further if applicable)
>
> Why not to use "active OCS" instead of "existing OCS" or "existing
> overload conditions"?
> Just to be inline with the RFC7683 this draft is extending...
>
> 18. section 5.2.4
>
> What is meant here? Especially "other overload reporting node behaviors".
> For me it says about all and about nothing at the same time. Please clarify
>
> The reporting agent must follow all other overload reporting node
>    behaviors outlined in the DOIC specification.
>
>
> 19. section 5.2.5
>
> Seems to be a little bit grammatically incorrect, "on" -> "to"
>
> If the request matches an active OCS then the reacting node MUST
>    apply abatement treatment *to *the request.
>
> Maybe "error response" sounds better than just an "error" in this case:
>
> ... agent MUST send an appropriate error response as defined in [RFC7683].
>
> Seems to be wrongly phrased:
>
>  ... then *abatement *associated with the overload *abatement *MUST be ended in a
>    controlled fashion.
>
>
> 20. section 6
>
> I do not know why but I do not like the following wording:
>
> ... used as part of the CER/CEA base Diameter capabilities exchange.
>
>
> Probably this version is better:
>
> ... used as part of the Diameter Capabilities Exchange procedure defined in [RFC6733].
>
>
> 21. section 6.1.1
>
> The peer report feature defines a new feature bit *that (?)*is added for the OC-Feature-Vector AVP.
>
>
> 22. section 6.2, similar to comment#16
>
> In the section 5.1.2 it says the following:
>
> When receiving a request a DOIC node that supports the OC_PEER_REPORT
>    feature MUST update transaction state with *an indication of whether
>    or not the peer from which the request was received supports the
>    OC_PEER_REPORT feature.*
>
>       Note: The transaction state is used when the DOIC node is acting
>       as a peer-report reporting node and needs send OC-OLR reports of
>       type PEER_REPORT in answer messages.  The peer overload reports
>       are only included in answer messages being sent to peers that
>       support the OC_PEER_REPORT feature.
>
> So, my gut feeling is that it means that peer report can't be sent thru a
> non-supporting agent???
> If I'm wrong, please clarify that.
>
> Also, just several updates in wording + corrected misprints.
>
> The overload report MUST also include the Diameter identity of the
>    agent that generated the report.  This is necessary to handle the
>    case where there is a non*-*supporting agent between the reporting node
>    and the reacting node.  Without the indication of the agent that
>    generated the overload *report*, the reacting node could erroneously
>    assume that the report applied to the non-supporting node.  This
>    could, in turn, result in unnecessary traffic being either
>    *diverged* or throttled.
>
>
> 23. section 6.3 "SourceID" -> "SourceID AVP" in the section header.
>
> Seems to be all from my side, but probably I have still missed something.
> Anyway, I'm ready to re-check the new version of the draft when available.
>
> Best regards,
>
> /Misha
>
> 2017-01-17 23:23 GMT+03:00 Misha Zaytsev <[e-mail]>:
>
>> 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]>:
>>
>>>
>>> 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] mailing lists by 2017-01-23. Exceptionally, comments may
>>> be
>>> sent to [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/
>>>
>>> IESG discussion can be tracked via
>>> 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]
>>> https://www.ietf.org/mailman/listinfo/dime
>>>
>>
>>
>