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

From: Stephen Farrell <[email-masked]>
Date: Fri, 6 Jan 2017 16:33:21 +0000

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.
> 
> (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.)
> 
> (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.
> 
> nits (fine to be considered last call comments):
> -------------------------------------------------
> 
> 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
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dime
>

---

From: Steve Donovan <[email-masked]>
Date: Mon, 9 Jan 2017 16:28:10 -0600

Stephen,

Thanks for the review and for the ping.  Comments below.

Regards,

Steve

>> Things to chat about before starting IETF LC:
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.
>> 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.
>> 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.

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 12 Jan 2017 11:54:02 +0000

Hiya,

Just one thing below I'd like to figure out before
IETF LC...

> do be in the DOIC RFC but it can go here if needed.

Ok, let's handle that as an IETF LC comment. If others
think a definition would be good you can add it later.
If it's just me, don't bother.

> through proprietary implementations.

Sure, that's not a good reason to make it harder though.
But see below...

> report of type peer that isn't of type peer are supposed to remove it.

I don't understand what " a Load report of type peer that
isn't of type peer" can mean.

> Doing anything other than this would require a change to the base

Either I'm mis-remembering the draft or we're talking at
cross purposes. My reading was that nodes that do support
the mechanism could delete a peer report that actually
comes via a node that does not support the mechanism. Am
I wrong? If so maybe there's a clarity issue. If not, I
don't see why that makes sense.

Cheers,
S.

PS: The rest below is fine to handle later as you suggest.

---

From: Steve Donovan <[email-masked]>
Date: Thu, 12 Jan 2017 09:34:09 -0600

Stephen,

I think that what you are proposing is for the following network:

A' ---- B' ----- C ----- D --|-- E'
                                               ^
                                 Administrative boundary

Where A', B' and E' support load and C and D do not.

I think you are proposing that node D strips a peer load report inserted 
by B'.  Is this correct?

The issue is that this is load-specific functionality, requiring D to 
understand at least some of the Load mechanism.  But, by definition,  D 
does not support or understand anything about the load mechanism.

I don't see a way of achieving what you propose without adding load 
specific functionality to D.

Steve

---

From: Steve Donovan <[email-masked]>
Date: Thu, 12 Jan 2017 11:16:28 -0600

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:
>
> 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 <[email-masked] 
> <mailto:[email-masked]>>:
>
>     Stephen,
>
>     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.
>     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.
>     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.
>>>     [email-masked] <mailto:[email-masked]>
>>>     <https://www.ietf.org/mailman/listinfo/dime>
>>>
>>     [email-masked] <mailto:[email-masked]>
>>     <https://www.ietf.org/mailman/listinfo/dime>
>     _______________________________________________ DiME mailing list
>     [email-masked] <mailto:[email-masked]>
>     <https://www.ietf.org/mailman/listinfo/dime> 
>

---

From: Misha Zaytsev <[email-masked]>
Date: Thu, 12 Jan 2017 20:51:14 +0300

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 <[email-masked]>:

> 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> 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 <[email-masked]>:
>
>> Stephen,
>>
>> 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.
>>
>> 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.
>>
>> 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.
>>
>> DiME mailing [email-masked]://www.ietf.org/mailman/listinfo/dime
>>
>> DiME mailing [email-masked]://www.ietf.org/mailman/listinfo/dime
>>
>> [email-masked] https://www.ietf.org/mailman/listinfo/dime
>
>

---

From: Steve Donovan <[email-masked]>
Date: Thu, 12 Jan 2017 14:56:58 -0600

Stephen correctly pointed out that there is missing language in the 
normative portion of the draft.  It is explained in section 5 that and 
agent removes all load reports that it receives in an answer message.  
The normative text for this requirement did not make it into section 
6.2.  As such, I will be submitting a new version of the draft with the 
following added to section 6.2:

"In all cases, a Diameter Agent MUST strip all load reports of type peer
received in answer messages.

     Note: This ensures that there will be precisely one load report of
type peer, that of the Diameter node sending the message, in any answer
messages sent by the Diameter agent. "

Let me know if there are any issues with this wording.

Regards,

Steve

On 1/12/17 9:34 AM, Steve Donovan wrote:
> I think you are proposing that node D strips a peer load report 
> inserted by B'.  Is this correct?
>
> understand at least some of the Load mechanism.  But, by definition,  
> D does not support or understand anything about the load mechanism.
>
>>> required

---

From: Steve Donovan <[email-masked]>
Date: Thu, 12 Jan 2017 15:06:28 -0600

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:
> 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".
>
> This is achieved by comparing the DiameterIdentity of the peer from which the message was received with the
> 2017-01-12 20:16 GMT+03:00 Steve Donovan <[email-masked] 
>>     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.
>     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?
>>         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.
>>         "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.
>>         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.
>>         _______________________________________________ DiME mailing
>>         list [email-masked] <mailto:[email-masked]>

---

From: Misha Zaytsev <[email-masked]>
Date: Sun, 15 Jan 2017 13:00:34 +0300

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 <[email-masked]>:

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

---

From: Steve Donovan <[email-masked]>
Date: Sun, 15 Jan 2017 22:19:18 -0800

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.
Agreed, I'll update in the next release.
> - Use "remove"/"discard" instead of "strip" since it seems to be more 
This is a matter of preference and I prefer strip. :-)
> - "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?
This is a good point.  I'll update accordingly.
> 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.
>     weight values specified in [RFC2782].
Agreed
> Under "These load reports are removed..." the report of type PEER is 
> meant, right?
> "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.
>        meaningful value, as long as the agent*e*nsures that all peers
Agreed
>     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
> But I have found only one place where it is mentioned: Appendix B, 
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 <[email-masked] 
>     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.
>
>>     My bad that I have not read the draft more carefully - will try
>>     to avoid this next time.
>>
>>     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?
>>
>>     Or it can be easily re-phrased in this way, the term "peer"
>>     should not confuse anyone, I guess :)
>>
>>     2017-01-12 20:16 GMT+03:00 Steve Donovan
>>     <[email-masked] <mailto:[email-masked]>>:
>>
>>         Misha,
>>
>>>         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> 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
>>>         <[email-masked] <mailto:[email-masked]>>:
>>>
>>>             Stephen,
>>>
>>>             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.
>>>             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
>>>             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
>>>             _______________________________________________ DiME
>>>             mailing list [email-masked] <mailto:[email-masked]>