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

From: [email-masked]
Date: Tue, 07 Feb 2017 08:16:45 -0800

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/

There's also a htmlized version available at:
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

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.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: Steve Donovan <[email-masked]>
Date: Tue, 7 Feb 2017 10:19:24 -0600

I've attached the diff file for this version of the draft.

Regards,

Steve

On 2/7/17 10:16 AM, [email-masked] wrote:
> _______________________________________________
> DiME mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dime

---

From: Misha Zaytsev <[email-masked]>
Date: Tue, 7 Feb 2017 21:10:01 +0300

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

---

From: Misha Zaytsev <[email-masked]>
Date: Wed, 22 Feb 2017 21:56:26 +0300

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.

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".

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.

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.

Best regards,

/Misha