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

["Carlos Pignataro (cpignata)" <[e-mail]>]

Good catch!

ICCN/OCCN should not be included. Fixed.

— Carlos.

> On Dec 19, 2015, at 12:54 AM, Loa Andersson <[e-mail]> wrote:
> 
> 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
>> 
>> 
>>