Hi Magnus,
 
Thanks for your comments.
 
Here are our responses (with input also from David Black and Al Morton).
 
Please let us know of any further comments or questions.
 
Thanks,
Regards,
Jerry
 
> 
> --- On Fri, 10/2/09, Magnus Westerlund <magnus.westerlund@ericsson.com> 
> wrote:
> 
> 
> From: Magnus Westerlund <magnus.westerlund@ericsson.com>
> Subject: [NSIS] AD comments on draft-ietf-nsis-qspec-21
> To: "NSIS" <nsis@ietf.org>, draft-ietf-nsis-qspec@tools.ietf.org
> Date: Friday, October 2, 2009, 5:32 AM
> 
> 
> Hi,
> 
> Very sorry for the long delay in getting this review completed.
> 
> 1. Section 1. last paragraph: I don't think "worked example" is easily
> understood. Can you find another word?
 
Will substitute "an example" for "a worked example".

> 2. Section 3.1:
> - QSPEC parameter behavior for new QSPEC parameters the QOSM
>     behavior (tear down preempted reservation) is not followed (see
>     specification defines
>   - define what happens in case of preemption if the default QNI
>     Section 4.3..5)
> 
> Some editing issues here around the second parenthesis in the first
> bullet. It closes in the second bullet.
 
These 2 bullet items should read as follows:
 
"  - QSPEC parameter behavior for new QSPEC parameters the QOSM
     specification defines
   - define what happens in case of preemption if the default QNI
     behavior (tear down preempted reservation) is not followed (see
     Section 5.3.5)
"

> 3. Section 3.2:
> "QSPEC
>   parameters are the parameters appearing in a QSPEC, which must
>   include traffic (TMOD),  ..."
> 
> I do believe there is a missing word between traffic and "(TMOD).
 
Will change "include traffic (TMOD)" to "include the traffic model parameter (TMOD)"

> 4. Section 3.2, second paragraph:
> 
> "IANA
>   assigns a new QSPEC version number when changes that are not
>   backwards compatible are made to the QSPEC and this document is
>   reissued. "
> 
> and
> "Later QSPEC versions MUST be
>   backward compatible with earlier QSPEC versions."
> 
> The two sentences are contradicting. Please make it clear when new
> version is needed.
 
Will change 
"IANA assigns a new QSPEC version number when changes that are not backwards compatible are made to the QSPEC and this document is reissued."
to
"IANA assigns a new QSPEC version number when the current version is deprecated or deleted (as required by a specification)."

> 5. Section 3.3.1: The 4 sub-parameters can you please specify units for
> them?
 
   o rate (r) specified in bytes per second 
   o bucket size (b) specified in bytes 
   o peak rate (p) specified in bytes per second 
   o minimum policed unit (m) specified in bytes 

> 6. Section 3.3.1: You don't seem to have a definition of what "minimum
> policed unit" is. I assume that this definition from RFC 2211 is
> applicable, but still either include it or reference it.
>
>   The minimum policed unit, m, is an integer measured in bytes.  All IP
>   datagrams less than size m will be counted against the token bucket
>   as being of size m."
 
Will include the above definition and also reference RFC 2211.

> 7. Section 3.3.1: What is the definition of peak rate? I understand that
> the "rate" is a uniform filling rate of the token bucket. However, peak
> rate implies that it is not a smoothed value nor has its own bucket
> depth parameter. So please provide a definition.
 
The following definition given in RFC 2212 will be added and referenced:
" The peak rate is the maximum rate at which the source and any
   reshaping points (reshaping points are defined below) may inject
   bursts of traffic into the network.  More precisely, it is a
   requirement that for all time periods the amount of data sent cannot
   exceed M+pT where M is the maximum datagram size and T is the length
   of the time period.  Furthermore, p MUST be greater than or equal to
   the token bucket rate, r.  If the peak rate is unknown or
   unspecified, then p MUST be set to infinity."

> 8. Section 3.3.2: " This delay
>   results from the combination of speed-of-light propagation delay,"
> 
> Wouldn't it be clearer to say "link" instead of speed-of-light.
> Especially in cases where the link has retransmission or other features
> that makes the links propagation delay not solely be dependent on the
> speed-of-light in the medium used.
 
