From nobody Sat Oct 3 05:05:59 2015 Return-Path: X-Original-To: p2psip@ietfa.amsl.com Delivered-To: p2psip@ietfa.amsl.com Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id AAC4C1A0393 for ; Sat, 3 Oct 2015 05:05:58 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.699 X-Spam-Level: X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=ham Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HjeS5OLkypuS for ; Sat, 3 Oct 2015 05:05:53 -0700 (PDT) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D517A1A038C for ; Sat, 3 Oct 2015 05:05:52 -0700 (PDT) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 3A29C2049B for ; Sat, 3 Oct 2015 08:05:52 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute6.internal (MEProxy); Sat, 03 Oct 2015 08:05:52 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=cooperw.in; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=5ru0g mLOuobLqk+hUWHogCJWw8o=; b=Er1bWyNUddCx1Cwc5lXvXwQ9sZ3jEJlXcm2eL 2s+OqwMP92xC/k89j7/92H+VKXvW56W2pO62bGyNy44lVPXyaG46/cfJeTVc7ZB1 LSUJvwoBo0dvpH7w+SaAgss5Qjapn7Jor1UW0LxxTa8lP3QfCzWeB7j+vOOTnOw9 Qm6clw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=5ru0gmLOuobLqk+hUWHogCJWw8o=; b=fg4V8 eKm1DJ0TqbbEOj4nUelALXDIuC0GwSO9r/yiu1EIRXDjYL32C/iuayo+gV5ST9zL /5l/Ukx00alMnTdYWKfPCCi54BU81xJFE7oYOKqP4JHwI+3dLVAY1sHawlPBWQ9g 17oQaNsVuVIm0y2EBCtpofpYRotFhCERDxazHg= X-Sasl-enc: mRfXsrvmHlrJD51bWsTZ7wuErr97xXNZxHcMo9P12CUT 1443873951 Received: from sjc-alcoop-8814.cisco.com (unknown [128.107.241.167]) by mail.messagingengine.com (Postfix) with ESMTPA id 434CBC0001A; Sat, 3 Oct 2015 08:05:51 -0400 (EDT) Content-Type: multipart/alternative; boundary="Apple-Mail=_A9165472-2A5B-4CBC-A9A5-C59BA7D1B420" Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) From: Alissa Cooper In-Reply-To: Date: Sat, 3 Oct 2015 05:05:50 -0700 Message-Id: References: <37059182-6C51-49E3-83A0-D27F2C15A366@cooperw.in> <5C131A2D-2519-4CD5-AAB0-1E01563C9665@cooperw.in> To: "Songhaibin (A)" X-Mailer: Apple Mail (2.2104) Archived-At: Cc: p2psip , "draft-ietf-p2psip-diagnostics@tools.ietf.org" Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17 X-BeenThere: p2psip@ietf.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Peer-to-Peer SIP working group discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 03 Oct 2015 12:05:58 -0000 --Apple-Mail=_A9165472-2A5B-4CBC-A9A5-C59BA7D1B420 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=windows-1252 Wanted to check if a rev of this document is forthcoming? Thanks, Alissa > On Sep 4, 2015, at 3:51 PM, Alissa Cooper wrote: >=20 > Thanks for the reply. Comments are inline. >=20 >> On Aug 20, 2015, at 8:22 PM, Songhaibin (A) > wrote: >>=20 >> Hi Alissa, >>=20 >> Thank you for the comments. Please find my explanations in line with = [haibin]. >>=20 >>> 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. >>=20 >> [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.=20 >>=20 >=20 > Yes, further elaboration would be appreciated. >=20 >>=20 >> From: Alissa Cooper [mailto:alissa@cooperw.in = ]=20 >> Sent: Wednesday, August 19, 2015 12:04 AM >> To: David Bryan >> Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org = >> Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17 >>=20 >> On Aug 17, 2015, at 8:54 AM, David Bryan > wrote: >>=20 >>=20 >> Thanks for the comments. I'll circle back with the team and will = revise. >>=20 >> 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. >>=20 >>=20 >>=20 >> 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. >>=20 >> Ah ok. >>=20 >> [haibin] It is true that the first version of the individual draft is = pre-5378. >=20 > Ok, my mistake then. >=20 >>=20 >> Alissa >>=20 >>=20 >>=20 >> On Fri, Aug 14, 2015 at 6:33 PM, Alissa Cooper > 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. >>=20 >>=20 >> =3D=3D Substantive comments and questions =3D=3D >>=20 >> =3D General=20 >>=20 >> I think this document should be listed as updating RFC 6940 in the = document header. >>=20 >> 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. >>=20 >> =3D Section 4.3: >>=20 >> "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)." >>=20 >> 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. >>=20 >> [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. >=20 > Ok, new text would be appreciated. >=20 >>=20 >> 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.=20 >>=20 >> [haibin] We can add some more explanations with diagnostics info = interpretation.=20 >=20 > Ok, good. >=20 >>=20 >> =3D Section 4.4: >>=20 >> 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: >>=20 >> 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 >>=20 >> These placeholders would then get used throughout the document. >>=20 >> [haibin] they are place holders.=20 >=20 > Ok, please clarify it in the document in that case. >=20 >>=20 >> This section also says: >>=20 >> "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." >>=20 >>=20 >> 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. >>=20 >>=20 >> [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. >=20 > Maybe it=92s just a language thing, but the first sentence saying = =93this document introduces =85=94 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: >=20 > 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. >=20 >=20 > NEW > Here are some examples of errors that might be expressed using the = error_info field in the case of Code [TBD1]: >=20 >=20 >>=20 >> =3D Section 5.2: >>=20 >>=20 >> "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." >>=20 >> 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. >>=20 >> [haibin] it is possible to reword it as "....then some extended = diagnostic information is responded.=94 >>=20 >=20 > How about =93is being sent in the response=94? >=20 >>=20 >> 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. >>=20 >> [haibin] Yes. It will be corrected. >=20 > Ok >=20 >>=20 >> =3D Section 5.3: >>=20 >> 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. >>=20 >> [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. >=20 > Ok, further clarification would help. >=20 >>=20 >> 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? >>=20 >> [haibin] Diagnostics usually take place when there are failures or = abnormal behaviors. Software version incompatibility might be a cause. >=20 > Ok, fair enough. Still seems like an odd choice to me. >=20 >>=20 >> 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? >>=20 >> [haibin] Will add it. >=20 > Ok >=20 >>=20 >> The definitions of EWMA_BYTES_SENT and EWMA_BYTES_RCVD seem = problematic. >>=20 >> sent =3D alpha x sent_present + (1 - alpha) x sent >> rcvd =3D alpha x rcvd_present + (1 - alpha) x rcvd >>=20 >> 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: >>=20 >> - 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? >>=20 >> [Haibin] This is incorporated from the early RELOAD document. Maybe = some author of the RELOAD protocol can give a better explanation. >>=20 >> 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? >>=20 >> [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. >=20 > Ok, will await the revision. >=20 >>=20 >>=20 >> =3D Section 6.1: >>=20 >> "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." >>=20 >> 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? >>=20 >> [haibin] I guess they should be MUST NOTs.=20 >=20 > Ok >=20 >>=20 >> =3D Section 6.4: >>=20 >> "However, for a single hop >> measurement, the traditional measurement methods MUST be used = instead >> of the overlay layer diagnostics methods." >>=20 >> What is meant by the traditional measurement methods? >>=20 >> [haibin] It mainly means IP layer ping. >=20 > Would be good to specify. >=20 > Thanks, > Alissa >=20 >>=20 >> =3D Section 9: >>=20 >> Please only use either RFC-XXXX or RFC-AAAA but not both as = placeholders. >>=20 >> [haibin] RFC-AAAA is RFC 6940. RFC-XXXX is the placeholder. They will = be corrected. >>=20 >> Please also use [TBDX] for values you expect to be allocated by IANA. >>=20 >> [haibin] ok. >>=20 >> =3D=3D Nits =3D=3D >>=20 >>=20 >> =3D General >>=20 >>=20 >> 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. >>=20 >> [haibin] ok. >>=20 >> =3D Section 4.1: >>=20 >>=20 >> OLD >> The extensions strictly follow RELOAD >> specification on the messages routing, transport, NAT traversal = etc. >>=20 >> NEW >> The extensions strictly follow how RELOAD specifies message routing, = transport, NAT traversal, and other RELOAD protocol features. >>=20 >> =3D Section 4.2.1: >>=20 >>=20 >> s/as specified in Table 4 and specified in the RELOAD/as specified in = Table 4/ >>=20 >> s/new requests of the the/new requests of the/ >>=20 >> =3D Section 4.3: >>=20 >> 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. >>=20 >> The operation of this request is shown below in Figure 2. >>=20 >>=20 >> NEW >> We define a simple PathTrack method for retrieving diagnostic >> information iteratively. The operation of this request is shown = below in Figure 2. >>=20 >> =3D Section 4.3.1.2: >>=20 >> s/after that/after receiving a PathTrackAns where the next_hop node = ID equals the responding node ID/ >>=20 >> =3D Section 4.3.2.1: >>=20 >> s/PathTrack Response/PathTrack response/ >>=20 >> =3D Section 4.4: >>=20 >> s/code. and we define/code. We define/ >>=20 >> =3D Section 5: >>=20 >> s/Path_track methods/PathTrack methods/ >>=20 >> =3D Section 5.1: >>=20 >> s/Note that is NOT/Note that it is not/ >>=20 >> =3D Section 5.2: >>=20 >> s/Note that is NOT/Note that it is not/ >>=20 >> s/consists one or more/consists of one or more/ >>=20 >> =3D Section 6.1: >>=20 >> s/`MAY/MAY/ >>=20 >> =3D Section 6.2: >>=20 >> s/return the response the initiator node/return the response to the = initiator node/ >>=20 >> s/The peer SHOULD/The intermediate peer SHOULD/ >>=20 >> [haibin] Thanks again. >>=20 >> BR, >> -Haibin Song >>=20 >>=20 >> _______________________________________________ >> P2PSIP mailing list >> P2PSIP@ietf.org >> https://www.ietf.org/mailman/listinfo/p2psip >>=20 >>=20 >=20 > _______________________________________________ > P2PSIP mailing list > P2PSIP@ietf.org > https://www.ietf.org/mailman/listinfo/p2psip --Apple-Mail=_A9165472-2A5B-4CBC-A9A5-C59BA7D1B420 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=windows-1252
Wanted to check if a rev of this document is = forthcoming?

Thanks,
Alissa

On = Sep 4, 2015, at 3:51 PM, Alissa Cooper <alissa@cooperw.in> = wrote:

Thanks for the reply. Comments are inline.

On Aug 20, 2015, at 8:22 PM, Songhaibin (A) <haibin.song@huawei.com> 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:alissa@cooperw.in]=
Sent: Wednesday, August 19, 2015 12:04 AM
To: David Bryan
Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org
Subject: Re: [P2PSIP] AD evaluation: = draft-ietf-p2psip-diagnostics-17

