Re: [Lsr] RtgDir review: draft-ietf-ospf-te-link-attr-reuse-08

[Peter Psenak <[e-mail]>]

Hi Daniele,

please see inline:

On 18/09/2019 11:01, Daniele Ceccarelli 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-ospf-te-link-attr-reuse-08.txt
> Reviewer: Daniele Ceccarelli
> Review Date: 2019-09-13
> IETF LC End Date: date-if-known
> Intended Status: Standard Track
> 
> *Summary:*
> I have significant concerns about this document and recommend that the 
> Routing ADs discuss these issues further with the authors.
> 
> *Comments:*
> 
> *The drafts needs some improvement to be clear and easy to read. It is 
> outside the scope of the RTG-Directorate review to consider consensus on 
> it, but the it is not possible to ignore comments received from a WG 
> member of its usefulness. Implementations on ISIS segment routing and 
> OSPF segment routing (publicly available) prove that applications like 
> Flexible Algorithm, TI-LFA and R-LFA can be implemented using TE 
> parameters compliant with RFC3630 and RFC5305 without the need for these 
> extensions.***

this has been discussed several times in the WG prior to WG acceptance 
and also during its lifetime as the WG document. If there was no 
sufficient support and a real problem to solve, the document would not 
have made it to its current state.


> 
> *That said the rest of the review will be limited only to the quality of 
> the document.*
> 
> *Major Issues:*
> 
>   * No major issue in addition to the one described in the comments.
> 
> *Minor Issues:*
> 
>   * Abstract: it would be nice to have an overview of what is the
>     purpose of distributing the attributes (in addition to MPLS-TE and
>     GMPLS). The document starts with a very generic scope but then
>     focuses on segment routing. It could be stated at the beginning.

the abstract says that link attributes can be used for other then MPLS 
TE and GMPLS and that this document defines how that can be done. Not 
sure what exactly do you want to add. Please let me know.

Not sure where you got the impression that the document focuses on SR, 
but that was not the intent.


>   * Section 2: what does this sentence mean?: “Additionally, there will
>     be additional standardization effort.Additionally, there will be
>     additional standardization effort.  However, this could also be
>     viewed as an advantage as the non-TE use cases for the TE link
>     attributes are documented and validated by the LSR working group”


It means that when the new link attribute is defined and it can be 
advertised as app specific one, the code point from OSPFv2 Extended Link 
TLV sub-TLVs and OSPFv3 Extended LSA Sub-TLV registries would have to be 
allocated so that it can be advertised in ASLA sub-TLV.


>   * It is not clear the usage of RFC2119 language (RECOMMENDED) in
>     section 2.1, is section 2.1 defining a new procedure? My
>     understanding is that section 2 is the actual solution while section
>     3 is the newly defined one. Am I wrong? If so it should be made a
>     bit more clear and I would expect to see RFC2119 language only in
>     section 3.

I'll change the wording and get rid of RECOMMENDED.


>   * Section 3: “This situation SHOULD be logged as an error” how? Should
>     a notification be sent? Logging an error is not part of the protocol
>     definition but rather an implementation issue.

not sure I see the problem. We use this language about logging an error 
message in many places.


>   * Section 4: the title is misleading. It is defining how to encode the
>     list of attributed defined at the end of section 3 (some of them are
>     reused, some others are TBD), why the title of the section is Reused
>     TE link attributes?

no attributes are defined at the end of section 3, these are all 
existing attributes. Section 4 defines the code points from the OSPFv2
Extended Link TLV Sub-TLV Registry and OSPFv3 Extended LSA Sub-TLV 
Registry for these. I will clean the TBDs as these now have early IANA 
allocations.


>   * Sections 5-6-7: Section 3 describes the procedure and TLV format,
>     section 4 the encoding of the attributes…what is defined in section
>     5-6-7. If I search for e.g. Maximum link bandwidth (the title of
>     section 5), the first occurrence is the title of section 5. Maybe
>     gouping sections 5-6-7 into a single one with an intro of what is
>     defined could improve the reading. 

Attribute in sections 5,6,7 are attributes that MUST NOT be advertised 
in ASLA sub-TLV as they are application independent. These sections 
define how to advertise these attributes as application independent link 
attributes without causing the link to be considered as enabled for RSVP TE.



> 
> *Nits:*
> 
>   * MPLS TE is sometimes in capital letters and sometimes not.

will fix that.


>   * SRTE expand on first use.

sure.

thanks,
Peter


> 
> BR
> 
> Daniele
>