Agree, will substitute "link" for "speed-of-light".

> 9. Section 3.3.2: "Furthermore, the
>   QNI MUST add the propagation delay of the ingress link, if this link
>   exists."
> 
> How, is the QNI supposed to know the propagation delay of the ingress
> link? I agree that there are cases where this can be configured, but
> there appear to be a number of cases when it would not be the case.
> Therefore I am a bit questioning a MUST on something that may not be
> possible to perform.
 
Agree, it must be configured.  Will substitute "SHOULD" for "MUST".

> 10. Section 3.3.2: What is the definition of path error ratio?
 
Per the definition in Y.1540 and Y.1541:
"Packet error ratio is the ratio of total errored IP packet outcomes to the total of successful IP packet transfer outcomes plus errored IP packet outcomes in a population of interest, with a resolution of at least 10–9.  If lesser resolution is available in a value, the unused digits shall be set to zero."
Will also add that the number of errored packets observed is directly related to the confidence in the result.  
 
> 11. This may seem out of context but I do want to know the answer to how
> the situation is handled when QNI and QNR are neighbors on a non QoS
> enabled network.
 
What "situation" are you referring to here?  IMO you can't have a QNI and QNR in a "non QoS enabled network".  By definition, a QNI and QNR are only defined in an NSIS enabled network (i.e., a QoS enabled network).

> 12. Section 4.2.1:
> "Note that
>   the TMOD parameter and all QSPEC parameters with the M flag set MUST
>   be examined by the RMF, and all QSPEC parameters with the M flag not
>   set SHOULD be examined by the RMF, and appropriately flagged."
> 
> What is meant with appropriately flagged in the above sentence?
 
It refers to setting the E flag (which is described in the sentence just before the one you quote above in the current qspec document).  The above sentence will be further clarified as follows:
"the TMOD parameter and all QSPEC parameters with the M flag set MUST be examined by the RMF, and all QSPEC parameters with the M flag not set SHOULD be examined by the RMF, and the E flag set to indicate whether the parameter could or could not be satisfied."
 
> 13. Section 5.1.1: Even if version numbers are assigned by IANA I think
> the spec should be explicit on which version this one defines. Also as
> being the first version, I think assigning it yourself would be fully
> appropriate..
 
QSPEC Version 0 is assigned in Section 7 (IANA Considerations), as follows:
 
"   QSPEC Version (4 bits):
   The following value is allocated by this specification: 
   0: assigned to Version 0 QSPEC" 
  
Will also note this in Section 5.1.1. 

> 14. Section 5.1.1.: QSPEC Type: I think this should have a pointer to
> the IANA registry.
 
OK.
 
> 15. Section 5.1.1:
> "   Length: The total length of the QSPEC excluding the common header"
> 
> Please make it clear that you count in 32-bit words (?) and add the "."
> at the end of the sentence.
 
"Length" is defined at the beginning of Section 5.1 as follows:
"   o Length has the units of 32-bit words, and measures the length of
     Value.  If there is no Value, Length=0.  The Object length 
     excludes the header." 
  
The sentence you quote above in Section 5.1.1 will also be clarified as follows: 
"   Length: The total length of the QSPEC (in 32-bit words) excluding the common header"
 
> 16. Section 5.1.2: "Parameter ID: Assigned to each parameter (see below)"
> 
> The text below this does not discuss parameter ID in any useful way.
> Please correct that or provide a better reference to where it is discussed. 
  
Will clarify this sentence as follows: 
"Parameter ID: Assigned consecutively to each QSPEC parameter (parameter ID's are assigned to each QSPEC parameter defined in this document in Section 5.2 below and in Section 7/IANA Considerations)"

> 17. Section 5.2.1:
> "he <TMOD> parameters are represented by three floating point
>   numbers in single-precision IEEE floating point format followed by
>   one 32-bit integer in network byte order."
> 
> Please add a reference for the IEEE floating point format. 
  