On Aug 17, = 2015, at 8:54 AM, David Bryan <dbryan@ethernot.org> 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 <alissa@cooperw.in> = 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.


=3D=3D Substantive comments and = questions =3D=3D

=3D 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.

=3D 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.


=3D 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] =             &n= bsp; Underlay Destination Unreachable
=      [TBD2] =             &n= bsp; Underlay Time exceeded
=      [TBD3] =             &n= bsp; Message Expired
=      [TBD4] =             &n= bsp; Upstream Misrouting
=      [TBD5] =             &n= bsp; Loop detected
=      [TBD6] =             &n= bsp; 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=92s just a language thing, but the first sentence = saying =93this document introduces =85=94 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]:



=3D 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.=94


How = about =93is being sent in the response=94?


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


=3D 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 =3D alpha x sent_present + (1 - alpha) x = sent
rcvd =3D 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.



=3D= 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


=3D 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


=3D 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.

=3D=3D Nits =3D=3D


=3D 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.

=3D 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.

=3D 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/

=3D = 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.

=3D Section 4.3.1.2:

s/after that/after receiving a PathTrackAns = where the next_hop node ID equals the responding node ID/

=3D Section 4.3.2.1:

s/PathTrack Response/PathTrack response/

=3D Section 4.4:

s/code. and we = define/code. We define/

