Subject: Re: [Teas] RtgDir review: draft-ietf-teas-gmpls-scsi-02

From: Lou Berger <[email-masked]>
Date: Mon, 19 Jun 2017 12:33:06 -0400

(as contributor)

Thomas,
	Sorry about the delayed response.  Thank you for the detailed review!
please see below.

On 05/11/2017 12:33 PM, Thomas Clausen wrote:
> Hello,
> 
> I have been selected as the Routing Directorate reviewer for this draft.
> The Routing Directorate seeks to review all routing or routing-related
> drafts as they pass through IETF last call and IESG review, and
> sometimes on special request. The purpose of the review is to provide
> assistance to the Routing ADs. For more information about the Routing
> Directorate, please see ​
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
> 
> Although these comments are primarily for the use of the Routing ADs, it
> would be helpful if you could consider them along with any other IETF
> Last Call comments that you receive, and strive to resolve them through
> discussion or by updating the draft.
> 
> *Document:* draft-ietf-teas-gmpls-scsi-02
> 
> *Reviewer:* Thomas Clausen
> 
> *Review Date:* 17/05/11
> *IETF LC End Date:* Unknown
> *Intended Status:* Proposed Standard
> 
> *Summary:* 
> 
>   * I have significant concerns about this document and recommend that
>     the Routing ADs discuss these issues further with the authors.
> 
> *Comments:*
> 
>   * The document is short, to the point of honestly being entirely
>     unreadable for a non-expert in the very narrow domain of gmpls. 

Yes, this is the intended audience.  Would adding the following
to the abstract help?

        The context for this document is Generalized MPLS, and the
        reader is expected to be familiar with the GMPLS architecture
        and associate protocol standards.

>   * This is frankly not helped by the document employing what I can only
>     assume to be a clever pun (SCSI ... ) which initially made me go
>     look at the STORM wg and RFC3720 (iSCSI). Unless there's a really
>     good reason, could the WG not chose a non-intentionally-misleading
>     acronym?

Well, context is everything.  Switching Capability-specific information
(SCSI) was ~2000 in the individual draft that became
https://tools.ietf.org/html/draft-ietf-ccamp-ospf-gmpls-extensions-00#section-5.6

>   * Another illustration of this is, that the document uses terminology
>     that I assume has a very specific interpretation (to those, actually
>     experts in the very narrow domain of gmpls) - but which is
>     incomprehensible outside. For example, the document talks (already
>     from the Abstract) about "any specific technology", and in general
>     "technology" - I can think of many things that falls under that term
>     (in general) but which I doubt have anything to do with what this
>     document is about. 

I'm not sure why this is an issue.  This is not a general into document.

>   *  So I went to read the introduction, hoping to understand what this
>     document was about. As far as I can gather, it has to do with
>     defining a TLV format, for use by GMPLS extensions for OSPF and
>     IS-IS. Reading through the introduction, and (quickly) skimming
>     through RFC4202, 4203 and 5307 didn't help a great deal in my
>     understanding of what the purpose of these TLVs are (nor, for that
>     matter, what "technology" is supposed to mean).

This is a fair point and we can bolster the intro text to provide
pointers to expected background reading/knowledge.  How about adding the
following as the first paragraph of the intro:

        The context for this document is Generalized MPLS, and the
        reader is expected to be familiar with the GMPLS architecture,
        associate terminology and protocol standards. Notably, but
        not limited to, <xref target="RFC3945"/>, <xref
        target="RFC4202"/>, <xref target="RFC4203"/> and <xref
        target="RFC5307"/>.

>   * It is not clear to me that the document does not violate "rules" in
>     the protocol that it is setting out to extend. See below.
>   * I also feel that there are several places where the document is too
>     vague.
> 
> *Major Issues:*
> 
>   * Fundamentally, the document needs an introduction written for
>     engineers who have not been part in the development of the document:
>     what is this? Why is it needed? How does it fit into the
>     architecture. I must admit that I have never before felt so lost as
>     to what a document was trying to accomplish, after having actually
>     read the document, twice.

Why?  the intended audience are specialists in GMPLS.  If someone is
unfamiliar with GMPLS this *is not* the right place for them to start.
To be fair, having a pointer to such is a reasonable addition and I
propose the previously stated text to address this.  Does that work for you?

>   * The document also needs, I suspect, a terminology section that
>     contains more than 2119-language -- for example, what's meant by
>     "Technology" ... 

How about:
        The reader is expected to be familiar with GMPLS terminology,
        e.g. as found in <xref target="RFC3945"/>, as well as the
        terminology used in <xref target="RFC4202"/>, <xref
        target="RFC4203"/> and <xref target="RFC5307"/>.