This is Reference [IEEE754] now given in Section 11 (Informative References).  Will give this reference also in the above sentence in Section 5.2.1.

> 18. Section 5.2.1: I don't think the reference in the section title is
> particular helping. I definitely prefer explicit references for each
> item so that you easily can find them. There is also a risk for
> confusion if multiple references are not agreeing on a parameter
> definition. For example take the "minimum policed unit". What it is is
> defined in RFC 2210, but there is also redundant interpretation text in
> RFC 2215. I also have a hard time determining if any reference is missing. 
  
OK, will move the references into each section, and avoid redundant references.

> 19. Section 5.2.3: The referenced documents does not define Path
> latency, only minimal path latency. Not obvious that they are the same,
> for example from a perspective of queuing delay or link ARQ. 
  
The intention of the Path Latency parameter is the same as the Minimal Path Latency parameter defined in Section 3.4 of RFC 2215.  Note that it also says in RFC 2215: 
  
"The purpose of this parameter is to provide a baseline minimum path
   latency for use with services which provide estimates or bounds on
   additional path delay, such as Guaranteed [RFC 2212]..  Together with
   the queuing delay bound offered by Guaranteed and similar services,
   this parameter gives the application knowledge of both the minimum
   and maximum packet delivery delay." 
  
This is also the intent of the parameter for use in the QSPEC.

> 20. Section 5.2.4: Once more the references are unclear. Especially as
> RFC 3393 doesn't use "jitter". You need to be explicit about what is what. 
  
It will be clarified that "jitter" is called "IP Delay Variation" in RFC 3393.  This is actually already stated in Section 3.3.2, but will be reiterated in Section 5.2.4. 

> 21. Section 5.2.5: First of all I am not convinced by looking in Y.1541
> that packet loss ratio has a clear definition. I am missing what type of
> averaging is assumed in the value. If you going to measure this value
> you need to do it over sufficiently many packets, measuring over 7
> packets with one loss doesn't say much. 
  
An evaluation interval of 1 minute is suggested in Y.1541.  This will be noted in Section 5.2.5.  Will also add that the number of losses observed is directly related to the confidence in the result.

> 22. Section 5.2.5: " An
>   individual QNE can advertise a PLR value between zero and 10^-2
>   and the total PLR added across all QNEs can range as high as 10^-1."
> 
> Isn't the restriction to advertise a link PLR no higher than 10^-2 a to
> strict limit. Doesn't there exist links out there that has that level of
> loss. Maybe not ones that you usually request QoS over, but anyway. 
  
The maximum limit of 10^-2 on a QNE's local PLR value and the maximum limit (clamp value) of 10^-1 on accumulated end-to-end PLR value are used to preserve the accuracy of the simple additive accumulation function specified in the QSPEC.  It's important to avoid more complex accumulation functions and stay with the simple additive accumulation function (Y.1541 mentions the different methods in section 8.2.2 and 8.2.3).  Furthermore, if these maximums are exceeded, then (as you say) the path would not have much quality and would likely not meet the QoS objectives anyway.  Finally the word "add" should replace the word "advertise" and the quoted sentence further clarified as follows:
"An individual QNE adds its local PLR value (up to a maximum of 10^-2) to the total path PLR value (up to a maximum of 10^-1) , where the acceptability of the total path PLR value added across all QNEs is determined based on the QOSM being used."  

> 23. Section 5.2.11:
> "When the shaping causes
>   unbounded queue growth at the shaper traffic can be dropped."
> 
> To me the shaping seems unclear, as it contains a unknown factor of the
> queue used. Both queue depth and dropping model would influence how the
> shaping happens. 
  
Actually, the bucket size in the TMOD parameter for excess traffic specifies the queuing behavior that is of concern.  The traffic in question here is not only out-of-profile wrt the primary traffic spec, but it's but it's *also* out of profile wrt to a secondary traffic spec for excess traffic.  However, we see no problem with saying that any traffic that is excess of the TMOD for excess traffic MAY be dropped (or even SHOULD be dropped), and will rephrase the sentence in question as such. 
  