=3D Section 5:

s/Path_track methods/PathTrack methods/

=3D Section 5.1:

s/Note that is NOT/Note that it is not/

=3D Section 5.2:

s/Note that is = NOT/Note that it is not/

s/consists one or = more/consists of one or more/

=3D Section = 6.1:

s/`MAY/MAY/

=3D 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
P2PSIP@ietf.org
https://www.ietf.org/mailman/listinfo/p2psip



_______________________________________________
P2PSIP mailing list
P2PSIP@ietf.org
https://www.ietf.org/mailman/listinfo/p2psip

= --Apple-Mail=_A9165472-2A5B-4CBC-A9A5-C59BA7D1B420-- From nobody Wed Oct 7 18:19:00 2015 Return-Path: X-Original-To: p2psip@ietfa.amsl.com Delivered-To: p2psip@ietfa.amsl.com Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 287B91B31C0 for ; Wed, 7 Oct 2015 18:18:58 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -4.21 X-Spam-Level: X-Spam-Status: No, score=-4.21 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01] autolearn=ham Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OsD3bk0PAi_b for ; Wed, 7 Oct 2015 18:18:47 -0700 (PDT) Received: from lhrrgout.huawei.com (lhrrgout.huawei.com [194.213.3.17]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 264041B31BD for ; Wed, 7 Oct 2015 18:18:45 -0700 (PDT) Received: from 172.18.7.190 (EHLO lhreml404-hub.china.huawei.com) ([172.18.7.190]) by lhrrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued) with ESMTP id BYM43897; Thu, 08 Oct 2015 01:18:42 +0000 (GMT) Received: from NKGEML402-HUB.china.huawei.com (10.98.56.33) by lhreml404-hub.china.huawei.com (10.201.5.218) with Microsoft SMTP Server (TLS) id 14.3.235.1; Thu, 8 Oct 2015 02:18:40 +0100 Received: from NKGEML501-MBS.china.huawei.com ([169.254.2.75]) by nkgeml402-hub.china.huawei.com ([10.98.56.33]) with mapi id 14.03.0235.001; Thu, 8 Oct 2015 09:18:34 +0800 From: "Songhaibin (A)" To: Alissa Cooper Thread-Topic: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17 Thread-Index: AQHQ1umosqhZ/UWGA0i8P+k8SWdo8p4P1gUAgAGVAoCAAygiwIAYAXYAgCzfIQCAB6yz8A== Date: Thu, 8 Oct 2015 01:18:33 +0000 Message-ID: References: <37059182-6C51-49E3-83A0-D27F2C15A366@cooperw.in> <5C131A2D-2519-4CD5-AAB0-1E01563C9665@cooperw.in> In-Reply-To: Accept-Language: en-US, zh-CN Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.138.41.75] Content-Type: multipart/alternative; boundary="_000_E33E01DFD5BEA24B9F3F18671078951F653A1614nkgeml501mbschi_" MIME-Version: 1.0 X-CFilter-Loop: Reflected Archived-At: Cc: p2psip , "draft-ietf-p2psip-diagnostics@tools.ietf.org" Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17 X-BeenThere: p2psip@ietf.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Peer-to-Peer SIP working group discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Oct 2015 01:18:58 -0000 --_000_E33E01DFD5BEA24B9F3F18671078951F653A1614nkgeml501mbschi_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes. We will give a new revision in a few days. Best Regards! -Haibin From: Alissa Cooper [mailto:alissa@cooperw.in] Sent: Saturday, October 03, 2015 8:06 PM To: Songhaibin (A) Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org 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 > wrote: Thanks for the reply. Comments are inline. On Aug 20, 2015, at 8:22 PM, Songhaibin (A) > wrote: Hi Alissa, Thank you for the comments. Please find my explanations in line with [haibi= n]. Thanks. I did have one further thought for Section 8. It strikes me that th= e combination of diagnostic elements that this spec makes available could e= asily be used to fingerprint a peer in an overlay where peers may otherwise= be attempting to remain anonymous. Although defenses against such fingerpr= inting are probably hard to come by (short of disallowing access to the dia= gnostics 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:alissa@cooperw.in] Sent: Wednesday, August 19, 2015 12:04 AM To: David Bryan Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17 On Aug 17, 2015, at 8:54 AM, David Bryan > 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 th= e combination of diagnostic elements that this spec makes available could e= asily be used to fingerprint a peer in an overlay where peers may otherwise= be attempting to remain anonymous. Although defenses against such fingerpr= inting are probably hard to come by (short of disallowing access to the dia= gnostics 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-5= 378. Ok, my mistake then. Alissa On Fri, Aug 14, 2015 at 6:33 PM, Alissa Cooper > wrote: I have reviewed this document in preparation for IETF last call. Before pro= ceeding to last call, I have some comments and questions that I'd like to d= iscuss. I've also included a list of nits that should be resolved together = with any last call comments. =3D=3D Substantive comments and questions =3D=3D =3D 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. =3D 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 r= ather than peers using RouteQuery plus Fetch. Furthermore, the document doe= s not explain how peers are supposed to interpret the results of the PathTr= ack method in the event that the route does change in between steps. This s= eems like a substantial omission. [haibin] This is a good question. As the route change is a problem (with li= ttle probability), we would like to retrieve diagnostics info in an efficie= nt 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-wordin= g the section. The implication of route change is that, the result of the d= iagnostics cannot be trusted with one hundred percent, which should be note= d 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 fac= t 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 interpreta= tion. Ok, good. =3D Section 4.4: I'm a little confused about the values given for the error codes. Do you wa= nt IANA to chose these specific values? Or are they just there as placehold= ers? 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 i= n 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 0x= 70 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, err= or_info is an optional implementation-specific byte string. If you are inte= nding to require implementations to include specific text in the error_info= for the first error code, then you need to explain under what conditions t= he specific text needs to be included for each string. And the text about t= he other codes seems redundant with how error_info is already defined in RF= C 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 docum= ent introduces ..." makes it sound like the list of strings is more than ju= st 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_in= fo field in the case of Code [TBD1]: =3D 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 reque= sting 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 ne= eds to be described, or at least it needs to be noted that a diagnostic res= ponse 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 th= e same I think. [haibin] Yes. It will be corrected. Ok =3D 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 n= ode supposed to interpret STATUS_INFO received from different peers? That i= s, couldn't one peer's level 7 actually mean it is less congested than anot= her peer's level 3, and won't that make the information somewhat meaningles= s to the initiator? Maybe this would be clearer if you could explain what y= ou 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 al= gorithms . This draft just defines the format and reserves the bits for the= ir use. As the section says " This document does not provide a specific met= hod for congestion, leaving this decision to each node. One possible optio= n 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, 0x= 0F]. 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 seem= s more like data that nodes could register or exchange when they join the o= verlay. 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 delimi= ter that indicates the end of the string? Otherwise, how is the recipient s= upposed 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 =3D alpha x sent_present + (1 - alpha) x sent rcvd =3D alpha x rcvd_present + (1 - alpha) x rcvd As written these equations are not right because sent/rcvd appear on both s= ides. It would be clearer to use last_sent and last_rcvd or some such on th= e right-hand side of these equations. But this begs some bigger questions: - Does this place a requirement on all nodes implementing this specificatio= n 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 au= thor of the RELOAD protocol can give a better explanation. Finally, I have trouble understanding why only some of the types are subjec= t 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 ma= king some of them subject to access control? [haibin] Actually it says different level of access control can be made. Fo= r example, diagnostic information A can be accessed by node 1 and 2, but di= agnostic information B can only be accessed by node 2. We can add some expl= anations as it seems not clear to readers. Ok, will await the revision. =3D 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 =3D 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 =3D 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 co= rrected. Please also use [TBDX] for values you expect to be allocated by IANA. [haibin] ok. =3D=3D Nits =3D=3D =3D General The document seems to repeatedly explain that it is doing things "as specif= ied in RFC6940." I don't think this necessary in many cases. It's worth doi= ng a pass through and removing the places where it's obvious that this spec= is building on RFC 6940. [haibin] ok. =3D 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, transp= ort, NAT traversal, and other RELOAD protocol features. =3D 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/ =3D 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. =3D Section 4.3.1.2: s/after that/after receiving a PathTrackAns where the next_hop node ID equa= ls the responding node ID/ =3D Section 4.3.2.1: s/PathTrack Response/PathTrack response/ =3D Section 4.4: s/code. and we define/code. We define/ =3D Section 5: s/Path_track methods/PathTrack methods/ =3D Section 5.1: s/Note that is NOT/Note that it is not/ =3D Section 5.2: s/Note that is NOT/Note that it is not/ s/consists one or more/consists of one or more/ =3D Section 6.1: s/`MAY/MAY/ =3D Section 6.2: s/return the response the initiator node/return the response to the initiat= or node/ s/The peer SHOULD/The intermediate peer SHOULD/ [haibin] Thanks again. BR, -Haibin Song _______________________________________________ P2PSIP mailing list P2PSIP@ietf.org https://www.ietf.org/mailman/listinfo/p2psip _______________________________________________ P2PSIP mailing list P2PSIP@ietf.org https://www.ietf.org/mailman/listinfo/p2psip --_000_E33E01DFD5BEA24B9F3F18671078951F653A1614nkgeml501mbschi_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Yes. We wi= ll give a new revision in a few days.

 = ;

