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

[Misha Zaytsev <[e-mail]>]

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
- Use "remove"/"discard" instead of "strip" since it seems to be more formal

1. section 4.1 (editorial)
-  "did not had" -> "did not have"
- "reacting node reduce" -> "reacting node reduce*s*"

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.

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

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


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

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.


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


8. section 6.4 (editorial) "Addition and removal of Nodes" -> "Addition and
Removal of Nodes"

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

Thanks a lot in advance.

/Misha

2017-01-13 0:06 GMT+03:00 Steve Donovan <[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]>:
>
>> 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]>:
>>
>>> 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 [e-mail]://www.ietf.org/mailman/listinfo/dime
>>>
>>> _______________________________________________
>>> DiME mailing [e-mail]://www.ietf.org/mailman/listinfo/dime
>>>
>>> _______________________________________________ DiME mailing list
>>> [e-mail] https://www.ietf.org/mailman/listinfo/dime
>>
>>