> 24. Section 5.2.11:
> Remark Value (6 bits): indicates either drop (set to 0) or DSCP
>                          value [RFC2474] to remark packets to when
>                          identified as excess
> 
> So you can't remark traffic into the best-effort traffic class? Isn't
> that a severe limitation? And why are there need for two ways of marking
> drop as excess treatments? 
  
Correct.  The usual DSCP for best-effort is zero, but the meaning of that value has been changed to drop in the quoted text.  That makes it impossible to remark to a DSCP of zero, which is usually the best-effort traffic class.  However, since "drop" is already encoded in the Excess Treatment value, as you point out, we will remove the ability to encode "drop" here, specifically, remove the words "either drop (set to 0) or" .

> 25. Section 5.2.12:
> 
>   0                   1                   2                   3
>    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>   |M|E|N|r|           12          |r|r|r|r|          1            |
>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>   | DSCP      |0 0 0 0 0 0 0 0 0 0|            (Reserved)         |
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> 
> Two comments about the field definition for the third 16-bit word.
> 
> First, why is one of the possible field value represented rather than
> giving the field a name. That way one can make it clear that the
> possibility for different structures in that 16-bit word. I would
> probably have defiend it as:
> 
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
> | PH Field Value            |EN |
> 
> I do understand that would be different from how RFC 3140 is
> representing the information, but be equivalent. As an alternative I
> would simple change the first diagram to say instead of DSCP   | 0 0 ...
> to say PHB Field. Anyway, I think the current representation is unclear. 
  
What you suggest is consistent with RFC 3140 and a reasonable way to do this.  We will substitute the 16 bits you suggest for the DSCP + 10 bits of zeroes, and explain that the result matches RFC 3140 and that the following four figures show 4 possible formats based on the value of the EN field.
 
> 26. Section 7. There are registries that requires standard action for
> new registrations. I think that is not consistent with publishing this
> as experimental. Please address. 
  
First, the QSPEC is an *Informational* document, not experimental.  In any case, we will change "Standards Action" to "Specification Required" in each case. 

> 27. Section 7.. QSPEC Procedures. This table is so confusing, I can't get
> my head around what code point space is registered. And if there are a
> second dimension for certain values and in that case which that is. 
  
The QSPEC Procedures object being registered by IANA specifications in Section 7 is specified in Section 5.1.1 (Common Header Format).  The code point space consists of a Message Sequence sub-parameter and an Object Combination sub-parameter.  Section 4.3 explains the Message Sequence and Object Combination sub-parameters.  Object Combinations 0, 1, and 2 are explained in Section 4.3.1, 4.3.2, and 4.3.3, respectively.  Tables 1, 2, and 3 in Section 4.3 assign the Message Sequence ID to Object Combinations 0, 1, and, 2, respectively.  There is an error to be corrected in the IANA specifications: Object Combination 2 (Section 4.3.3) has only one Message Sequence ID, not 3 (this will be corrected.  Hopefully this explains whatever confusion there is here. 
 
> 28. References: Can you please ensure that there is a blank line between
> each reference. Now it is very difficult to read the list. 
  
OK 
  
> Also, making
> the reference using a hanging style makes it easier to find the
> reference identifiers. 
  
Not sure what you mean by "hanging style"?  Do you mean to indent the references, e.g., 
  
[3GPP-1] 3GPP TS 22.067 V7.0.0 (2006-03) Technical Specification, 3rd
               Generation Partnership Project; Technical Specification Group
               Services and System Aspects; enhanced Multi Level Precedence and
               Preemption service (eMLPP) - Stage 1 (Release 7). 
 
If so, OK.
 
> 29. Appendix B has alignment issues in the Protocol header length table. 

  
OK, will fix 

> 
> Cheers
> 
> Magnus Westerlund
> 
> IETF Transport Area Director
> ----------------------------------------------------------------------
> Multimedia Technologies, Ericsson Research EAB/TVM
> ----------------------------------------------------------------------
> Ericsson AB                | Phone  +46 10 7148287
> Färögatan 6                | Mobile +46 73 0949079
> SE-164 80 Stockholm, Sweden| mailto: magnus.westerlund@ericsson.com
> ----------------------------------------------------------------------