Best Regards!

-Haibin

 = ;

From: Alissa Cooper [mailto:alissa@cooperw.in]
Sent: Saturday, October 03, 2015 8:06 PM
To: Songhaibin (A)
Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org
Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-1= 7

 

Wanted to check if a rev of thi= s document is forthcoming?

 

Thanks,

Alissa

 

On Sep 4, 2015, at 3:51 PM, Ali= ssa Cooper <alissa@cooperw.in&g= t; wrote:

 

Thanks for the reply. Comments = are inline.

 

On Aug 20, 2015, at 8:22 PM, So= nghaibin (A) <haibin.song@huaw= ei.com> wrote:

 

Hi Alissa,

Thank you for the comments. Please find my explanations in line with [haibi= n].


Thanks. I did have one further = thought for Section 8. It strikes me that the combination of diagnostic ele= ments 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 fingerpr= inting are probably hard to come by (short of disallowing access to the dia= gnostics via the config file), the threat is worth noting at least.

=
[haibin]Actually we imply your concern with the sentence "It should al= so be noted that attackers can use diagnostics to analyze overlay informati= on to attack certain key peers." But we could elaborate more in this s= ection.

 

Yes, further elaboration would = be appreciated.




From: Alissa Cooper [mailto:alissa@coo= perw.in]
Sent: Wednesday, August 19, 2015 12:04 AM
To: David Bryan
Cc: p2psip; draft-ietf-p2psip-diagnostics@tools.ietf.org
Subject: Re: [P2PSIP] AD evaluation: draft-ietf-p2psip-diagnostics-17

