Point number 2- "As if the client were …" is correct (subjunctive for a "condition contrary to fact") "As if the client was" … is grammatically INCORRECT. Janet From: DiME [mailto:dime-bounces@xxxxxxxx]
On Behalf Of Misha Zaytsev 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 includes 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 sends 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 <iesg-secretary@xxxxxxxx>:
This electronic message transmission contains information from CSRA that may be attorney-client privileged, proprietary or confidential. The information in this message is intended only for use by the individual(s) to whom it is addressed. If you believe you have received this message in error, please contact me immediately and be aware that any use, disclosure, copying or distribution of the contents of this message is strictly prohibited. NOTE: Regardless of content, this email shall not operate to bind CSRA to any order or other contract unless pursuant to explicit written agreement or government initiative expressly permitting the use of email for such purpose. |