Thanks Haibin. This looks good but there are still a few unresolved issues from
our earlier mail exchange:
> The definitions of EWMA_BYTES_SENT and EWMA_BYTES_RCVD seem problematic.
>
> sent = alpha x sent_present + (1 - alpha) x sent
> rcvd = alpha x rcvd_present + (1 - alpha) x rcvd
>
> As written these equations are not right because sent/rcvd appear on both
> sides. It would be clearer to use last_sent and last_rcvd or some such on the
> right-hand side of these equations. But this begs some bigger questions:
>
> - Does this place a requirement on all nodes implementing this specification
> to have to calculate these values every 5 seconds?
> - How are the values calculated the first time?
> - How was the value of 5 seconds chosen?
>
> [Haibin] This is incorporated from the early RELOAD document. Maybe some
> author of the RELOAD protocol can give a better explanation.
This still has not been explained.
>
> Finally, I have trouble understanding why only some of the types are subject
> to access control as described in Section 7. It seems to me that in some
> situations all of the fields defined here could be considered sensitive by
> certain nodes or on certain networks. What is the justification for only
> making some of them subject to access control?
>
> [haibin] Actually it says different level of access control can be made. For
> example, diagnostic information A can be accessed by node 1 and 2, but
> diagnostic information B can only be accessed by node 2. We can add some
> explanations as it seems not clear to readers.
Although I see the new text, the problem still remains that in Section 5.3 only
some of the fields have this disclaimer:
Access to this kind of diagnostic information MUST
NOT be allowed unless compliant to the rules defined in Section 7
<https://tools.ietf.org/html/draft-ietf-p2psip-diagnostics-18#section-7>.
I think the disclaimer should apply to all of the fields, not only a selection
of them. As such, I would recommend just putting the disclaimer once at the top
of section 5.3 and explaining that it applies to all of the fields.
Thanks,
Alissa
> On Nov 8, 2015, at 5:41 PM, Songhaibin (A) <[email protected]> wrote:
>
> Hi Alissa,
>
> The revision of this draft is now available. Please check if it solves your
> comments.
>
> Best Regards!
> -Haibin
>
> From: Alissa Cooper [mailto:[email protected] <mailto:[email protected]>]
> Sent: Saturday, October 03, 2015 8:06 PM
> To: Songhaibin (A)
> Cc: p2psip; [email protected]
> <mailto:[email protected]>
> Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17
>
> Wanted to check if a rev of this document is forthcoming?
>
> Thanks,
> Alissa
>
> On Sep 4, 2015, at 3:51 PM, Alissa Cooper <[email protected]
> <mailto:[email protected]>> wrote:
>
> Thanks for the reply. Comments are inline.
>
> On Aug 20, 2015, at 8:22 PM, Songhaibin (A) <[email protected]
> <mailto:[email protected]>> wrote:
>
> Hi Alissa,
>
> Thank you for the comments. Please find my explanations in line with [haibin].
>
>
> Thanks. I did have one further thought for Section 8. It strikes me that the
> combination of diagnostic elements that this spec makes available could
> easily be used to fingerprint a peer in an overlay where peers may otherwise
> be attempting to remain anonymous. Although defenses against such
> fingerprinting are probably hard to come by (short of disallowing access to
> the diagnostics via the config file), the threat is worth noting at least.
>
> [haibin]Actually we imply your concern with the sentence "It should also be
> noted that attackers can use diagnostics to analyze overlay information to
> attack certain key peers." But we could elaborate more in this section.
>
>
> Yes, further elaboration would be appreciated.
>
>
>
> From: Alissa Cooper [mailto:[email protected] <mailto:[email protected]>]
> Sent: Wednesday, August 19, 2015 12:04 AM
> To: David Bryan
> Cc: p2psip; [email protected]
> <mailto:[email protected]>
> Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17
>
> On Aug 17, 2015, at 8:54 AM, David Bryan <[email protected]
> <mailto:[email protected]>> wrote:
>
>
> Thanks for the comments. I'll circle back with the team and will revise.
>
> Thanks. I did have one further thought for Section 8. It strikes me that the
> combination of diagnostic elements that this spec makes available could
> easily be used to fingerprint a peer in an overlay where peers may otherwise
> be attempting to remain anonymous. Although defenses against such
> fingerprinting are probably hard to come by (short of disallowing access to
> the diagnostics via the config file), the threat is worth noting at least.
>
>
>
> I have to check, but I actually think the first few versions of this (when it
> was individual) *may* be pre-5378, but will double check.
>
> Ah ok.
>
> [haibin] It is true that the first version of the individual draft is
> pre-5378.
>
> Ok, my mistake then.
>
>
>
> Alissa
>
>
>
> On Fri, Aug 14, 2015 at 6:33 PM, Alissa Cooper <[email protected]
> <mailto:[email protected]>> wrote:
> I have reviewed this document in preparation for IETF last call. Before
> proceeding to last call, I have some comments and questions that I'd like to
> discuss. I've also included a list of nits that should be resolved together
> with any last call comments.
>
>
> == Substantive comments and questions ==
>
> = General
>
> I think this document should be listed as updating RFC 6940 in the document
> header.
>
> The document contains a disclaimer for pre-RFC5378 work, which I assume is in
> error. If using xml2rfc to generate the text, using the most up-to-date
> version of xml2rfc to create the next version should fix this.
>
> = Section 4.3:
>
> "There have been proposals that RouteQuery and a series of Fetch
> requests can be used to replace the PathTrack mechanism, but in the
> presence of churn such an operation would not, strictly speaking,
> provide identical results, as the path may change between RouteQuery
> and Fetch operations. (although obviously the path could change
> between steps of PathTrack as well)."
>
> This text is confusing. It doesn't explain why PathTrack is being defined
> rather than peers using RouteQuery plus Fetch. Furthermore, the document does
> not explain how peers are supposed to interpret the results of the PathTrack
> method in the event that the route does change in between steps. This seems
> like a substantial omission.
>
> [haibin] This is a good question. As the route change is a problem (with
> little probability), we would like to retrieve diagnostics info in an
> efficient way, so one simple message is preferred. And you are right, it does
> not change the truth that route can change in between steps. We need
> re-wording the section. The implication of route change is that, the result
> of the diagnostics cannot be trusted with one hundred percent, which should
> be noted to the users that perform the diagnostics.
>
> Ok, new text would be appreciated.
>
>
>
> In general, this document provides very little information about how peers
> are expected to interpret and use the diagnostic information. I understand
> that peers can do whatever they want with it, but in several places the fact
> that this is not explained makes the reasoning behind the protocol design
> decisions opaque. I would suggest providing a little more context, at the
> very least by moving the examples into the body of the draft somewhere.
>
> [haibin] We can add some more explanations with diagnostics info
> interpretation.
>
> Ok, good.
>
>
>
> = Section 4.4:
>
> I'm a little confused about the values given for the error codes. Do you want
> IANA to chose these specific values? Or are they just there as placeholders?
> The customary way of doing placeholders would be as follows:
>
> Code Value Error Code Name
> [TBD1] Underlay Destination Unreachable
> [TBD2] Underlay Time exceeded
> [TBD3] Message Expired
> [TBD4] Upstream Misrouting
> [TBD5] Loop detected
> [TBD6] TTL hops exceeded
>
> These placeholders would then get used throughout the document.
>
> [haibin] they are place holders.
>
> Ok, please clarify it in the document in that case.
>
>
>
> This section also says:
>
> "In addition, this document introduces several types of error information in
> the error_info field in the case of Code 0x65. ... Here are some examples for
> the error info. ... The error_info field values of the Code 0x66 to 0x70 are
> to be application specific and defined by the particular overlay."
>
>
> I'm not sure what is meant by all of this text. As defined in RFC 6940,
> error_info is an optional implementation-specific byte string. If you are
> intending to require implementations to include specific text in the
> error_info for the first error code, then you need to explain under what
> conditions the specific text needs to be included for each string. And the
> text about the other codes seems redundant with how error_info is already
> defined in RFC 6940.
>
>
> [haibin] if a peer receives a diagnostic request found that it cannot reach
> its next hop due to underlay problem, it needs to explain with error_info
> what happened. I do not know what you mean by redundant with RFC 6940. They
> are different errors.
>
> Maybe it’s just a language thing, but the first sentence saying “this
> document introduces …” makes it sound like the list of strings is more than
> just a list of examples. If these are just examples that implementations can
> choose to use or not, I would suggest the following re-phrasing:
>
> OLD
> In addition, this document introduces several types of error
> information in the error_info field in the case of Code 0x65. These
> are represented as an opaque UTF-8 text string. Here are some
> examples for the error info.
>
>
> NEW
> Here are some examples of errors that might be expressed using the error_info
> field in the case of Code [TBD1]:
>
>
>
>
> = Section 5.2:
>
>
> "ext_length : the length of the returned DiagnosticInfo information
> in bytes. If the value is greater than or equal to 1, then some
> extended diagnostic information is requested."
>
> This is defining the diagnostic response - why would the extension be
> requesting something? And if it is possible to use an extension in that way,
> the expected behavior from the initiator when it receives such an extension
> needs to be described, or at least it needs to be noted that a diagnostic
> response could provoke a request back to the initiator.
>
> [haibin] it is possible to reword it as "....then some extended diagnostic
> information is responded.”
>
>
> How about “is being sent in the response”?
>
>
>
> Also, in the struct the diagnostic info is called "diagnostic_info_contents"
> but in the text it is called "diagnostic_information." These should be the
> same I think.
>
> [haibin] Yes. It will be corrected.
>
> Ok
>
>
>
> = Section 5.3:
>
> I'm concerned about the way STATUS_INFO is defined. First, why are there 16
> different levels of congestion status? What are they supposed to signify? If
> there is no standardized way of measuring congestion defined, how is a node
> supposed to interpret STATUS_INFO received from different peers? That is,
> couldn't one peer's level 7 actually mean it is less congested than another
> peer's level 3, and won't that make the information somewhat meaningless to
> the initiator? Maybe this would be clearer if you could explain what you
> expect an initiator to do with the information about 16 different levels of
> congestion.
>
> [haibin] Your concern makes sense. Actually it is intended to leave it for
> overlay implementers or a future standard to define the congestion level
> algorithms . This draft just defines the format and reserves the bits for
> their use. As the section says " This document does not provide a specific
> method for congestion, leaving this decision to each node. One possible
> option for a node would be to take its CPU/memory/bandwidth usage percentage
> in the past 600 seconds and normalize the highest value to the range [0x00,
> 0x0F]. A future draft may define an objective measure or specific algorithm
> for this." I think we should change the node decision to overlay decision.
> And an overlay implementer may choose to not use all that 16 values.
>
> Ok, further clarification would help.
>
>
>
> SOFTWARE_VERSION doesn't really seem like diagnostic information. While all
> of the other fields may be constantly changing and therefore it would make
> sense to inquire about them in a diagnostic fashion, SOFTWARE_VERSION seems
> more like data that nodes could register or exchange when they join the
> overlay. Why is it included as a diagnostic?
>
> [haibin] Diagnostics usually take place when there are failures or abnormal
> behaviors. Software version incompatibility might be a cause.
>
> Ok, fair enough. Still seems like an odd choice to me.
>
>
>
> Also, doesn't the SOFTWARE_VERSION type need a length or a specified
> delimiter that indicates the end of the string? Otherwise, how is the
> recipient supposed to know when to stop parsing it?
>
> [haibin] Will add it.
>
> Ok
>
>
>
> The definitions of EWMA_BYTES_SENT and EWMA_BYTES_RCVD seem problematic.
>
> sent = alpha x sent_present + (1 - alpha) x sent
> rcvd = alpha x rcvd_present + (1 - alpha) x rcvd
>
> As written these equations are not right because sent/rcvd appear on both
> sides. It would be clearer to use last_sent and last_rcvd or some such on the
> right-hand side of these equations. But this begs some bigger questions:
>
> - Does this place a requirement on all nodes implementing this specification
> to have to calculate these values every 5 seconds?
> - How are the values calculated the first time?
> - How was the value of 5 seconds chosen?
>
> [Haibin] This is incorporated from the early RELOAD document. Maybe some
> author of the RELOAD protocol can give a better explanation.
>
> Finally, I have trouble understanding why only some of the types are subject
> to access control as described in Section 7. It seems to me that in some
> situations all of the fields defined here could be considered sensitive by
> certain nodes or on certain networks. What is the justification for only
> making some of them subject to access control?
>
> [haibin] Actually it says different level of access control can be made. For
> example, diagnostic information A can be accessed by node 1 and 2, but
> diagnostic information B can only be accessed by node 2. We can add some
> explanations as it seems not clear to readers.
>
> Ok, will await the revision.
>
>
>
>
> = Section 6.1:
>
> "The destination field MUST be set to the desired destination, which MAY be
> either a NodeId or ResourceID but SHOULD NOT be the broadcast NodeID."
>
> What is the expected behavior if a Ping or a PathTrack does get sent to the
> broadcast NodeID? Or should these requirements really be MUST NOTs?
>
> [haibin] I guess they should be MUST NOTs.
>
> Ok
>
>
>
> = Section 6.4:
>
> "However, for a single hop
> measurement, the traditional measurement methods MUST be used instead
> of the overlay layer diagnostics methods."
>
> What is meant by the traditional measurement methods?
>
> [haibin] It mainly means IP layer ping.
>
> Would be good to specify.
>
> Thanks,
> Alissa
>
>
>
> = Section 9:
>
> Please only use either RFC-XXXX or RFC-AAAA but not both as placeholders.
>
> [haibin] RFC-AAAA is RFC 6940. RFC-XXXX is the placeholder. They will be
> corrected.
>
> Please also use [TBDX] for values you expect to be allocated by IANA.
>
> [haibin] ok.
>
> == Nits ==
>
>
> = General
>
>
> The document seems to repeatedly explain that it is doing things "as
> specified in RFC6940." I don't think this necessary in many cases. It's worth
> doing a pass through and removing the places where it's obvious that this
> spec is building on RFC 6940.
>
> [haibin] ok.
>
> = Section 4.1:
>
>
> OLD
> The extensions strictly follow RELOAD
> specification on the messages routing, transport, NAT traversal etc.
>
> NEW
> The extensions strictly follow how RELOAD specifies message routing,
> transport, NAT traversal, and other RELOAD protocol features.
>
> = Section 4.2.1:
>
>
> s/as specified in Table 4 and specified in the RELOAD/as specified in Table 4/
>
> s/new requests of the the/new requests of the/
>
> = Section 4.3:
>
> OLD
> We define a simple PathTrack method for retrieving diagnostic
> information iteratively. The mechanism defined in this document
> follows the RELOAD specification, the new request and response
> message use the message format specified in RELOAD messages. Please
> refer to the RELOAD [RFC6940] for details of the protocol.
>
> The operation of this request is shown below in Figure 2.
>
>
> NEW
> We define a simple PathTrack method for retrieving diagnostic
> information iteratively. The operation of this request is shown below in
> Figure 2.
>
> = Section 4.3.1.2:
>
> s/after that/after receiving a PathTrackAns where the next_hop node ID equals
> the responding node ID/
>
> = Section 4.3.2.1:
>
> s/PathTrack Response/PathTrack response/
>
> = Section 4.4:
>
> s/code. and we define/code. We define/
>
> = Section 5:
>
> s/Path_track methods/PathTrack methods/
>
> = Section 5.1:
>
> s/Note that is NOT/Note that it is not/
>
> = Section 5.2:
>
> s/Note that is NOT/Note that it is not/
>
> s/consists one or more/consists of one or more/
>
> = Section 6.1:
>
> s/`MAY/MAY/
>
> = Section 6.2:
>
> s/return the response the initiator node/return the response to the initiator
> node/
>
> s/The peer SHOULD/The intermediate peer SHOULD/
>
> [haibin] Thanks again.
>
> BR,
> -Haibin Song
>
>
> _______________________________________________
> P2PSIP mailing list
> [email protected] <mailto:[email protected]>
> https://www.ietf.org/mailman/listinfo/p2psip
> <https://www.ietf.org/mailman/listinfo/p2psip>
>
>
> _______________________________________________
> P2PSIP mailing list
> [email protected] <mailto:[email protected]>
> https://www.ietf.org/mailman/listinfo/p2psip
> <https://www.ietf.org/mailman/listinfo/p2psip>
_______________________________________________
P2PSIP mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/p2psip
