Re: [Gen-art] Gen-ART review of draft-ietf-ippm-6man-pdm-option: Minor and editorial

["jouni.nospam" <[e-mail]>]

Hi,

Thank you for the updates. I am fine with them.

I still have one fundamental complaint. There is no discussion or definition of the message timestamp points. All this nanosecond or better accuracy, and the concern of truncation inaccuracy do not really matter if the message timestamp point varies node to node or even within the node. I suggest taking a look at standards like IEEE 802.1AS, IEEE 1588v2 or IEEE 1722 for discussions of this topic. You really need to have a good definition & discussion of the message timestamp point, or justify that in the text why it is not there.

Regards,
	Jouni



> On Feb 28, 2017, at 7:04 AM, [e-mail] wrote:
> 
> 
> 
> Jouni,
> 
> We have posted a new version which addresses the minor and editorial comments. It is below.  I am waiting for your response to the major comments & then I will integrate.   As far as I know now, I have addressed all outstanding issues.   My comments to your editorial / minor is below.
> 
> https://datatracker.ietf.org/doc/draft-ietf-ippm-6man-pdm-option/
> 
> 
> Nits/editorial comments: 
> 
>> 1) Section 1.4 numbered list: add missing full stops. 
> 
> Actually, I think it is better to remove the full stops from the two that have it.  These are sentence 
> fragments. 
> 
>> 2) Section 3.2: remove 
> 
>> "The 5-tuple consists of 
> 
>>  the source and destination IP addresses, the source and destination 
> 
>>  ports, and the upper layer protocol (ex. TCP, ICMP, etc)." 
> 
>> since this is unnecessary repetition. 
> 
> 
> This is in the section below: 
> 
> Old 
> ---- 
> Packet Sequence Number This Packet (PSNTP) 
> 
> 16-bit unsigned integer.  This field will wrap. It is intended for 
> use while analyzing packet traces. 
> 
> Initialized at a random number and incremented monotonically for each 
> packet of the session flow of the 5-tuple.  The 5-tuple consists of 
> the source and destination IP addresses, the source and destination 
> ports, and the upper layer protocol (ex. TCP, ICMP, etc).   The 
> random number initialization is intended to make it harder to spoof 
> and insert such packets. 
> 
> New 
> ---- 
> Packet Sequence Number This Packet (PSNTP) 
> 
> 16-bit unsigned integer.  This field will wrap. It is intended for 
> use while analyzing packet traces. 
> 
> Initialized at a random number and incremented monotonically for each 
> packet of the session flow of the 5-tuple. 
> 
> 
>> 3) Section 3.2: remove 
> 
>> "Operating systems MUST NOT implement a single 
> 
>> counter for all connections." 
> 
>> Seems again like unnecessary repetition to previous sentence. 
> 
> 
> This is in the section on "Packet Sequence Number This Packet (PSNTP)" 
> 
> Old 
> --- 
> Operating systems MUST implement a separate packet sequence number 
> counter per 5-tuple. Operating systems MUST NOT implement a single 
> counter for all connections. 
> 
> New 
> --- 
> Operating systems MUST implement a separate packet sequence number 
> counter per 5-tuple. 
> 
> 
>> 4) Section 3.2 again unnecessary repetition of IPv6 basics that can be read from RFC2460. Suggest strongly to remove: 
> 
>> "This indicates the following processing requirements: 
> 
>>  00 - skip over this option and continue processing the header. 
> 
>>  RFC2460 [RFC2460] defines other values for the Option Type field. 
> 
>> These MUST NOT be used in the PDM." 
> 
>> and 
> 
>> "The possible values are as follows: 
> 
>> 0 - Option Data does not change en-route 
> 
>>  1 - Option Data may change en-route 
> 
>>  The three high-order bits described above are to be treated as part 
> 
>>  of the Option Type, not independent of the Option Type.  That is, a 
> 
>> particular option is identified by a full 8-bit Option Type, not just 
> 
>> the low-order 5 bits of an Option Type." 
> 
> 
> Old 
> ---- 
> Option Type 
> 
> The two highest-order bits of the Option Type field are encoded to 
> indicate specific processing of the option; for the PDM destination 
> option, these two bits MUST be set to 00. This indicates the 
> following processing requirements: 
> 
> 00 - skip over this option and continue processing the header. 
> 
> RFC2460 [RFC2460] defines other values for the Option Type field. 
> These MUST NOT be used in the PDM. 
> 
> In keeping with RFC2460 [RFC2460], the third-highest-order bit of the 
> Option Type specifies whether or not the Option Data of that option 
> can change en-route to the packet's final destination. 
> 
> In the PDM, the value of the third-highest-order bit MUST be 0.  The 
> possible values are as follows: 
> 
> 0 - Option Data does not change en-route 
> 
> 1 - Option Data may change en-route 
> 
> The three high-order bits described above are to be treated as part 
> of the Option Type, not independent of the Option Type.  That is, a 
> particular option is identified by a full 8-bit Option Type, not just 
> the low-order 5 bits of an Option Type. 
> 
> 
> 
> New 
> ---- 
> Option Type 
> 
> In keeping with RFC2460[RFC2460], the two high order bits of the Option Type field are encoded to 
> indicate specific processing of the option; for the PDM destination 
> option, these two bits MUST be set to 00. 
> 
> The third high order bit of the Option Type specifies whether or not the Option Data of that option 
> can change en-route to the packet's final destination. 
> 
> In the PDM, the value of the third high order bit MUST be 0. 
> 
> 
> 
> 5) Section 3.3 same as in comment 4). Suggest strongly removing: 
> 
>> "This follows the order defined in RFC2460 [RFC2460] 
> 
>>               IPv6 header 
> 
>>              Hop-by-Hop Options header 
> 
>>             Destination Options header  <-------- 
> 
>>            Routing header 
> 
>>           Fragment header 
> 
>>          Authentication header 
> 
>>         Encapsulating Security Payload header 
> 
>>        Destination Options header <------------ 
> 
>>       upper-layer header" 
> 
> 
> 
>> 6) Suggest removing entire Section 3.4 and moving the following text to Section 3.3: 
> 
>> "PDM MUST be placed before the ESP header in 
> 
>> order to work.  If placed before the ESP header, the PDM header will 
> 
>> flow in the clear over the network thus allowing gathering of 
> 
>> performance and diagnostic data without sacrificing security." 
> 
> 
> ESP processing with PDM was changed because of comments from security reviewer. 
> 
> 
> 
>> 7) Section 3.6 suggest removing the following text. I see no value it would add to what has already been said: 
> 
>> "As with all other destination options extension headers, the PDM is 
> 
>>  for destination nodes only. As specified above, intermediate devices 
> 
>>  MUST neither set nor modify this field." 
> 
> 
> Done 
> 
> 
>> 8) Section 3.6 suggest removing the following 5-tuple text as it has already been described earlier in Section 2: 
> 
>> "The 5-tuple is: 
> 
>>  SADDR : IP address of the sender SPORT : Port for sender DADDR : IP 
> 
>>  address of the destination DPORT : Port for destination PROTC : 
> 
>>  Protocol for upper layer (ex. TCP, UDP, ICMP)" 
> 
> Done 
> 
> 
>> 9) Sections 4.2 and 4.3 suggest removing them entirely. I see what value these sections add. I acknowledge they are good 
>> to know information of timer hardware implementation difference but do not really add value on the on-wire encoding of the PDM option. 
> 
>> 10) Section 4.4 suggest removing the entire section. Time Base was already described in detail enough in Section 3.2. 
> 
>> 11) Section 4.5 time base for picoseconds is 11 not 00. 
> 
>> 12) Section 4.5 suggest removing the following text, since it does not add any more clarity to what has already been said in my opinion. This is because all the examples follow nice nybble increment in scaling: 
> 
>> "Sample binary values (high order 16 bits taken) 
> 
>>  1 psec            1                                              0001 
> 
>>  1 nsec          3E8                                    0011 1110 1000 
> 
>>  1 usec        F4240                          1111 0100 0010 0100 0000 
> 
>>  1 msec     3B9ACA00           0011 1011 1001 1010 1100 1010 0000 0000 
> 
>>  1 sec    E8D4A51000 1110 1000 1101 0100 1010 0101 0001 0000 0000 0000" 
> 
>> 12) Section 4.6 I do not understand why this section is here. I strongly suggest removing it. Sections 4.5 and 3.2 already describe how I would encode the delta time using scaling as a separate fields not embedded (option fields ScaleDTLR and ScaleDTLS). Did I misunderstand something here? 
> 
> 
> Points 9 - 12 refer to the timing calculations.  The timebase has been fixed to be attoseconds. 
> The entire timing section has been re-written.  It has also been moved to an appendix. 
> Please let us know if it this addresses your concerns. 
> 
> 
> 
>> 13) Section 5 suggest removing the following text because of it repeating what has already been said earlier: 
> 
>> "Each packet, in addition to the PDM contains information on the 
> 
>> sender and receiver. As discussed before, a 5-tuple consists of: 
> 
>>   SADDR : IP address of the sender 
> 
>>   SPORT : Port for sender 
> 
>>  DADDR : IP address of the destination 
> 
>>   DPORT : Port for destination 
> 
>>   PROTC : Protocol for upper layer (ex. TCP, UDP, ICMP) 
> 
>> It should be understood that the packet identification information is 
> 
>> in each packet. We will not repeat that in each of the following 
> 
>> steps." 
> 
> 
> This is now in Appendix B.  It has been removed from that section. 
> 
> 
> 
>> 14) Section 5.3 suggest merging the following text into one example and do necessary rewording. There is no need to do the same 
>>  calculation twice on almost adjacent lines: 
> 
>> "Sending time : packet 2 - receive time : packet 1 
> 
>> We will call the result of this calculation: Delta Time Last Received 
> 
>> (DELTATLR) 
> 
>> That is: 
> 
>> Delta Time Last Received = (Sending time: packet 2 - receive time: packet 1)" 
> 
> Done 
> 
> 
>> 15) Expand RTT and PSN on their first use. 
> 
> Done
> 
> Thanks,
> 
> Nalini Elkins
> Inside Products, Inc.
> www.insidethestack.com
> [phone]