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

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

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.

On Fri, Aug 14, 2015 at 6:33 PM, Alissa Cooper <alissa@= cooperw.in> wrote:

I have reviewed this document in preparati= on for IETF last call. Before proceeding to last call, I have some comments= and questions that I=E2=80=99d like to discuss. I=E2=80=99ve also included= a list of nits that should be resolved together with any last call comment= s.

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

=3D General=C2=A0
=
I think this document should be listed as updating RFC 6940 = in the document header.

The document contains=C2= =A0a disclaimer for pre-RFC5378 work, = which I assume is in error. If using xml2rfc to generate the text, using th= e most up-to-date version of xml2rfc to create the next version should fix = this.

=3D Section 4.3:

=
"There have be= en proposals that RouteQuery and a series of Fetch
   reque=
sts 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=E2=80=99t explain why PathTrack is being defin= ed rather than peers using RouteQuery plus Fetch. Furthermore, the document= does not explain how peers are supposed to interpret the results of the Pa= thTrack method in the event that the route does change in between steps. Th= is seems like a substantial omission.

In general, = this document provides very little information about how peers are expected= to interpret and use the diagnostic information. I understand that peers c= an 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 o= paque. I would suggest providing a little more context, at the very least b= y moving the examples into the body of the draft somewhere.=C2=A0
=3D Section 4.4:

I=E2=80=99m a lit= tle confused about the values given for the error codes. Do you want IANA t= o 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.
=
This section also says:

"In ad= dition, this document introduces several types of error information in the = error_info field in the case of Code 0x65. ... Here are some examples for t= he 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=E2=80=99m not sure what is meant by al= l of this text. As defined in RFC 6940, error_info is an optional implement= ation-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 inc= luded for each string. And the text about the other codes = seems redundant with how error_info is already defined in RFC 6940.<= /span>

=3D Section 5.2:

<= /span>
"= ext_length : the length of the returned DiagnosticInfo information
      in bytes.  If the value is greater than or equal to 1, then som=
e
      extended diagnostic information is requested."
This is defining the diagnostic response =E2=80=94 why would the ex= tension 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.
<= div>
Also, in the struct the diagnostic info is called =E2=80= =9Cdiagnostic_info_contents=E2=80=9D but in the text= it is called=C2=A0=E2=80=9Cdiagnostic_infor= mation.=E2=80=9D These should be the same I think.
=3D Section 5.3:

I=E2=80=99m= concerned about the way STATUS_INFO is defined. First, why are there 16 di= fferent 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=E2=80=99s level 7 actually mean it is less congested = than another peer=E2=80=99s level 3, and won=E2=80=99t that make the inform= ation 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.
=
SOFTWARE_VERSION doesn=E2=80=99t rea= lly 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 a= s a diagnostic?

Also, doesn=E2=80=99t 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?
=

The definition= s of=C2=A0EWMA_BYTES_SE= NT and=C2=A0EWMA_BYTES_= RCVD seem problematic.
sent =3D alpha x s=
ent_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 side= s. It would be clearer to use last_sent and last_rcvd or some such on the r= ight-hand side of these equations. But this begs some bigger questions:

- Doe= s 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?

Finally, I have troubl= e 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 ce= rtain networks. What is the justification for only making some of them subj= ect to access control?

<= div>=3D Section 6.1:
=E2=80=9CThe=C2=A0destination field MUST be set to the desired= destination, which MAY=C2=A0be either a NodeId or ResourceID but SHOULD NOT be the broadcast=C2= =A0NodeID."=

What is the expected behavior if a Ping or a Path= Track does get sent to the broadcast NodeID? Or should these requirements r= eally be MUST NOTs?

=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?

=
=3D Section 9:

Please only use either R= FC-XXXX or RFC-AAAA but not both as placeholders.

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

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

<= /span>
=3D General
The document seems to repeatedly explain that it is doing things =E2=80=9Cas = specified in RFC6940.=E2=80=9D I don=E2=80=99t think this necessary in many= cases. It=E2=80=99s worth doing a pass through and removing the places whe= re it=E2=80=99s obvious that this spec is building on RFC 6940.

=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 R=
ELOAD protocol features.
=3D Section 4.2.1:

= s/as specified in Table= 4 and=C2=A0specified i= n the RELOAD/as specifi= ed in Table 4/

s/new requests of the the/new requests of the/

<= div>=3D Section 4.3:

= OLD
We defi=
ne 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 i=
n Figure 2.
=3D Section 4.3.1.2:

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

<= span style=3D"font-size:13.3333330154419px">=3D Section 4.3.2.1:

s/Pat= hTrack Response/PathTra= ck response/
=
=3D Sect= ion 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 t= hat is NOT/Note that it= is not/

=
=3D Sec= tion 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 Sectio= n 6.2:

s/return the response the initiator n= ode/return the response= to the initiator node/

s/The peer SHOULD/The intermediate peer SHO= ULD/

_________________________________= ______________
P2PSIP mailing list
P2PSIP@ietf.org
https://www.ietf.org/mailman/listinfo/p2psip

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

Co= de 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

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.