Re: [Dime] AD evaluation of draft-ietf-dime-load-06

[Steve Donovan <[e-mail]>]

Misha,

Thank you for the very detailed review of the draft.  See my comments 
inline.

Regards,

Steve

On 1/15/17 2:00 AM, Misha Zaytsev wrote:
> Hi Steve,
>
> Sure, you can treat it as you wish since it is just a minor comment 
> from my side.
> However, I have other comments/questions to address:
>
> 0. general (editorial) Proposals regarding wording in the spec:
> - Use either "fulfil" or "fulfill" even if both forms are possible
Agreed, I'll update in the next release.
> - Use "remove"/"discard" instead of "strip" since it seems to be more 
> formal
This is a matter of preference and I prefer strip. :-)
>
> 1. section 4.1 (editorial)
> -  "did not had" -> "did not have"
> - "reacting node reduce" -> "reacting node reduce*s*"
Agreed, I'll update accordingly.
>
> 2. general (question): Why is different writing used for the same 
> terms through the spec?
> Example#1, "Load report" and "load report"
> Example#2, "report of type peer" and "load report of PEER"
> Example#3, "Diameter agent" and "Diameter Agent"
> Is this intentional? If yes, please clarify the intention.
This is a good point.  I'll update accordingly.
>
> 3. section 5 (editorial)
>
> The load report includes a value indicating the load of the sending
>     node relative load of the sending node, specified in a manner
>     consistent with that defined for DNS SRV [RFC2782].
> Should not be re-phrased a little bit in this way  - "...a value 
> indicating the relative load of the sending node, specified..."?
>
Yes, that is more clear.
> 4. section 5 (editorial): "weigh" -> "weigh*t*"
>
> The distribution algorithm used by Diameter nodes supporting the
>     Diameter Load mechanism is an implementation decision but it needs to
>     result in similar behavior to the algorithm described for the use of
>     weight values specified in [RFC2782].
>
Agreed
> 5. section 5 (question)
>
> The second type of load report is a peer report.  This report is used
>     by Diameter nodes as part of the logic to select the next-hop
>     Diameter node and, as such, does not have significance beyond the
>     peer node.  These load reports are removed by the first supporting
>     Diameter node to receive the report.
> Under "These load reports are removed..." the report of type PEER is 
> meant, right?
> Probably it is better to re-phrase this statement saying more explicitly:
> "The report of type PEER is removed by the first Diameter node 
> supporting Load mechanism."
The paragraph is talking about peer reports so I don't think there is 
any ambiguity.  However, I will update the last sentence to be clear it 
is referring to PEER reports.
>
> 6. section 6.1.2 (editorial) "insures" -> "*e*nsures"
>
> Note: In the case of peer load reports it is only necessary to
>        include load reports when the load value has changed by some
>        meaningful value, as long as the agent*e*nsures that all peers
>        receive the report.  It is also acceptable to include the load
>        report in every answer message handled by the Diameter agent.
>
Agreed
> 7. section 6.2 (editorial) "weigth" -> "weight"
>
> How a Diameter node uses load information for making routing
>     decisions is an implementation decision.  However, the distribution
>     algorithm MUST result in similar behavior as the algorithm described
>     for the use of weig*ht*  values in [RFC2782].
>
Agreed
> 8. section 6.4 (editorial) "Addition and removal of Nodes" -> 
> "Addition and Removal of Nodes"
Agreed
>
> 9. general. I'm voting for that the term "server selection" would be 
> defined explicitly in the spec.
> You answered on the comment from Stephen that this term came from RFC7683.
> But I have found only one place where it is mentioned: Appendix B, 
> "Non-supporting Agents".
I'll add a definition to the next version.
>
> Thanks a lot in advance.
>
> /Misha
>
> 2017-01-13 0:06 GMT+03:00 Steve Donovan <[e-mail] 
> <mailto:[e-mail]>>:
>
>     Misha,
>
>     This draft has completed working group last call, so to some
>     degree the comment period has passed.  That said, there are no
>     hard and fast rules and we can certainly make changes that improve
>     the draft.  However, once it gets into IESG review, which is the
>     next step, it will be more difficult to make significant changes.
>
>     On your comment below, I don't see the need for the change.  I am
>     willing, however, to qualify the word connection to say "Diameter
>     connection".
>
>     Regards,
>
>     Steve
>
>     On 1/12/17 11:51 AM, Misha Zaytsev wrote:
>>     Hi Steve,
>>
>>     Thanks a lot for your explanation!
>>     My bad that I have not read the draft more carefully - will try
>>     to avoid this next time.
>>
>>     Then I can propose another version of statement that will exclude
>>     confusing "connection" term.
>>
>>>     This is achieved by comparing SourceID AVP in the Load report with host identity field of the entries from peer table defined in RFC6733.
>>
>>     If SourceID AVP contains the identity of one of the peer nodes
>>     (that our node has a direct connection to), then this is what we
>>     are looking for, right?
>>
>>     However, I find the term "connection" legal as it is defined in
>>     RFC6733, for instance section 5.1 "Peer Connections".
>>
>>     Or it can be easily re-phrased in this way, the term "peer"
>>     should not confuse anyone, I guess :)
>>
>>     This is achieved by comparing the DiameterIdentity of the peer from which the message was received with the
>>     DiameterIdentity included in the SourceID AVP in the Load report.
>>
>>     Btw, what is the due date for providing the comments for this draft?
>>
>>     Best regards,
>>
>>     /Misha
>>
>>     2017-01-12 20:16 GMT+03:00 Steve Donovan
>>     <[e-mail] <mailto:[e-mail]>>:
>>
>>         Misha,
>>
>>         I've added the DIME WG mailing list.
>>
>>         See my comments inline.
>>
>>         Regars,
>>
>>         Steve
>>
>>         On 1/11/17 12:03 PM, Misha Zaytsev wrote:
>>>         Hi Steve,
>>>
>>>         I'm a newcomer in DiME working group - so, may not be
>>>         familiar with all rules.
>>>         So, excuse me in advance if it is not a good manner to
>>>         comment the ongoing draft review.
>>         SRD> Welcome to the working group!
>>>
>>>         But let me still put my 5 cents here.
>>>
>>>         Regarding this question from Stephen:
>>>
>>>         - 6.2: What is a Diameter "connection?" I thought that
>>>         Diameter could use UDP as well as TCP so is that really
>>>         the right term? Maybe "message sender" is a better way to
>>>         identify the peer?
>>>
>>>         Why can't we re-phrase the related statement in the
>>>         following way?
>>>
>>>         This is achieved by comparing Origin-Host AVP with SourceID AVP in the Load report.
>>>
>>         SRD> This actually doesn't work for the peer report case. 
>>         The Origin-Host AVP is inserted by the Diameter client and it
>>         remains the same as the request passes through Diameter
>>         agents.  For peer reports, it is necessary to know the peer
>>         that sent the request, not the client.  Thus the need to look
>>         at the DiameterID associated with the Diameter connection.
>>
>>>         Also if it does not bother you, could you explain what is
>>>         the official way to comment DiME drafts?
>>         SRD> The official way is to do exactly what you are doing,
>>         but by sending the questions and comments to the DIME WG
>>         mailing list.
>>
>>>
>>>         Thanks a lot in advance!
>>>
>>>         /Misha
>>>
>>>
>>>
>>>         2017-01-10 1:28 GMT+03:00 Steve Donovan
>>>         <[e-mail] <mailto:[e-mail]>>:
>>>
>>>             Stephen,
>>>
>>>             Thanks for the review and for the ping.  Comments below.
>>>
>>>             Regards,
>>>
>>>             Steve
>>>
>>>             On 1/6/17 10:33 AM, Stephen Farrell wrote:
>>>>             Just bumping this, post holidays. I believe the
>>>>             ball is not in my court for this one:-)
>>>>
>>>>             Cheers,
>>>>             S.
>>>>
>>>>             On 16/12/16 17:38, Stephen Farrell wrote:
>>>>>             Hiya,
>>>>>
>>>>>             Thanks for getting this stuff progressed. I've done my
>>>>>             AD evaluation and have a few questions I'd like to ask
>>>>>             before starting IETF last call. Those are below along
>>>>>             with some more nitty comments that can be handled now or
>>>>>             later as the authors/WG prefer.
>>>>>
>>>>>             Cheers,
>>>>>             S.
>>>>>
>>>>>             Things to chat about before starting IETF LC:
>>>>>             ---------------------------------------------
>>>>>
>>>>>             (1) Is "server selection" sufficiently clear? Where is
>>>>>             that defined? I was a bit confused as to what this means
>>>>>             that is not next-hop selection.
>>>             SRD> Server selection is touched on in RFC7638 (DOIC)
>>>             and the concept carries over to Load.  It refers to
>>>             selection of the specific server instance that will be
>>>             handling the request.  This is according to the Diameter
>>>             Client, Server model.  I think it is well understood
>>>             what is meant by those who understand Diameter and would
>>>             be implementing this spec.  We can, if needed, add some
>>>             definition. That would have been best do be in the DOIC
>>>             RFC but it can go here if needed.
>>>>>             (2) PEER reports that are first received at a
>>>>>             non-supporting node will be left in place and will reach
>>>>>             the destination of the message. If that destination is in a
>>>>>             different domain then that leaks some internal structure
>>>>>             (the SourceID) to outsiders. Is that desirable?  Why not
>>>>>             have the first node that does support this AVP delete the
>>>>>             PEER report even if the node that added the PEER report is
>>>>>             not a peer of this node? (Note: I see this risk is ack'd
>>>>>             in section 8, I'm asking if we can avoid it almost
>>>>>             entirely by removing PEER reports that are useless.)
>>>             SRD> There is no formal mechanism in place in Diameter
>>>             to do "topology hiding".  There are many other places
>>>             where topology information can leak, so it isn't an
>>>             issue specific to Load.  It is addressed today through
>>>             proprietary implementations. SRD> Having a node that
>>>             does not support would go against the Diameter
>>>             extensibility strategy.  Nodes that don't understand an
>>>             AVP are required to pass it on.  Nodes that do support
>>>             the mechanism and see a Load report of type peer that
>>>             isn't of type peer are supposed to remove it.  Doing
>>>             anything other than this would require a change to the
>>>             base Diameter specification.
>>>>>             (3) This spec is a bit like RFC7944 (DRMP) in that it
>>>>>             defines some but not all of the things one needs to end up
>>>>>             with a workable system. That aspect of DRMP caused some
>>>>>             discussion during IESG evaluation. Have the authors of
>>>>>             this reviewed that discussion to see if we can avoid any
>>>>>             likely iterations being needed at that point? I'm hoping
>>>>>             that Steve, as an author of both, won't find this too
>>>>>             hard to do:-) If that's been done, great. If not, please
>>>>>             consider if there's any additional explanatory material
>>>>>             that could be added that might help us not to have to
>>>>>             iterate to discuss the same kinds of concern.
>>>             SRD> I'll go back and review that discussion and see if
>>>             there is something that needs to be added.  I'm hoping
>>>             that the fact that we made it through the discussion
>>>             with DRMP will make it easier to do so with Load (and
>>>             maybe agent overload).  I'm doubtful that we can fully
>>>             inoculate the draft from some of this level of
>>>             discussion as we are dealing with Diameter here.
>>>>>             nits (fine to be considered last call comments):
>>>>>             -------------------------------------------------
>>>             SRD> I'll deal with these as part of last call.
>>>>>             abstract: maybe put the 1st sentence last? that might read
>>>>>             better
>>>>>
>>>>>             4.1: the "opinion of the authors" isn't really of interest
>>>>>             at this point - is this also the opinion of the WG? (I
>>>>>             assume it is)
>>>>>
>>>>>             section 5 says "The load report includes a value
>>>>>             indicating the load of the sending node relative load of
>>>>>             the sending node, specified in a manner consistent with
>>>>>             that defined for DNS SRV [RFC2782]." I can't parse that.
>>>>>
>>>>>             - 6.2: What is a Diameter "connection?" I thought that
>>>>>             Diameter could use UDP as well as TCP so is that really
>>>>>             the right term? Maybe "message sender" is a better way to
>>>>>             identify the peer?
>>>>>
>>>>>             - section 8: "might require a transitive trust model" is
>>>>>             far too coy IMO. I think you should say that DOIC and this
>>>>>             entirely require transitive trust because we have no
>>>>>             Diameter mechanism that allows authenticated adding and
>>>>>             removal of AVPs as messages transit a network. (We did try
>>>>>             develop that ages ago but it was too complex, so I'm not
>>>>>             arguing to try again, just that we clearly ack the fact
>>>>>             that this stuff requires transitive trust.)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>             _______________________________________________
>>>>>             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>
>>>             _______________________________________________ DiME
>>>             mailing list [e-mail] <mailto:[e-mail]>
>>>             https://www.ietf.org/mailman/listinfo/dime
>>>             <https://www.ietf.org/mailman/listinfo/dime> 
>>>