Re: [Dime] I-D Action: draft-ietf-dime-agent-overload-09.txt

[Steve Donovan <[e-mail]>]

Misha,

See my comments inline.

Steve

On 2/22/17 12:56 PM, Misha Zaytsev wrote:
> Hi Steve,
>
> Let me also reply to your edits when you were applying my comments. I 
> still have some concerns.
> I will use the old numbering of the comments from my previous mails.
>
> 8. section 4
> LOSS" Is it a new type defined in the scope of this draft?
>
> SRD> Loss is defined in the DOIC specification (RFC 7683)
>
> [Misha]: All mentions about "LOSS" in the RFC7683 are related to loss 
> algorithm.
> Nothing is mentioned in relation to report type like in this section. 
> So, I think my comment is still valid.
>
SRD> I agree the wording is bad.  I've changed it to teh following -- 
"This is especially a concern if both
         reports indicate the LOSS abatement algorithm."
> 15. section 5.2.3
> Still thinking that it is more correct to change "request" to "report" 
> in 2nd and 3rd paragraphs.
>
> If a reacting node receives an OC-OLR AVP of type peer and the
>    SourceID matches the DiameterIdentity of the Diameter peer from which
>    the *report *was received then the report was generated by a Diameter
>    peer.
>
>    If a reacting node receives an OC-OLR AVP of type peer and the
>    SourceID does not match the DiameterIdentity of the Diameter peer
>    from which the *report *was received then the reacting node MUST
>    ignore the overload report.
>
> This will be aligned with 1st paragraph in this case. If you still do 
> no agree, then please explain what request is meant here since the 1st 
> paragraph is saying about "report".
SRD> A report is carried in a request.  The DiameterIdentity contained 
in the SourceID is compared to the DiameterIdentity of the entity that 
sent the message that carried the report.  I have changed "request" to 
"response message" to be correct here.
>
> Could you check this particular section?
> I think you can easily find the places where these 3 different terms 
> are used in the section text: "Peer Report OLR", "OC-OLR AVP", "OLR"
>
> Here is just one example:
>
> If the *Peer Report OLR *was received from a Diameter peer then the
> reacting node MUST determine if it is for an existing or new overload
> condition.
>
> My proposal was to use the only term "peer report" for all such 
> occurrences.
> The reason behind is to avoid multiplying the usage of the different 
> terms that are about the same entity and just use the only one.
SRD> Done.
>
> 16. section 5.2.3 + 22. section 6.2
> 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?
> SRD> This is to handle cases where there are agents in the network 
> that do not support the peer report feature. These agents would relay 
> peer reports, as they would any other AVP they don't understand.
>
> SRD> It is correct that a DOIC node should *not (?)* send a peer 
> report to a non supporting node.  We, however, are paranoid, and left 
> the wording in the specification to handle miss behaving DOIC nodes, 
> given that bad things can happen when erroneous or malicious overload 
> reports are inserted into the network.
>
> [Misha]: If peer report cannot/should not be sent thru an agent that 
> does not support peer report feature and the only reason to leave such 
> wording in the spec is protect against potential malicious reports 
> inserted somehow in the network, then I'm proposing to explicitly 
> state such things in the spec. Otherwise, this wording may be 
> misleading/contradicting (with other parts of the spec) when reading 
> the spec
SRD> I've added the following note to clarify -- "Note: Under normal 
circumstances, a Diameter node will not add a peer report when sending 
to a peer that does not support this extension.  This requirement is to 
handle the case where peer reports are erroneously or maliciously 
inserted into response messages.
> Best regards,
>
> /Misha
>
> 2017-02-07 21:10 GMT+03:00 Misha Zaytsev <[e-mail] 
> <mailto:[e-mail]>>:
>
>     Hi Steve,
>
>     It looks that some of my comments (which I did not send in the
>     first 2 mail) were lost.
>     So, I'm putting them here:
>
>     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.
>
>     28. section 6.4.
>
>     6.4. Attribute Value Pair *F*lag *R*ules
>
>     29. section 7.1
>
>     7.1. AVP *C*odes
>
>
>     30. section 7.2
>
>     7.2. New *R*egistries
>
>
>     Also I will check your answers on my applied comments in detail
>     and come back to you if I still have any concerns.
>
>     /Misha
>
>
>     2017-02-07 19:19 GMT+03:00 Steve Donovan <[e-mail]
>     <mailto:[e-mail]>>:
>
>         I've attached the diff file for this version of the draft.
>
>         Regards,
>
>         Steve
>
>
>         On 2/7/17 10:16 AM, [e-mail]
>         <mailto:[e-mail]> wrote:
>
>             A New Internet-Draft is available from the on-line
>             Internet-Drafts directories.
>             This draft is a work item of the Diameter Maintenance and
>             Extensions of the IETF.
>
>                      Title           : Diameter Agent Overload and the
>             Peer Overload Report
>                      Author          : Steve Donovan
>                     Filename        :
>             draft-ietf-dime-agent-overload-09.txt
>                     Pages           : 17
>                     Date            : 2017-02-07
>
>             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 IETF datatracker status page for this draft is:
>             https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/
>             <https://datatracker.ietf.org/doc/draft-ietf-dime-agent-overload/>
>
>             There's also a htmlized version available at:
>             https://tools.ietf.org/html/draft-ietf-dime-agent-overload-09
>             <https://tools.ietf.org/html/draft-ietf-dime-agent-overload-09>
>
>             A diff from the previous version is available at:
>             https://www.ietf.org/rfcdiff?url2=draft-ietf-dime-agent-overload-09
>             <https://www.ietf.org/rfcdiff?url2=draft-ietf-dime-agent-overload-09>
>
>
>             Please note that it may take a couple of minutes from the
>             time of submission
>             until the htmlized version and diff are available at
>             tools.ietf.org <http://tools.ietf.org>.
>
>             Internet-Drafts are also available by anonymous FTP at:
>             ftp://ftp.ietf.org/internet-drafts/
>             <ftp://ftp.ietf.org/internet-drafts/>
>
>             _______________________________________________
>             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] <mailto:[e-mail]>
>         https://www.ietf.org/mailman/listinfo/dime
>         <https://www.ietf.org/mailman/listinfo/dime>
>
>
>