>   * I went to read the Shepherd write-up (so, they serve an actual
>     purpose), which indicates that this document was the result of a
>     GEN-ART review of some other document through the WG. I would assume
>     that a synthesis of that review, plus the resulting WG discussion,
>     would make for excellent fodder for an introduction here.

I leave this to the shepherd to decide if any additional text is needed.

>   * Section 3 specifies a Type field, stating "the lower range is used
>     ..." and "...while the higher range is reserved .." -- I see nowhere
>     a definition of "lower range" or "higher range", not in this
>     section, nor in the IANA section.

This is an excellent catch.  The ranges were eliminated recently and we
missed this!  The sentence will be removed.

>   * What does "formatted according to the value of the Type field" in
>     the ultimate bullet of section 3 mean? Essentially, that "the
>     interpretation and format of the Type field MUST be specified when
>     making the IANA registration" (or something of the sort), which must
>     to be included as advice to the Designated Expert
How about:

          A variable length field, formatted according to the definition
          indicated by value of the Type field.  This field can be
          omitted for certain types.

>   * Section 4 calls out a set of rules for inclusion of the defined TLVs
>     - specifically calling for preservation of ordering both when
>     processing and when re-originating. Is this enabled or prohibited by
>     the protocols into which these TLVs are to be inserted?

These rules only apply to the SCSI-TLVs which are carried in the
Generalized SCSI which is defined by this document.

> If
>     explicitly enabled, I would appreciate specific pointers to where
>     this is enabled? -- if this is not explicitly enabled, may there be
>     potential interoperability problems here? 

agreed.  see the first paragraph of the procedures section covering this
point.

> For having in a different
>     space written TLV-based protocols, and seen (locally highly
>     optimized) parsers/processors/forwarders implementing different
>     ordering priorities, this merits clarification.
Please review the above text and let us know if you think a
clarification is still warranted.

>   * The IANA section tells IANA to create "either XXX, or YYY" (2nd
>     paragraph) -- but which is it? I doubt that it is IANA's role to
>     make this arbitration. The WG should make a clear recommendation.
> 
we don't care.  How about if we add: ", at their desecration."?

> *Minor Issues:*
> 
>   * Section 4 calls out "Sub-TLV parsing (format) errors, such as an
>     underrun or overrun, MUST be treated as a malformed ISCD".  This
>     seems either overreaching or underachieving: are we strictly talking
>     about "there's not the promised amount of octets in the value field"
>     (overrun/underrun)? In which case, perhaps state just that. However
>     the "Sub-TLV parsing" indicates that also an error in parsing of the
>     Value field should be treated the same way? Could you clarify this. 
it says: "Sub-TLV parsing (format) errors MUST be treated as a malformed
ISCD." The "such as" clause is an example.

>   * I would be surprised, but leave to the SEC-DIR/SEC-AD, if the
>     security considerations section is strong enough. The first half of
>     the security considerations states "This document does not introduce
>     any security issues beyond those discussed in ...." -- but then goes
>     on, saying "Tampering with .... may have an effect...mechanisms such
>     as ... are suggested". While I am by no means a security expert, it
>     would seem that yes, indeed, this document does introduce new
>     security issues -- for which I would expect a MTI security mechanism
>     (Or, a convincing explanation as to why these already are covered).
> 
> *Nits:*
> 
>   * The errata to RFC2119 is not reflected in the terminology section
>     (missing "NOT RECOMMENDED")
added

>   * The abstract has an "modify an existing technology specific formats"
>     - which, presumably, should be "modify any existing..." or
>     "...specific format" ?
yes, thank you!

>   * 2nd paragraph of the introduction, any good reason why the HTML
>     version doesn't have a hyperlink to RFC7138 (XML snafu, I gather)?
> 

No idea.  It looks okay now though.

you can see a formatted version (link at bottom of page) and all changes
proposed above at: https://github.com/louberger/teas-gmpls-scsi

Thanks again for the comments.
Lou

> 
> --
> *Thomas Heide Clausen 
> <mailto:[email-masked]> •  @thclausen 
> <http://twitter.com/thclausen>  •  thomasclausen.org 
> <http://www.thomasclausen.org/>
> www.arkko.com/tools/allstats/thomasheideclausen.html
> <http://www.arkko.com/tools/allstats/thomasheideclausen.html>*
> **
> 
> 
> _______________________________________________
> Teas mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/teas
>

---

From: Lou Berger <[email-masked]>
Date: Wed, 28 Jun 2017 19:27:17 -0400

Hi,
	Hearing no objection, I'll upload the version that addresses the review
comments as discussed below.  We of course can changed as/if needed.

Lou
(as contributor)