Re: Review of draft-ietf-nsis-qspec-18.txt

Sorry for being completely out of touch on this. I didn't even get to  
read the thread until today. I've been utterly consumed with IAB work  
and my day job. I likely won't have any spare cycles until after IETF,  
so I too would like to deeply thank Jerry for stepping in out of  
retirement (although not even Junior Seau could save the Super bowl  
for the Patriots...sigh).

I will however do my utmost to read the final version after it's  
submitted.

Best to all,

Dave.

On Feb 15, 2008, at 11:55 AM, Gerald Ash wrote:

> Martin,
>
> Many thanks for your careful review.
>
> Please see comments in line below.
>
> Please let us know of any further comments.  We'll resubmit the  
> document
> prior to cutoff date.
>
> Jerry
>
> > Hi all,
> >
> > Please find my review of draft-ietf-nsis-qspec-18.txt.
> >
> > There are some minor technical issues to fix. In general the  
> document
> > seems
> > to be ready to go for AD review,  once the editorials and technical
> > issues
> > have been fixed.
> >
> > Please address the issues and submit an updated version of the  
> draft if
> > d
> > one so. I will go ahead with the updated draft for the protocol  
> write
> > up
> > (i.e., the step before sending to Magnus and the IESG).
> >
> >  Martin
> >
> > ****
> > General
> > ****
> > I found the first part of the document (i.e., until section 5) quite
> > confusing and after section five quite clear. This does not mean  
> that
> > there
> > is an issue, as typically Internet drafts have the need to sketch  
> tech
> > things in the beginning without having the reader already at the
> > technical
> > level. I assume that the text is fine right now, as the WG has not
> > pushed to
> > change it.
>
> These sections have been rewritten many times.  Please let us know  
> if you
> have any particular suggestion to improve clarity.
>
> >
> > ****
> > Editorial:
> > ****
> >
> > - the idnits tool has found some issues (see below)
>
> OK, will address.
>
> > - Add line in memo heading with: Intended Status: Informational
>
> Although the NSIS milestone says "Informational", the ID tracker  
> lists the
> status as "Proposed Standard"
> https://datatracker.ietf.org/idtracker/draft-ietf-nsis-qspec/.  IMO  
> the
> QSPEC document should be a PS document, in close alignment with the
> NSLP PS document.  Many IANA registries are created, etc.  I can't  
> see the
> rationale for NSLP being PS and QSPEC being Informational.  I do  
> recall a
> discussion on this point earlier and several WG members expressed  
> the same
> view.  Perhaps the WG members and chairs can comment.
>
> > -  Section 1: Move to the very end of memo. It it is good to
> > acknowledge
> > other people, but it is fair enough to do it at the very end, as
> > technical
> > protocol stuff comes first.
>
> OK.
>
> > - section 2: Adding a line saying that QSPEC readers should (or  
> better
> > must?) read the QoS NSLP in advance would be advantageous for the  
> whole
> > document.
>
> Note that the first sentence of Section 2 references QoS NSLP since  
> the
> documents are closely tied.  The QSPEC document is intended to be (and
> should be) self contained so that one doesn't have to ("must") read
> something else first before reading and understanding QSPEC.   
> However, your
> point is well taken.  We can add a sentence in Section 2 such as "See
> [QoS-SIG] for further details on QoS NSLP."
>
> > - section 2, page 5, para starting with "Interoperability between  
> QoS
> > NSIS
> > entities (QNEs)..." says that parameters can be flagged or not  
> flagged.
> > I
> > suggest to put the terms "flagged"/"non flagged" in brackets and  
> write
> > mandatory and optional parameters. This makes it easier for first  
> time
> > readers.
>
> We've eliminated the terminology "mandatory" and "optional"  
> parameters.
> Parameters can be flagged by the M flag if they MUST be interpreted,  
> or
> otherwise, as follows:
>
>  M Flag: When set indicates the subsequent parameter MUST be
>            interpreted. Otherwise the parameter can be ignored if not
>            understood.
>
> We don't want to get back into the mandatory/optional terminology at  
> this
> point.
>
> > - - section 2, page 5, para starting with " A local QSPEC can be
> > defined in
> > a local domain with the initiator QSPEC encapsulated, ...". This
> > paragraph
> > is brief but describes a very powerful mechanism of the QSPEC. I  
> would
> > suggest to remove the hint the TMOD parameter and just write that  
> the
> > QSPEC
> > allows local QPSECs that are domain specific. This also helps first
> > time
> > readers to understand a basic feature of  QPSEC but not the full
> > implementation of it.
>
> OK.
>
> > - section 4.1, 2nd para, text in para starting with " New QoS NSLP
> > message
> > processing rules can only be defined in Standard...". Is it  
> required to
> > say
> > this at this part of the document at all? I remember this again  
> stated
> > in
> > the document somewhere later on at an appropriate place.
>
> It's an important point and I think it appropriate to mention it in  
> Section
> 4.1.  Furthermore, I recall extensive discussion on this sentence  
> and I
> believe it was explicitly agreed to include it here as well as later  
> in the
> document.  Also, mentioning important points more than once often  
> helps
> clarity.
>
> > - section 4.2 two paras below figure 1. Suddenly there is text about
> > RESERVE
> > and QUERY, but no hint to the QoS NSLP at all. Adding a reference to
> > the QoS
> > NSLP at this occasion would be excellent. Otherwise it  is hard to  
> see
> > where
> > those terms are coming from
>
> Good point, will do.
>
> > - section 4.3.1, para starting with " When the TMOD parameter in
> > included in
> > QoS Available, it..." should read " When the TMOD parameter are
> > included in
> > QoS Available, it..."
>
> I think the English is correct as it stands.  However, we could say  
> "When
> the TMOD sub-parameters are included ..."
>
> > - section 4.3.1. This section nevers talks about tmod-2! tmod-2 is
> > mentioned only  in sections 6.2.1 and 6.2.2.
>
> Right, good point.  We'll revise the text to refer to TMOD-1 and  
> TMOD-2 and
> explain the difference.  We could include the example of the need  
> for the
> TMOD-2 parameter in Section 4.3.1 (see discussion/example below of  
> the need
> for the TMOD-2 parameter).
>
> > - section 5, 1st and 2nd para suddenly use the E-/N-flag without  
> every
> > explaining their meaning.
>
> Well, this is where the M flag, E flag, and N flag are being  
> explained.  The
> first paragraph introduces them and there are details later.  We  
> could add
> text in the beginning such as "Three flags are used in QSPEC, the M  
> flag, E
> flag, and N flag, which are explained in this section."
>
> > - section 5.1, again the support of local QSPECs is quite powerful  
> but
> > never
> > really good explained with a figure. For instance, the first para of
> > section
> > 5.1 could refer to a simple figure showing a network configuration  
> for
> > a
> > single flow, but with indications where which QSPEC is used. This
> > instantly
> > makes the whole concept better undestandable.
>
> OK, we could add a figure analogous to Figure 3 in the Y.1541-QOSM  
> document
> http://www.ietf.org/internet-drafts/draft-ietf-nsis-y1541- 
> qosm-05.txt to
> explain the concept.
>
> > - section 5.2.5. formatting error
>
> OK.
>
> > - section 5.3.1 and related. You use tables to define cases and also
> > refer
> > to them later in the object definition. Thus it would be good to  
> label
> > those
> > tables.
>
> OK, good suggestion.
>
> > - section 5.3.5 has one formatting issue and one spelling error
> > (s/described
> > In [QoS-SIG]/ described in [QoS-SIG])
>
> OK.
>
> > - section 5.4. remove reference to NSIS extensibility document, as  
> we
> > do not
> > have it ready by now.
>
> OK.
>
> > - section 6.1. I got confused by all the different header  
> definition. I
> > would propose to split 6.1. in subsections, i.e., one for each  
> header
> > definition. This improves readability.
>
> OK, good suggestion.
>
> > - section 6.2.1: Why is there this line    <TMOD-1> = <r> <b> <p>  
> <m>
> > [RFC2210, RFC2215]? It is now required for the object definition.
>
> Right, the format is inconsistent with the other subsections and  
> will be
> revised accordingly.
>
> > - section 8, text says " Object Types (12 bits):" but should read
> > "QSPEC
> > Object Types (12 bits):".
>
> But then Section 6.1 would have to be revised to be consistent in
> terminology.  While I have no big objection to changing all of this,  
> IMO it
> should stay as is; I think it is clear that this is a QSPEC object  
> type.
>
> > - appendix b: add a note in the first line that this appendix  
> should be
> > removed by the RFC editor before publication.
>
> OK.
>
> >
> >
> >
> > ***
> > Technical
> > ***
> > - section 5.3.1, para starting with " The QSPEC parameter IDs and
> > values
> > included in the QoS Reserved...". This paragraph talks about how  
> to set
> > the
> > parameters in the RESPONSE message for case 2. It is basically fine,
> > but the
> > second sentence is for me not understandable (" For those QSPEC
> > parameters
> > that were also included in the QoS Available object in the RESERVE
> > message,
> > their value is copied into the QoS Desired object.")
> > The paragraph is talking about responses, but in RESPONSE there is  
> no
> > QoS
> > desired object anymore (at least according to the table above). I
> > probably
> > do misunderstand something here?
>
> Good catch.  The sentence needs to be corrected.  It should be: "For  
> those
> QSPEC parameters that were also included in the QoS Available object  
> in the
> RESERVE message, their value is copied from the QoS Available object  
> (in
> RESERVE) into the QoS Reserved object (in RESPONSE) ."
>
> > - section 6.1 qspec header element definition. The length  
> definition in
> > the
> > qspec header definition contradicts the length definition at the
> > beginning
> > of section 6.1 One says "total length of qspec" the other "length is
> > always
> > exept header"
>
> Very good catch!  IMO the section definition should be revised as
> follows:
>
>  Length: The total length of the QSPEC -->
>  Length: The total length of the QSPEC excluding the common header
>
> > - section 6.2.2., see also my comment above about tmod-2. Even after
> > reading
> > section 6.2.2. I do not have any clue why there two tmod parameters.
>
> This is based on discussions with David Black, who recommended the  
> second
> TMOD parameter to support DiffServ applications (as noted in Section  
> 6.2.2).
> Earlier versions of QSPEC included the following example (due to  
> David),
> which can be added back into the draft (probably in Section 4.3.1):
>
> "It is typically assumed that DiffServ EF traffic is shaped at the  
> ingress
> by a single rate token bucket.  Therefore, a single TMOD parameter is
> sufficient to signal DiffServ EF traffic.  However, for DiffServ AF  
> traffic
> two sets of token bucket parameters are needed, one token bucket for  
> the
> average traffic and one token bucket for the burst traffic.  [RFC2697]
> defines a Single Rate Three Color Marker (srTCM), which meters a  
> traffic
> stream and marks its packets according to three traffic parameters,
> Committed Information Rate (CIR), Committed Burst Size (CBS), and  
> Excess
> Burst Size (EBS), to be either green, yellow, or red.  A packet is  
> marked
> green if it does not exceed the CBS, yellow if it does exceed the  
> CBS, but
> not the EBS, and red otherwise.  [RFC2697] defines specific  
> procedures using
> two token buckets that run at the same rate.  Therefore 2 TMOD  
> parameters
> are sufficient to distinguish among 3 levels of drop precedence.  An  
> example
> is also described in the Appendix to [RFC2597]."
>
> Also, the first sentence in Section 6.2.2 should be changed slightly  
> to: ''A
> second, QSPEC <TMOD-2> parameter is specified, as could be needed for
> example to support some Diffserv applications." (added word "some").
>
> > - section 6.2.7. The slack term is 32 bits as integer. However the
> > slack
> > term must only be non-negative, i.e., it is actually an unsigned  
> int.
> > Why is
> > is not specified as unsigned int? However, the current value range
> > definition is broken, as an integer of 32 bits cannot have a maximum
> > range
> > to  2**32-1 (there are only 31 bits left...).
>
> The coding of the slack term is based on RSVP coding:
>
> RFC 2210 says:
>
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- 
> +-+
> 11  |  Slack Term [S]  (32-bit integer)                             |
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+- 
> +-+
>
> RFC 2212 says:
> "The Slack term, S, can be represented as a 32-bit integer.  Its value
>    can range from 0 to (2**32)-1 microseconds."
>
> I agree it should be specified as an unsigned integer, and the  
> notation
> revised to (2**32)-1 as the maximum, to be clear.  I believe the  
> range 0 to
> (2**32)-1 is correct for an unsigned integer.
>
> > This also holds true for
> > other
> > occasions where integer values are used (e.g. section 6.2.3!
>
> Same comments as above.
>
> > - section 6.2.11. para starting with " When excess treatment is  
> set to
> > 'shape', ". What is the error handling if excess treatment is set to
> > 'shape'
> > and no TMOD parameter is given?
>
> The E flag is set for the parameter and the reservation fails.  This  
> can
> be specified in Section 6.2.11.
>
> > - section 7. This section is extremely short. This might a non- 
> issue,
> > but I
> > personally had the feeling the security discussion is too brief.  
> This
> > might
> > be OK, but the document authors and the WG should be prepared for  
> tough
> > questions regarding this when going to the IESG. What about this
> > document
> > defining just the basics, but other documents defining the real
> > behaviour?
> > Is it assumed that this does not raise any new security issue?
>
> Good points and questions.  QSPEC security is directly tied to QoS  
> NSLP
> security, and the QoS NSLP document has a very detailed security  
> section.
> All these considerations apply to QSPEC and that should be noted at  
> the
> beginning of Section 7.  I'm not aware of any new security issues  
> introduced
> by QSPEC beyond the point made already in Section 7 on priority.
> Suggestions welcome from the WG on other security issues that might  
> arise
> from QSPEC.
>
> > - section 8 says in several places " A specification is required to
> > depreciate, delete, or modify QSPEC versions." What happens to  
> deleted
> > IANA
> > values? Is IANA allowed to re-assign those or should they be  
> treated as
> > deprecated values, i.e., they cannot be used anymore, but the  
> value is
> > blocked for ever?
>
> These are IANA specific rules that I don't think need to be reviewed  
> in
> Section 8.
>
> >
> > ****
> > idnits 2.06.01
> >
> > tmp/draft-ietf-nsis-qspec-18.txt:
> > tmp/draft-ietf-nsis-qspec-18.txt(2867): Found possible IPv4 address
> > '7.2.9.4' in position 6; this doesn't match RFC3330's suggested
> > 192.0.2.0/24
> > address range.
> >
> >  Checking boilerplate required by RFC 3978 and 3979, updated by RFC
> > 4748:
> >
> >  
> ----------------------------------------------------------------------------
> >
> >     No issues found here.
> >
> >  Checking nits according to
> > http://www.ietf.org/ietf/1id-guidelines.txt:
> >
> >  
> ----------------------------------------------------------------------------
> >
> >  == No 'Intended status' indicated for this document; assuming
> > Proposed
> >     Standard
> >
> >  == The page length should not exceed 58 lines per page, but there  
> was
> > 3
> >     longer pages, the longest (page 6) being 59 lines
> >
> >  == It seems as if not all pages are separated by form feeds -  
> found 0
> > form
> >     feeds but 55 pages
> >
> >
> >  Checking nits according to http://www.ietf.org/ID-Checklist.html:
> >
> >  
> ----------------------------------------------------------------------------
> >
> >  == There are 1 instance of lines with non-RFC3330-compliant IPv4
> > addresses
> >     in the document.  If these are example addresses, they should be
> > changed.
> >
> >
> >  Miscellaneous warnings:
> >
> >  
> ----------------------------------------------------------------------------
> >
> >  == The copyright year in the IETF Trust Copyright Line does not  
> match
> > the
> >     current year
> >
> >
> >  Checking references for intended status: Proposed Standard
> >
> >  
> ----------------------------------------------------------------------------
> >
> >     (See RFC 3967 for information about using normative references  
> to
> >     lower-maturity documents in RFCs)
> >
> >  == Missing Reference: 'S' is mentioned on line 1730, but not  
> defined
> >     '|  Slack Term [S]  (32-bit integer)
> > |...'
> >
> >  -- Possible downref: Undefined Non-RFC (?) reference : ref. 'S'
> >
> >  == Unused Reference: 'RFC4506' is defined on line 2363, but no
> > explicit
> >     reference was found in the text
> >     '[RFC4506] Eisler, M., "XDR: External Data Representation
> > Standard,"...'
> >
> >  == Unused Reference: 'IEEE754' is defined on line 2375, but no
> > explicit
> >     reference was found in the text
> >     '[IEEE754] Institute of Electrical and Electronics Engineers,
> > "IEEE
> > S...'
> >
> >  == Unused Reference: 'NETWORK-BYTE-ORDER' is defined on line 2381,
> > but no
> >     explicit reference was found in the text
> >     '[NETWORK-BYTE-ORDER] Wikipedia, "Endianness,"
> > http://en.wikipedia.or...'
> >
> >  -- Possible downref: Non-RFC (?) normative reference: ref. '3GPP-1'
> >
> >  -- Possible downref: Non-RFC (?) normative reference: ref. '3GPP-2'
> >
> >  -- Possible downref: Non-RFC (?) normative reference: ref. '3GPP-3'
> >
> >  -- Possible downref: Non-RFC (?) normative reference: ref. 'GIST'
> >
> >  -- Possible downref: Non-RFC (?) normative reference: ref. 'QoS- 
> SIG'
> >
> >
> >     Summary: 0 errors (**), 9 warnings (==), 6 comments (--).
> >  
> ----------------------------------------------------------------------------
> > ----
> >
> >
> > [email protected]
> >
> > NEC Laboratories Europe - Network Research Division
> >
> > NEC Europe Limited | Registered Office: NEC House, 1 Victoria Road,
> > London
> > W3 6BL | Registered in England 2832014
> >
>
> Never miss a thing. Make Yahoo your homepage.  
> _______________________________________________
> nsis mailing list
> [email protected]
> http://www.ietf.org/mailman/listinfo/nsis