On Aug 17, 2015, at 8:54 AM, David Bryan <dbryan@ethernot.org> 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 th= e combination of diagnostic elements that this spec makes available could e= asily 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-5= 378.

 

Ok, my mistake then.=




Alissa



On Fri, Aug 14, 2015 at 6:33 PM, Alissa Cooper <alissa@cooperw.in> wrote:
I have reviewed this document in preparation for IETF last call. Before pro= ceeding to last call, I have some comments and questions that I'd like to d= iscuss. I've also included a list of nits that should be resolved together = with any last call comments.


=3D=3D Substantive comments and questions =3D=3D

=3D 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 assum= e 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.

=3D 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 speakin= g,
  provide identical results, as the path may change between Route= Query
  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 r= ather than peers using RouteQuery plus Fetch. Furthermore, the document doe= s not explain how peers are supposed to interpret the results of the PathTr= ack 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 li= ttle probability), we would like to retrieve diagnostics info in an efficie= nt 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 implica= tion of route change is that, the result of the diagnostics cannot be trust= ed with one hundred percent, which should be noted to the users that perfor= m the diagnostics.

 

Ok, new text would be appreciat= ed.




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 fac= t 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 interpreta= tion.

 

Ok, good.




=3D Section 4.4:

I'm a little confused about the values given for the error codes. Do you wa= nt IANA to chose these specific values? Or are they just there as placehold= ers? The customary way of doing placeholders would be as follows:

