Re: [L2tpext] [RTG-DIR] rtg dir review of draft-ietf-l2tpext-sbfd-discriminator

[Loa Andersson <[e-mail]>]

Deborah, authors,

I found one more small glitch.

At most place the document says that the "S-BFD Target Discriminator
ID" AVP goes with ICRQ, ICRP, OCRQ, and OCRP.

But section 2.1. includes ICCN also.

/Loa

On 2015-12-18 22:22, Loa Andersson 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-l2tpext-sbfd-discriminator-01.txt
> Reviewer: Loa Andersson
> Review Date: 2015-12-18
> IETF LC End Date: date-if-known
> Intended Status: Proposed Standard (ID says Standards track)
>
> Summary:
>
>      I have some minor concerns about this document that I think should
> be resolved before publication.
>
> Comments:
>
>       I have considerable problems reading the draft, first it does not
> really follow RFC 7322 in some important details, also the format figure
> (as I understand it) is misleading. The document need a facelift.
>
>
>
> Major Issues:
>
>      "No major issues found."
>
> Minor Issues:
>
>      Even though the nit-picking below is pretty massive, it is purely
> editorial and should be fixed before going to IETF Last Call. I have
> no real concerns about the technical content
>
> Abstract:
> ---------
>      I used often "my immediate manager" as a reference and said that
> the abstract should give her/him a good idea about what the draft is
> about. I don't think the abstract meet that standard. Could you please
> flesh out.
>
>      RFC 7322 says that "Similarly, the Abstract should be complete
> in itself.  It will appear in isolation in publication announcements
> and in the online index of RFCs." If I encounter this and is not up to
> speed on l2tp and bfd, this does not give a good idea what it is about.
>
> Abbreviations
>
> RFC 7322 says:
>     Abbreviations should be expanded in document titles and upon first
>     use in the document.  The full expansion of the text should be
>     followed by the abbreviation itself in parentheses.  The exception is
>     an abbreviation that is so common that the readership of RFCs can be
>     expected to recognize it immediately; examples include (but are not
>     limited to) TCP, IP, SNMP, and HTTP.  The online list of
>     abbreviations [ABBR] provides guidance.  Some cases are marginal, and
>     the RFC Editor will make the final judgment, weighing obscurity
>     against complexity.
>
> The abbreviations are not expanded in title, abstract, and some other
> places, nor are they "well-known".
>
> Examples
> AVP - the RFC Editor abbreviation list gives two expansions, since this
> is l2tp I come to the conclusion that this is "attribute-value pair" and
> not the wellknow "Audio-Visual Profile (AVP)"
>
> S-BFD - BFD is not well-known, and S-BFD is not even in the RFC Editors
> abbreviations list.
>
> L2TPv3 - L2TP or L2TPv3 are not well-know. There is no RFC that does
> not expand the abbreviation in the title.
>
> ICRQ, ICRP, OCRQ, and OCRP are sued but not expanded, the pointer to
> where to find the expansions (RFC 3931) are nit give until the third
> time the quartet is mentioned
>
> LCCE - used but not expanded. nor well-known.
> The RFC Editor abbreviations has two expansions
> LCCE       - Logical Cluster Computing Environment (LCCE) or
>             - L2TP Control Connection Endpoint (RFCs 3931 and 4719)
> Since this is is L2TP context, it is the latter that is correct, but
> since we have an ambiguity we need to expand.
>
> Section 2
>
> I have a problem parsing this sentence:
>
>     This AVP is exchanged during session negotiation (ICRQ, ICRP, OCRQ,
>     OCRP).
>
> Do you mean to say
>
>     The "S-BFD Target Discriminator ID" AVP is exchanged using the ICRQ,
>     ICRP, OCRQ, and OCRP control messages during session negotiations.
>
> Section 2.1
>     There is a TBD in the first sentence of this paragraph. While I
>     agree that TDB is "well-know" I prefer using [TBA by IANA],
>     where TBA stands for To Be Assigned.
>     If you change this you also need to change in the IANA section.
>
> Excuse me if I don't understand this figure
>
>                                                       No. of octets
>                   +-----------------------------+
>                   | Discriminator Value(s)      |     4/Discriminator
>                   :                             :
>                   +-----------------------------+
>
> First I think you say that a Discriminator is 4 octets
> Second there can be a variable number of discriminators per attribute
> value field
> The box in your figure seems to 29 bits wide, this is unorthodox.
>
>
>                    0       1       2       3
>                    01234567012345670123456701234567
>                   +-----------------------------+
>                   | Discriminator Value(s)      |
>                   :                             :
>                   +-----------------------------+
>
> Is this what you mean?
>
>                    0       1       2       3
>                    01234567012345670123456701234567
>                   +--------------------------------+
>                   | Discriminator Value (1)        |
>                   +--------------------------------+
>                   :                                :
>                   +--------------------------------+
>                   | Discriminator Value (n-1)      |
>                   +--------------------------------+
>                   | Discriminator Value (n)        |
>                   +--------------------------------+
>
> Discriminator - a 4 octet value
>
> IANA consideration
>
>       There is a practice - which I disagree with - to assume that IANA
> and readers know where to the registries. Please point it out so no
> mistakes are possible.
>
> OLD TEXT
> This number space is managed by IANA as per [RFC3438].
>
> NEW TEXT
> IANA maintain a sub-registry "Message Type AVP (Attribute Type 0)
> Values" in the "Control Message Attribute Value Pairs" as per
> [RFC3438]. IANA is requested to assign the first free value from this
> sub-registry as the Message typ AVP for "S-BFD Discriminators".
>
>
> Nits:
>
> The nits tool does only give us the date warning.
>
> /Loa
>
>
>