Last Call Review of draft-ietf-isis-te-app-13
review-ietf-isis-te-app-13-rtgdir-lc-decraene-2020-05-29-00

Request Review of draft-ietf-isis-te-app
Requested rev. no specific revision (document currently at 19)
Type Last Call Review
Team Routing Area Directorate (rtgdir)
Deadline 2020-05-29
Requested 2020-05-14
Requested by Alvaro Retana
Authors Les Ginsberg, Peter Psenak, Stefano Previdi, Wim Henderickx, John Drake
Draft last updated 2020-05-29
Completed reviews Rtgdir Last Call review of -06 by Bruno Decraene (diff)
Rtgdir Last Call review of -13 by Bruno Decraene (diff)
Genart Last Call review of -13 by Jouni Korhonen (diff)
Tsvart Last Call review of -13 by Kyle Rose (diff)
Assignment Reviewer Bruno Decraene
State Completed
Review review-ietf-isis-te-app-13-rtgdir-lc-decraene-2020-05-29
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/YehoNxNoTt21MNfbNT2_QUSZq74
Reviewed rev. 13 (document currently at 19)
Review result Has Issues
Review completed: 2020-05-29

Review
review-ietf-isis-te-app-13-rtgdir-lc-decraene-2020-05-29

 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-isis-te-app-13
Reviewer: Bruno Decraene
Review Date: 2020-05-29
IETF LC End Date: 2020-05-29
Intended Status: Standards Track 

Summary:
    I have some minor concerns about this document that I think should be resolved before publication.

Comments:
  Draft is clear.

Minor Issues:

§4.1
*2 (for SABM & UDABM fields)
OLD: The length SHOULD be the minimum required to send all bits which are set.
I'd propose
NEW: The length SHOULD be the minimum required to send all the meaningful bits which are set.

Motivation; the 'bits which are sent' are the bits in the SABM field. (they do include non-meaningful and padding bits)

----

OLD: Undefined bits MUST be transmitted as 0
NEW: Undefined transmitted bits MUST be cleared (0)

Motivation: currently the number of undefined bits is 8*8-3. They SHOULD not be transmitted (beyond the first ones fitting in the first N required octet). The sentence "Undefined bits MUST be transmitted as 0" could be read as all defined bits MUST be transmitted (as 0).
---
User Defined Application Identifier Bits have no name. I'd propose to call them UDABM[0], UDABM[1]... This may avoid that different implementation use different names and, more problematic, that some implementations starting with 1 (the first, the second) while while some other implementations starts as 0, creating interop issues (SABM[1] on node A is SABM[0] on node B)
---
§4.2

"In cases where conflicting values for the same application/attribute/link are advertised all the conflicting values MUST be ignored."
I'd propose to add "for this application"
(IOW, those values are still applicable for all other applications)
---
§6.2
I'd argue that the first part of section 3.2 is a specification of the behavior and hence should be moved to section 4.1, rather than placed in the section "deployment consideration" which eventually will not be read by someone implementing the specification.
Especially since the text in section 4.1 implies a different behavior: "Bits that are NOT transmitted MUST be treated as if they are set to 0 on receipt."
---
§5
"In the case of SRTE, advertisement of application specific link attributes does NOT indicate enablement of SRTE."
What does "enablement of SRTE" means? Do you have a pointer to a document/text?

I'm not sure I would keep that paragraph on SR-TE enablement. 
---
§6.1
"Under the conditions defined above, implementations which support the
   extensions defined in this document have the choice of using legacy
   advertisements or application specific advertisements in support of
   SRTE and/or LFA.  This will require implementations to provide
   controls specifying which type of advertisements are to be sent/
   processed on receive for these applications."
   
I think that "have the choice" is not prescriptive enough given the deployment issues described in section 6.3
I'd rather say that implementations MUST support the use of both advertisements (legacy and application specific advertisement) and MUST provide controls specifying which type of advertisements are to be processed on receive for these applications.