Code Value         Error Code Name<= br>      [TBD1]       &n= bsp;       Underlay Destination Unreacha= ble
     [TBD2]       &n= bsp;       Underlay Time exceeded
     [TBD3]       &n= bsp;       Message Expired
     [TBD4]       &n= bsp;       Upstream Misrouting
     [TBD5]       &n= bsp;       Loop detected
     [TBD6]       &n= bsp;       TTL hops exceeded

These placeholders would then get used throughout the document.

[haibin] they are place holders.

 

Ok, please clarify it in the do= cument in that case.




This section also says:

"In addition, this document introduces several types of error informat= ion in the error_info field in the case of Code 0x65. ... Here are some exa= mples 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, err= or_info is an optional implementation-specific byte string. If you are inte= nding 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 in= cluded 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 languag= e thing, but the first sentence saying “this document introduces R= 30;” 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:<= /o:p>

 

OLD

In a=
ddition, this document introduces several types of error<=
/pre>

   i=
nformation in the error_info field in the case of Code 0x65.  These


   a=
re represented as an opaque UTF-8 text string.  Here are some


   e=
xamples for the error info.




 

 

NEW

Here=
 are some examples of errors that might be expressed using the error_info f=
ield in the case of Code [TBD1]:

 



=
=3D 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 reque= sting 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 ne= eds to be described, or at least it needs to be noted that a diagnostic response could provoke a request ba= ck to the initiator.

