Hi Greg,
Thank you for the answer and for addressing my comments. All your explanations are clear and satisfactory, and the proposed edits are fine with me.
Regards,
Dan
On Thu, Jul 2, 2020 at 10:22 PM Greg Mirsky <gregimirsky@xxxxxxxxx> wrote:
Hi Dan,thank you for your review, detailed questions, and helpful suggestions. Please find my answers and notes below tagged GIM>>.Regards,GregOn Mon, Jun 29, 2020 at 8:02 AM Dan Romascanu via Datatracker <noreply@xxxxxxxx> wrote:Reviewer: Dan Romascanu
Review result: Ready with Issues
I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please treat these comments just
like any other last call comments.
For more information, please see the FAQ at
<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
Document: draft-ietf-ippm-stamp-option-tlv-06
Reviewer: Dan Romascanu
Review Date: 2020-06-29
IETF LC End Date: 2020-07-06
IESG Telechat date: Not scheduled for a telechat
Summary: Ready with issues
This is a clear, well-written document. There are a few minor issues that would
benefit from clarifications and possible edits before approval.
Major issues:
Minor issues:
1. Section 3. Is there any recommended strategy to generate SSIDs? Are these
supposed to be generated sequentially? Randomly? How soon is the 16 -bit space
supposed to wrap-up? Some clarification would be useful I believe.GIM>> Because test sessions, in general, will be performed for different periods of time, implementation will need to manage the pool of available identifiers. I agree, the initial allocation may use sequential ascending increment by one method, but at some point, it will be "get-the-next-available number". I propose to update the text as follows:OLD TEXT:
A STAMP
Session-Sender MAY generate a locally unique STAMP Session Identifier
(SSID). SSID is two octets long non-zero unsigned integer.
NEW TEXT:
A STAMP
Session-Sender MAY generate a locally unique STAMP Session Identifier
(SSID). SSID is two octets long non-zero unsigned integer. SSID generation
policy is implementation-specific. For example, sequentially ascending
incremented by one method could be used for the initial allocation of SSID.
Because of test sessions lasting different time an implementation that uses
SSID MUST monitor the pool of available identifiers. An implementation
SHOULD NOT assign the same identifier to different STAMP test sessions.
2. Section 4.5 - how is the value Session-Sender Tx counter (S_TxC) determined
by the sender?GIM>> The value of S_TxC is the current value of the transmitted in-profile packets. Would the following update (also addressing the #3) make it clearer?OLD TEXT:o Session-Sender Tx counter (S_TxC) is four octets long field.
o Session-Reflector Rx counter (R_RxC) is four octets long field.
MUST be zeroed by the Session-Sender and filled by the Session-
Reflector.
o Session-Reflector Tx counter (R_TxC) is four octets long field.
MUST be zeroed by the Session-Sender and filled by the Session-
Reflector.NEW TEXT:o Session-Sender Tx counter (S_TxC) is four octets long field. The
Session-Reflector MUST set its value equal to the number of the
transmitted in-profile packets..
o Session-Reflector Rx counter (R_RxC) is four octets long field.
MUST be zeroed by the Session-Sender on transmit and ingored by
the Session-Reflector on receipt. The Session-Reflector MUST fill
it with the value of in-profile packets received.
o Session-Reflector Tx counter (R_TxC) is four octets long field.
MUST be zeroed by the Session-Sender and ignored by the Session-
Reflector on receipt. The Session-Reflector MUST fill with the
value of the transmitted in-profile packets.
3. Section 4.5 - (R_RxC) and (R_TxC) MUST be zeroed by the Session-Sender - Is
this verified at reception? What happens if a Session-Reflector detects a
non-zero value in one of these fields?GIM>> Please let us know if the update above addresses your concern.
4. Section 4.6 - it seems that understanding [TS23501] is needed to properly
implement this section and interpret the content of the TLV. Should not this
reference be Normative rather than Informative?GIM>> Agreed and moved it to the list of Normative References
5. Section 5.2 - as the values for Synchronization Sources in Table 4 refer to
'this document', it seems to be necessary to include in this document
references to the documents that define the respective terms / sourcesGIM>> The only convenient place for references I see is in the Acronyms section. Would you suggest another section in the document? Besides the location, some of the listed sources of synchronization do not have a standard specification, e.g. BITS/SSU, or the specification is not easily available, e.g., Russian government's GLONASS. Some systems, like LORAL-C, are in the process of being decommissioned and only a few LORAL transmitters remain operational. Would adding references to NTP and PTP in the Acronyms section be acceptable?BITS Building Integrated Timing Supply
CoS Class of Service
DSCP Differentiated Services Code Point
ECN Explicit Congestion Notification
GLONASS Global Orbiting Navigation Satellite System
GPS Global Positioning System [GPS]
HMAC Hashed Message Authentication Code
LORAN-C Long Range Navigation System Version C
MBZ Must Be Zero
NTP Network Time Protocol [RFC5905]
PMF Performance Measurement Function
PTP Precision Time Protocol [IEEE.1588.2008]
TLV Type-Length-Value
SSID STAMP Session Identifier
SSU Synchronization Supply Unit
STAMP Simple Two-way Active Measurement Protocol
6. Section 6 - Security Considerations: Is not sending of test packets to a
reflector that does not support SSID a potential sourse for DoS attacks?GIM>> A Session-Reflector that does not support SSID would transmit reflected test packet with SSID field zeroed. The local to the Session-Sender policy will control whether the Session-Sender stops or continues the test session.Same
question about sending packets with unsupported TLV types. How do Reflectors
protect against such situations? As such attacks would be beyond STAMP base
specifications, it may be good to discuss these.GIM>> A Session-Reflector that does not support STAMP extensions is not expected to compare the value in the Length field of the UDP header and the length of the STAMP base packet. Hence the Session-Reflector will transmit the base STAMP packet. It is the local policy on the Session-Sender (similar to the handling of SSID == 0 situation) that will control the Sender's behavior. I think the text might be appended to the second paragraph of Section 4. The updated paragraph is below:A STAMP node, whether Session-Sender or Session-Reflector, receiving
a test packet MUST determine whether the packet is a base STAMP
packet or includes one or more TLVs. The node MUST compare the value
in the Length field of the UDP header and the length of the base
STAMP test packet in the mode, unauthenticated or authenticated based
on the configuration of the particular STAMP test session. If the
difference between the two values is larger than the length of UDP
header, then the test packet includes one or more STAMP TLVs that
immediately follow the base STAMP test packet. A Session-Reflector
that does not support STAMP extensions is not expected to compare the
value in the Length field of the UDP header and the length of the
STAMP base packet. Hence the Session-Reflector will transmit the
base STAMP packet. It is the local policy on the Session-Sender
(similar to the handling of SSID == 0 scenario described in
Section 3) that will control the sender's behavior.
Nits/editorial comments:
1. Section 2.1 - it's more convenient for future users of the document if
acronyms were listed in alphabetical orderGIM>> Agree. Done (please check it above).
2. Sections 4.6, 4.7 - inconsistent use of capitalization:
Reserved - ... must be zeroed on transmission
and ignored on receipt.
It's a 'must' in 4.6, and a 'MUST' in 4.7GIM>> Thank you for pointing it out. I've found two cases of "must" that changed to normative-style.
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
