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

From: Steve Donovan <[email-masked]>
Date: Tue, 7 Mar 2017 08:45:19 -0600

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 <[email-masked] 
> <mailto:[email-masked]>>:
>
>     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 <[email-masked]
>     <mailto:[email-masked]>>:
>
>         I've attached the diff file for this version of the draft.
>
>         Regards,
>
>         Steve
>
>
>         On 2/7/17 10:16 AM, [email-masked]
>         <mailto:[email-masked]> 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
>             [email-masked] <mailto:[email-masked]>
>             https://www.ietf.org/mailman/listinfo/dime
>             <https://www.ietf.org/mailman/listinfo/dime>
>
>
>
>         _______________________________________________
>         DiME mailing list
>         [email-masked] <mailto:[email-masked]>
>         https://www.ietf.org/mailman/listinfo/dime
>         <https://www.ietf.org/mailman/listinfo/dime>
>
>
>

---

From: Misha Zaytsev <[email-masked]>
Date: Wed, 8 Mar 2017 23:07:57 +0300

Hi Steve,

I'm OK with all your answers, except one:

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.

[Misha]: You mean an overload report can be carried in a request? If so,
I'm a bit confused to hear that...
Anyhow, you have already corrected "request" to "response message" that
will be interpreted in the only possible way.

Thanks!

2017-03-07 17:45 GMT+03:00 Steve Donovan <[email-masked]>:

> Nothing is mentioned in relation to report type like in this section. So,
> I think my comment is still valid.
>
> Still thinking that it is more correct to change "request" to "report" in
> 2nd and 3rd paragraphs.
>
> 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
> 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.
>
>
> 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"
>
> 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.
>
>
> How may it happen that peer report reacting node receives a peer report
> not from the peer that generated it?
> 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
>
> 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 <[email-masked]>:
>
>> Hi Steve,
>>
>> It looks that some of my comments (which I did not send in the first 2
>> mail) were lost.
>> 24. section 6.1.1. "OC-Feature-Vector" -> "OC-Feature-Vector AVP" in the
>> header.
>>
>> Probably, it is the matter of preference, but I would still propose to
>> rename SourceID to Source-Identity.
>>
>> 28. section 6.4.
>>
>> 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 <[email-masked]>:
>>
>>> On 2/7/17 10:16 AM, [email-masked] 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
>>>>
>>>> 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.
>>>>
>>>> [email-masked]
>>> [email-masked]