[haibin] it is possible to reword it as "....then some extended diagno= stic information is responded.”

 

How about “is being sent = in the response”?



Also, in the struct the diagnostic info is called "diagnostic_info_con= tents" but in the text it is called "diagnostic_information.= " These should be the same I think.

[haibin] Yes. It will be corrected.

 

Ok




=3D 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 n= ode supposed to interpret STATUS_INFO received from different peers? That is, couldn't one peer's level 7 actual= ly mean it is less congested than another peer's level 3, and won't that ma= ke 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 al= gorithms . This draft just defines the format and reserves the bits for the= ir use. As the section says " This document does not provide a specific method for congestion, leaving this d= ecision 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 norm= alize the highest value to the range [0x00, 0x0F].  A future draft may define an objective measure or spec= ific algorithm for this." I think we should change the node decision t= o overlay decision. And an overlay implementer may choose to not use all th= at 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 seem= s more like data that nodes could register or exchange when they join the overlay. Why is it included as a d= iagnostic?

[haibin] Diagnostics usually take place when there are failures or abnormal= behaviors. Software version incompatibility might be a cause.

 

Ok, fair enough. Still seems li= ke an odd choice to me.




Also, doesn't the SOFTWARE_VERSION type need a length or a specified delimi= ter that indicates the end of the string? Otherwise, how is the recipient s= upposed to know when to stop parsing it?

