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 Steve,

OK, thanks for the note! Next time I will use only DIME mailing list.
Let's hope no one suffered much from my "spam" :)

Will be waiting for your answers.
And as I said in one of my mails, I'm ready to re-check the draft when new
version is available.

/Misha



2017-01-20 20:16 GMT+03:00 Steve Donovan <[e-mail]>:

> Misha,
>
> Thanks for the detailed review.
>
> In the future this type of message only needs to go to the DIME mailing
> list.
>
> I will deal with all of your comments in one email.
>
> 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]>:
>>
>> 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/d
>> oc/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/l
>> istinfo/dime
>
>