[haibin] Will add it.

 

Ok




The definitions of EWMA_BYTES_SENT and EWMA_BYTES_RCVD seem probl= ematic.

sent =3D alpha x sent_present + (1 - alpha) x sent
rcvd =3D alpha x rcvd_present + (1 - alpha) x rcvd

As written these equations are not right because sent/rcvd appear on both s= ides. It would be clearer to use last_sent and last_rcvd or some such on th= e right-hand side of these equations. But this begs some bigger questions:<= br>
- Does this place a requirement on all nodes implementing this specificatio= n 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 au= thor of the RELOAD protocol can give a better explanation.

Finally, I have trouble understanding why only some of the types are subjec= t 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 c= ontrol?

[haibin] Actually it says different level of access control can be made. Fo= r example, diagnostic information A can be accessed by node 1 and 2, but di= agnostic information B can only be accessed by node 2. We can add some expl= anations as it seems not clear to readers.

 

Ok, will await the revision.





=3D Section 6.1:

"The destination field MUST be set to the desired destination, wh= ich MAY be either a NodeId or ResourceID but SHOULD NOT be the broadca= st 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




=3D Section 6.4:

"However, for a single hop
  measurement, the traditional measurement methods MUST be used i= nstead
  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.<= /o:p>

 

Thanks,

Alissa



=
=3D 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 co= rrected.

Please also use [TBDX] for values you expect to be allocated by IANA.

[haibin] ok.

=3D=3D Nits =3D=3D


=3D General


The document seems to repeatedly explain that it is doing things "as s= pecified 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.

=3D 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, transp= ort, NAT traversal, and other RELOAD protocol features.

=3D 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/

=3D Section 4.3:

OLD
We define a simple PathTrack method for retrieving diagnostic
  information iteratively.  The mechanism defined in this do= cument
  follows the RELOAD specification, the new request and response<= br>   message use the message format specified in RELOAD messages. &n= bsp;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.

=3D Section 4.3.1.2:

s/after that/after receiving a PathTrackAns where the next_hop node ID= equals the responding node ID/

=3D Section 4.3.2.1:

s/PathTrack Response/PathTrack response/

=3D Section 4.4:

s/code. and we define/code. We define/

=3D Section 5:

s/Path_track methods/PathTrack methods/

=3D Section 5.1:

s/Note that is NOT/Note that it is not/

=3D Section 5.2:

s/Note that is NOT/Note that it is not/

s/consists one or more/consists of one or more/

=3D Section 6.1:

s/`MAY/MAY/

=3D Section 6.2:

s/return the response the initiator node/return the response to the initiat= or node/

s/The peer SHOULD/The intermediate peer SHOULD/

[haibin] Thanks again.

BR,
-Haibin Song


_______________________________________________
P2PSIP mailing list
P2PSIP@ietf.org
https://www.ietf.o= rg/mailman/listinfo/p2psip

 

_______________________________= ________________
P2PSIP mailing list
P2PSIP@ietf.org
https://www.ietf.o= rg/mailman/listinfo/p2psip

 

--_000_E33E01DFD5BEA24B9F3F18671078951F653A1614nkgeml501mbschi_--