[Lsr] AD Review of draft-ietf-isis-te-app-09

[Alvaro Retana <[e-mail]>]

Dear Authors:

Happy New Year!

I have finished reading this document, reviewing the e-mail archive
and all the various reviews and comments.  Quoting one of the authors,
"it is essential that the two IGPs provide equivalent functionality"
[1] -- so I have considered draft-ietf-ospf-te-link-attr-reuse-10 in
parallel with this one (I'm sending out a separate yet very similar
review for it).

In general, I think both drafts need some work.  When appropriate, I
have used "[c]" to highlight comparisons.  I tried to put equivalent
comments in both documents, but may have missed a few...

I have some significant concerns (see details inline) about this
document -- and as a result about the OSPF draft.  I want to point out
a couple of them here:

(A) Deployment Considerations

This document contains what I would characterize as a "distributed"
Deployment Considerations section through §5, §6 and §7.  There is a
lot of content, but I still made some comments in-line about other
important information.  Please consider consolidating all the
deployment-related information in one place.  rfc5706 (specially §2)
may be useful, please take a look.


(B) I was able to identify one significant functional difference which
warrants a discussion of the reason for it and maybe the pros/cons:

   §4.1 talks about the behavior when "both SABM Length and UDABM Length are
   zero".  This document makes the use of the attributes optional while the
   OSPF draft mandates their use (§3).


I will wait for this review to be addressed before moving both
documents forward together.

Thanks!

Alvaro.

[1] https://mailarchive.ietf.org/arch/msg/ospf/7zkoaLfUe19JxTOWaKWO51wK9gg


[Line numbers from idnits.]


idnits 2.16.02

...
16	Abstract

18	   Existing traffic engineering related link attribute advertisements
19	   have been defined and are used in RSVP-TE deployments.  Since the
20	   original RSVP-TE use case was defined, additional applications (e.g.,
21	   SRTE, LFA) have been defined which also make use of the link
22	   attribute advertisements.  In cases where multiple applications wish
23	   to make use of these link attributes the current advertisements do
24	   not support application specific values for a given attribute nor do
25	   they support indication of which applications are using the
26	   advertised value for a given link.

[minor] Expand SRTE and LFA.

28	   This draft introduces new link attribute advertisements which address
29	   both of these shortcomings.  It also discusses backwards
30	   compatibility issues and how to minimize duplicate advertisements in
31	   the presence of routers which do not support the extensions defined
32	   in this document.

[nit] s/draft/document

[c] The OSPF abstract is more general, while this one provides more specifics...


...
107	1.  Introduction

109	   Advertisement of link attributes by the Intermediate-System-to-
110	   Intermediate-System (IS-IS) protocol in support of traffic
111	   engineering (TE) was introduced by [RFC5305] and extended by
112	   [RFC5307], [RFC6119], and [RFC8570].  Use of these extensions has
113	   been associated with deployments supporting Traffic Engineering over
114	   Multiprotocol Label Switching (MPLS) in the presence of Resource
115	   Reservation Protocol (RSVP) - more succinctly referred to as RSVP-TE.

[nit] s/presence of Resource/presence of the Resource

[minor] Please add a reference for RSVP-TE.

117	   In recent years new applications have been introduced which have use
118	   cases for many of the link attributes historically used by RSVP-TE.
119	   Such applications include Segment Routing Traffic Engineering (SRTE)
120	   and Loop Free Alternates (LFA).  This has introduced ambiguity in
121	   that if a deployment includes a mix of RSVP-TE support and SRTE
122	   support (for example) it is not possible to unambiguously indicate
123	   which advertisements are to be used by RSVP-TE and which
124	   advertisements are to be used by SRTE.  If the topologies are fully
125	   congruent this may not be an issue, but any incongruence leads to
126	   ambiguity.

[minor] What is an application?  From the examples above I think I can
tell the difference between an "application", as used in this
document, and an "application" as what the ART area does: "The ART
area develops application protocols and architectures in the IETF."
[1] Please define what an application is in the context of this
document.

[1] https://datatracker.ietf.org/group/art/about/

[minor] Add references for SRTE and LFA...and any use
cases/requirements that support the need (or a forward reference to
§2, for this last point).  All Informational.


...
177	3.  Legacy Advertisements

179	   There are existing advertisements used in support of RSVP-TE.  These
180	   advertisements include sub-TLVs for TLVs 22, 23, 25, 141, 222, and
181	   223 and TLVs for SRLG advertisement.

[minor] Expand SRLG on first use.

[minor] Please add references to where all the values in this section
are defined...if the same as the references above.

183	3.1.  Legacy sub-TLVs
184	   Sub-TLVs for TLVs 22, 23, 25, 141, 222, and 223

186	   Code Point/Attribute Name
187	   --------------------------
188	    3 Administrative group (color)
189	    9 Maximum link bandwidth
190	   10 Maximum reservable link bandwidth
191	   11 Unreserved bandwidth
192	   14 Extended Administrative Group
193	   18 TE Default Metric
194	   33 Unidirectional Link Delay
195	   34 Min/Max Unidirectional Link Delay
196	   35 Unidirectional Delay Variation
197	   36 Unidirectional Link Loss
198	   37 Unidirectional Residual Bandwidth
199	   38 Unidirectional Available Bandwidth
200	   39 Unidirectional Utilized Bandwidth

[nit] To be consistent with the registry, rename the headings to Type
and Description.

[nit] A Table could be nice.


...
231	4.1.  Application Identifier Bit Mask

233	   Identification of the set of applications associated with link
234	   attribute advertisements utilizes two bit masks.  One bit mask is for
235	   standard applications where the definition of each bit is defined in
236	   a new IANA controlled registry.  A second bit mask is for non-
237	   standard User Defined Applications(UDAs).

[nit] s/Applications(UDAs)/Applications (UDAs)

[minor] Include a forward reference to the IANA Considerations.

[minor] "second bit mask is for non-standard User Defined
Applications"  The explicit point is made here that the UDAs are used
for *non-standard* applications.  Given that these bits are for *user
defined* applications, the user can make the bits mean anything they
want (including SRTE, LFA, etc.), right?  If so, then highlighting
*non-standard* seems not to be needed.   s/for non-standard User
Defined Applications/for User Defined Applications

[c] The OSPF draft doesn't make the same emphasis.


...
262	      L-flag: When set, applications listed (both Standard
263	       and User Defined) MUST use the legacy advertisements
264	       for the corresponding link found in TLVs 22, 23,
265	       25, 141, 222, and 223 or TLV 138 or TLV 139 as
266	       appropriate.

[major] The corollary seems to be that if the L-flag is not set then
the applications MUST (?) use the Application Specific Link Attributes
sub-TLV.  Is that correct?  Please be specific.

[major] In a mixed network, where some routers support this
specification, but other don't, it seems that setting the L-flag would
be useful as all applications would "fall back" to the legacy
advertisements.  However, not setting the L-flag seems to have the
potential for the routers to use different information resulting in
(at best) inconsistent decisions if the values are not the same.

[minor] I think that some of the issues related to whether all nodes
support something or not is an operational/deployment consideration;
it should be made clear (in the Deployment Considerations section)
what the requirements are inside a domain.  I would think that the
support requirements change depending on the deployment model -- what
are the assumptions?

268	      SABM Length: Indicates the length in octets (0-127) of the
269	       Bit Mask for Standard Applications.

[minor] s/Bit Mask for Standard Applications/Standard Application
Identifier Bit Mask


...
283	      UDABM Length: Indicates the length in octets (0-127) of the
284	       Bit Mask for User Defined Applications.

[minor] s/Bit Mask for User Defined Applications/User Defined
Application Identifier Bit Mask

286	   SABM  (variable length)
287	      Standard Application Identifier Bit Mask

289	      (SABM Length * 8) bits

291	      This is omitted if SABM Length is 0.

[nit] s/This is omitted/This field is omitted/g


...
303	         F-bit: Set to specify Loop Free Alternate (LFA)
304	          (includes all LFA types)

[just wondering] Given that we're trying to future-proof...  Can there
be cases where the link attributes for "normal" LFA, vs Remote LFA
(for example) might need to be different?  Considering that part of
the justification for this work is the need to potentially advertise
different attributes per application, and given that the question
"what is meant by LFA?" came up more than once on the mailing list, I
guess the answer might be yes (at least for some use cases).  Knowing
that the user has their own bits to play with, I'm sure they can
accommodate if the need comes up.   Just thinking out loud...

306	    UDABM  (variable length)
307	      User Defined Application Identifier Bit Mask

309	      (UDABM Length * 8) bits

311	             0 1 2 3 4 5 6 7 ...
312	            +-+-+-+-+-+-+-+-+...
313	            |                ...
314	            +-+-+-+-+-+-+-+-+...

316	      This is omitted if UDABM Length is 0.

318	   NOTE: If both SABM Length and UDABM Length are zero, then the
319	   attributes associated with this Attribute Identifier Bit Mask
320	   MAY be used by any Standard Application and any User Defined
321	   Application.

[major] This is what happens today: any application can use the
attributes, but they don't have to, right?  I think that the MAY
(Normative) is not needed because it is not really specifying a
behavior that requires interoperability...but it is just mentioning a
fact.  s/MAY/may

[major] [c] The OSPF draft makes it mandatory for all applications to
use the attributes when no bits are set while this document makes it
optional.  I think this is a significant functional difference.

[minor] The same behavior is specified (again!) in §5.2; please
specify it only once.

323	   Standard Application Identifier Bits are defined/sent starting with
324	   Bit 0.  Additional bit definitions that may be defined in the future
325	   SHOULD be assigned in ascending bit order so as to minimize the
326	   number of octets that will need to be transmitted.  Undefined bits
327	   MUST be transmitted as 0 and MUST be ignored on receipt.  Bits that
328	   are NOT transmitted MUST be treated as if they are set to 0 on
329	   receipt.

[major] "Additional bit definitions that may be defined in the future
SHOULD be assigned in ascending bit order so as to minimize the number
of octets that will need to be transmitted."  This statement about
assignment belongs in the IANA Consideration section as part of the
instructions to IANA on how to manage the space.

[major] When would it be ok for IANA not to assign the values in
order?  IOW, why is that a SHOULD and not a MUST?

[minor] Given the intent to minimize the transmissions, should there
be a statement standardizing that behavior?  I'm thinking of something
like: "In order to minimize the number of octets transmitted, the
sender SHOULD transmit only the minimum necessary amount of
information..."  IOW, the assignment policy, and the ability to
advertise the *ABM Length is useless unless the sender takes advantage
of it.

[major] "Undefined bits MUST be transmitted as 0 and MUST be ignored
on receipt."  What if the sender and the receiver support a different
set of applications?  For example, suppose that the sender supports a
new application identified by the N-bit, and sets only that bit; the
receiver treats the N-bit as undefined and ignores it.  The result is
that, for the receiver, there seems to be no indication of an
application.  Is "no application" a valid indication, or should at
least one bit always be set?

[minor] Following on the previous comment...  It seems that a
requirement is for the routers/nodes in the domain to agree on the
meaning of the bits: support the same Standard Applications and
interpret the User Defined bit mask the same way.  Please add this
type of information to the Deployment Considerations Section.

[nit] "Bits that are NOT transmitted MUST be..."  While I understand
what you mean, having Normative language for the behavior of things
that are not received doesn't seem appropriate.  How about something
like this: "Any omitted bits MUST be..."?


...
337	4.2.  Application Specific Link Attributes sub-TLV
...
343	      Type: 16 (temporarily assigned by IANA)
344	      Length: Variable (1 octet)

[major] I hope I'm missing something...  The maximum Length is 254
octets, but the Application Identifier Bit Masks (both together) could
be as big as 256 octets.  I realize that (1) it'll take a while (a
long while) before so many applications are defined, and (2) the SABM
and UDABM fields can be advertised in different sub-TLVs.  It might be
a good idea to say something about separating the
advertisements...given that the assignment of values is the only
protection against someone forcing the advertisement of all the
bits...

345	      Value:

347	        Application Identifier Bit Mask
348	        (as defined in Section 4.1)

350	        Link Attribute sub-sub-TLVs - format matches the
351	        existing formats defined in [RFC5305] and [RFC8570]

353	   When the L-flag is set in the Application Identifier Bit Mask, all of
354	   the applications specified in the bit mask MUST use the link
355	   attribute sub-TLV advertisements listed in Section 3.1 for the
356	   corresponding link.  Link attribute sub-sub-TLVs for the
357	   corresponding link attributes MUST NOT be advertised for the set of
358	   applications specified in the Standard/User Application Identifier
359	   Bit Masks and all such advertisements MUST be ignored on receipt.

[major] "When the L-flag is set...MUST use the link attribute sub-TLV
advertisements listed in Section 3.1"

(a) The behavior of the L-flag is already specified in §4.1.  Please
only specify behaviors in one place.

(b) The specification here is not the same as what §4.1 says.  The
text in §4.1 says that "legacy advertisements for the corresponding
link found in TLVs 22, 23, 25, 141, 222, and 223 or TLV 138 or TLV
139", which I interpret as using any current or new (!) sub-TLVs for
those TLVs.  OTOH, the text in this section limits the use to the
"link attribute sub-TLV advertisements listed in Section 3.1".   I'm
assuming the correct specification is the one in §4.1.  IOW, this
document doesn't preclude continued extensions to RFC5305, RFC5307,
RFC6119, or RFC8570, right?

[?] Just to make sure I understand: "Link attribute sub-sub-TLVs for
the corresponding link attributes MUST NOT be advertised..."  This
means that if the L-flag is set, all the attributes will be advertised
using legacy means, right?

361	   Multiple Application Specific Link Attribute sub-TLVs for the same
362	   link MAY be advertised.  When multiple sub-TLVs for the same link are
363	   advertised, they SHOULD advertise non-conflicting application/
364	   attribute pairs.  A conflict exists when the same application is
365	   associated with two different values of the same link attribute for a
366	   given link.  In cases where conflicting values for the same
367	   application/attribute/link are advertised all the conflicting values
368	   MUST be ignored.

370	   For a given application, the setting of the L-flag MUST be the same
371	   in all sub-TLVs for a given link.  In cases where this constraint is
372	   violated, the L-flag MUST be considered set for this application.

[major] Tying these two paragraphs together...  If one of the sub-TLVs
has the L-flag set, then all the sub-TLVs that include at least one
common application MUST be considered as having the L-flag set as
well.  This seems to become an attack vector: a rogue IS can set the
L-flag (and/or set application bits) in all/some of the
sub-TLVs...which may result in no attribute information (if the
sub-sub-TLVs were the only source of information and are ignored) for
a link...

374	   A new registry of sub-sub-TLVs is to be created by IANA which defines
375	   the link attribute sub-sub-TLV code points.  This document defines a
376	   sub-sub-TLV for each of the existing sub-TLVs listed in Section 3.1
377	   except as noted below.  The format of the sub-sub-TLVs matches the
378	   format of the corresponding legacy sub-TLV and IANA is requested to
379	   assign the legacy sub-TLV identifer to the corresponding sub-sub-TLV.

[minor] Include a forward reference to the IANA Considerations...

[major] "...and IANA is requested to assign the legacy sub-TLV
identifer to the corresponding sub-sub-TLV." (s/identifer/identifier)
Is this assignment something that should also be done going forward
(for new "legacy sub-TLVs"), or just for the initial values in the
registry?   The new registry has a registration policy of "Expert
Review", please include any instructions for the DEs-to-be in the IANA
Considerations section...and maybe a (Normative) pointer to rfcxxx for
completeness.

381	4.2.1.  Special Considerations for Maximum Link Bandwidth

383	   Maximum link bandwidth is an application independent attribute of the
384	   link.  When advertised using the Application Specific Link Attributes
385	   sub-TLV, multiple values for the same link MUST NOT be advertised.
386	   This can be accomplished most efficiently by having a single
387	   advertisement for a given link where the Application Identifier Bit
388	   Mask identifies all the applications which are making use of the
389	   value for that link.

[minor] I understand why the "Maximum link bandwidth is an application
independent attribute of the link".  But I am having a harder time
understanding why other bandwidth-related metrics (specifically
Unidirectional Residual Bandwidth, Unidirectional Available Bandwidth
and Unidirectional Utilized Bandwidth) are not treated the same way.
A similar question was raised on the list and the treatment was
justified in order to support use cases where the bandwidth is managed
per application [1].  I don't want to revisit the discussion here
about the treatment, but it is important for the reader to understand
the existence of those use cases.  Please add a paragraph or two
talking about the consideration.

[1] https://mailarchive.ietf.org/arch/msg/ospf/xl8HWDeQqYNcv_X_568-xv6rfvk

391	   It is also possible to advertise the same value for a given link
392	   multiple times with disjoint sets of applications specified in the
393	   Application Identifier Bit Mask.  This is less efficient but still
394	   valid.

[minor] This attribute and other bandwidth-related ones are prone to
interaction as multiple applications may want to use the resources.
While the use of the resources is out of scope, it would be helpful to
include that type of information in a Deployment Considerations
section, as mentioned already on the mailing list [1].

[1] https://mailarchive.ietf.org/arch/msg/isis-wg/2M2pho1EZcVTkOOQssLQUVO0HY4

396	   If different values for Maximum Link Bandwidth for a given link are
397	   advertised, all values MUST be ignored.

[major] Also an attack vector: for a specific link, a rogue IS can
advertise multiple values, rendering the information unusable for all
applications.

399	4.2.2.  Special Considerations for Reservable/Unreserved Bandwidth

401	   Maximum Reservable Link Bandwidth and Unreserved Bandwidth are
402	   attributes specific to RSVP.  When advertised using the Application
403	   Specific Link Attributes sub-TLV, bits other than the RSVP-TE(R-bit)
404	   MUST NOT be set in the Application Identifier Bit Mask.  If an
405	   advertisement of Maximum Reservable Link Bandwidth or Unreserved
406	   Bandwidth is received with bits other than the RSVP-TE bit set, the
407	   advertisement MUST be ignored.

[nit] s/specific to RSVP/specific to RSVP-TE

[nit] s/RSVP-TE(R-bit)/RSVP-TE (R-bit)

[major] It seems to me that "the L-flag MUST be set" is missing above.

409	4.3.  Application Specific SRLG TLV

411	   A new TLV is defined to advertise application specific SRLGs for a
412	   given link.  Although similar in functionality to TLV 138 (defined by
413	   [RFC5307]) and TLV 139 (defined by [RFC6119], a single TLV provides
414	   support for IPv4, IPv6, and unnumbered identifiers for a link.
415	   Unlike TLVs 138/139, it utilizes sub-TLVs to encode the link
416	   identifiers in order to provide the flexible formatting required to
417	   support multiple link identifier types.

[nit] s/TLV 138 (defined by [RFC5307]) and TLV 139 (defined by
[RFC6119],/TLV 138 [RFC5307] and TLV 139 [RFC6119],

419	       Type: 238 (Temporarily assigned by IANA)
420	       Length: Number of octets in the value field (1 octet)

[major] As with the other sub-TLV, the length is < the potential size
of the Application Identifier Bit Mask...

421	       Value:
422	         Neighbor System-ID + pseudo-node ID (7 octets)
423	         Application Identifier Bit Mask
424	          (as defined in Section 4.1)
425	         Length of sub-TLVs (1 octet)
426	         Link Identifier sub-TLVs (variable)
427	         0 or more SRLG Values (Each value is 4 octets)

429	       The following Link Identifier sub-TLVs are defined. The type
430	       values are suggested and will be assigned by IANA - but as
431	       the formats are identical to existing sub-TLVs defined for
432	       TLVs 22, 23, 25, 141, 222, and 223 the use of the suggested
433	       sub-TLV types is strongly encouraged.

[major] "The type values are suggested and will be assigned by
IANA..."  No.  These values are part of a new registry, so this
document can define any value it wants.

[minor] Instead of the text above, include a forward reference to the
IANA Considerations where the whole registry in included.

435	       Type    Description
436	        4      Link Local/Remote Identifiers (see [RFC5307])
437	        6      IPv4 interface address (see [RFC5305])
438	        8      IPv4 neighbor address (see [RFC5305])
439	       12      IPv6 Interface Address (see [RFC6119])
440	       13      IPv6 Neighbor Address (see [RFC6119])

[nit] s/(see [RFCxxxx])/[RFCxxxx]/g

442	   At least one set of link identifiers (IPv4, IPv6, or unnumbered) MUST
443	   be present.  TLVs which do not meet this requirement MUST be ignored.

[major] "one set of link identifiers...MUST be present"  Is a set one
of those sub-TLVs or more than one?


...
463	5.1.  Use of Legacy Advertisements

[c] I made the same comments in §8.1 of the OSPF draft.

465	   Bit Identifers for Standard Applications are defined in Section 4.1.
466	   All of the identifiers defined in this document are associated with
467	   applications which were already deployed in some networks prior to
468	   the writing of this document.  Therefore, such applications have been
469	   deployed using the legacy advertisements.  The Standard Applications
470	   defined in this document MAY continue to use legacy advertisements
471	   for a given link so long as at least one of the following conditions
472	   is true:

[nit] s/Identifers/Identifiers

[major] Even if specifying the behavior for the Standard Applications,
this document cannot Normatively specify anything affecting the case
where "applications have been deployed using the legacy
advertisements".  s/MAY/may

[] I think that the text in this section can be generalized to any
application, not just ones defined in this document.  s/The Standard
Applications defined in this document MAY continue/The Standard
Applications may continue

474	   o  The application is RSVP-TE

476	   o  The application is SRTE or LFA and RSVP-TE is not deployed
477	      anywhere in the network

479	   o  The application is SRTE or LFA, RSVP-TE is deployed in the
480	      network, and both the set of links on which SRTE and/or LFA
481	      advertisements are required and the attribute values used by SRTE
482	      and/or LFA on all such links is fully congruent with the links and
483	      attribute values used by RSVP-TE

[] To generalize:

   o  A single application is deployed in the network

   o  Multiple applications are deployed in the network and all the links and
      attribute values are fully congruent between them.

485	   Under the conditions defined above, implementations which support the
486	   extensions defined in this document have the choice of using legacy
487	   advertisements or application specific advertisements in support of
488	   SRTE and/or LFA.  This will require implementations to provide
489	   controls specifying which type of advertisements are to be sent/
490	   processed on receive for these applications.  Further discussion of
491	   the associated issues can be found in Section 7.

[] To generalize: s/in support of SRTE and/or LFA/in support of other
applications

493	   New applications which future documents define to make use of the
494	   advertisements defined in this document MUST NOT make use of legacy
495	   advertisements.

[major] Why not?  Just as the cases above, the application may run my
itself on the network, and/or the attributes may be fully congruent.

497	5.2.  Use of Zero Length Application Identifier Bit Masks

[minor] This section is partially redundant with the behavior
specified in §4.1.  Please make the specification only once.

499	   If link attributes are advertised associated with zero length
500	   Application Identifier Bit Masks for both standard applications and
501	   user defined applications, then that set of link attributes MAY be
502	   used by any application.  If support for a new application is
503	   introduced on any node in a network in the presence of such
504	   advertisements, these advertisements MAY be used by the new
505	   application.  If this is not what is intended, then existing
506	   advertisements MUST be readvertised with an explicit set of
507	   applications specified before a new application is introduced.

[major] s/MAY be used/may be used
It is just a statement of fact...

509	6.  Attribute Advertisements and Enablement

[c] This section is pretty much the same in both drafts.  Consider
making the specification only once (maybe here  -- because the
applications are defined here) and then referencing it.  Note the
generalization comment below.


...
520	   In the case of RSVP-TE, the advertisement of application specific
521	   link attributes implies that RSVP is enabled on that link.  The
522	   absence of RSVP-TE application specific link attributes in
523	   combination with the absence of legacy advertisements implies that
524	   RSVP is NOT enabled on that link.

[minor] Specifying by implication sounds confusing to me.  Can we come
up with Normative language to accompany the text above?

Suggestion>
   The R-bit in the SABM MUST be set only if RSVP is enabled on the
   corresponding link.  All RSVP-enabled links MUST have a corresponding link
   attributes advertisement.

526	   In the case of SRTE, advertisement of application specific link
527	   attributes does NOT indicate enablement of SRTE.  The advertisements
528	   are only used to support constraints which may be applied when
529	   specifying an explicit path.  SRTE is implicitly enabled on all links
530	   which are part of the Segment Routing enabled topology independent of
531	   the existence of link attribute advertisements

533	   In the case of LFA, advertisement of application specific link
534	   attributes does NOT indicate enablement of LFA on that link.
535	   Enablement is controlled by local configuration.

537	   If, in the future, additional standard applications are defined to
538	   use this mechanism, the specification defining this use MUST define
539	   the relationship between application specific link attribute
540	   advertisements and enablement for that application.

[] To generalize:

Suggestion>
   For Standard Applications, advertisement of application specific link
   attributes does NOT indicate enablement of the application on that link.  If
   additional Standard Applications are defined with a different enablement
   expectation, then the corresponding specification MUST define the alternate
   relationship between application specific link attribute advertisements and
   enablement for that application.

542	   This document allows the advertisement of application specific link
543	   attributes with no application identifiers i.e., both the Standard
544	   Application Identifier Bit Mask and the User Defined Application
545	   Identifier Bit Mask are not present (See Section 4.1).  This supports
546	   the use of the link attribute by any application.  In the presence of
547	   an application where the advertisement of link attribute
548	   advertisements is used to infer the enablement of an application on
549	   that link (e.g., RSVP-TE), the absence of the application identifier
550	   leaves ambiguous whether that application is enabled on such a link.
551	   This needs to be considered when making use of the "any application"
552	   encoding.

[major] What does the use of the "any application" encoding say about
RSVP-TE?  My interpretation is that if RSVP-TE is used, then the "any
application" encoding can't be used.  Is that correct?  Please be
specific.


...
572	7.1.  Multiple Applications: Common Attributes with RSVP-TE

574	   In cases where multiple applications are utilizing a given link, one
575	   of the applications is RSVP-TE, and all link attributes for a given
576	   link are common to the set of applications utilizing that link,
577	   interoperability is achieved by using legacy advertisements and
578	   sending application specific advertisements with L-bit set and no
579	   link attribute values.  This avoids duplication of link attribute
580	   advertisements.

[nit] s/L-bit/L-flag/g

[nit] s/with L-bit set/with the L-flag set

582	7.2.  Multiple Applications: All Attributes Not Shared w RSVP-TE

[nit] s/w/with

584	   In cases where one or more applications other than RSVP-TE are
585	   utilizing a given link and one or more link attribute values are NOT
586	   shared with RSVP-TE, it is necessary to use application specific
587	   advertisements as defined in this document.  Attributes for
588	   applications other than RSVP-TE MUST be advertised using application
589	   specific advertisements which have the L-bit clear.  In cases where
590	   some link attributes are shared with RSVP-TE, this requires duplicate
591	   advertisements for those attributes.

[major] "Attributes for applications other than RSVP-TE MUST be
advertised using application specific advertisements..."  §7 sets up
the section mentioning the need "to co-exist with use of the legacy
advertisements by routers which do not support the extensions defined
in this document."  In this case, the routers which support the
extensions will have a different view compared to the routers which
don't.  This can result in sub-optimal routing, loops, etc..  It is
obviously not a backwards compatible deployment.  How can that be
addressed/mitigated?


...
598	7.3.  Use of Application Specific Advertisements for RSVP-TE
...
605	   1)Upgrade all routers to support extensions in this document

[nit] s/support extensions/support the extensions

607	   2)Readvertise all legacy link attributes using application specific
608	   advertisements with L-bit clear and R-bit set.

[minor] s/Readvertise/Advertise

610	   3)Remove legacy advertisements

612	   Migrating RSVP-TE away from legacy advertisements could result in
613	   some implementation simplification as it allows the removal of code
614	   which encodes/decodes the legacy advertisements.  Whether this is
615	   seen as desirable is something for the marketplace to determine.

[?] "allows the removal of code which encodes/decodes the legacy
advertisements"  If the sub-sub-TLVs used are the same as the legacy
ones, wouldn't the code be pretty much the same??  Either way,
considerations which may be implementation-dependent seems out of
place is a section talking about deployment.

617	8.  IANA Considerations

[minor] For clarity, consider using sub-sections below.

619	   This document defines a new sub-TLV for TLVs 22, 23, 25, 141, 222,
620	   and 223.

[major] Please explicitly include the name of the registry.

622	    Type  Description             22   23   25  141  222  223
623	    ----  ---------------------  ---- ---- ---- ---- ---- ----
624	     16   Application Specific     y    y  y(s)   y    y    y
625	           Link Attributes

627	   This document defines one new TLV:

[major] Explicitly mention the name of the registry.

629	    Type  Description             IIH LSP SNP Purge
630	    ----  ---------------------   --- --- --- -----
631	     238  Application Specific     n   y   n    n
632	           SRLG

634	   This document requests a new IANA registry be created to control the
635	   assignment of sub-sub-TLV codepoints for the Application Specific
636	   Link Attributes sub-TLV.  The suggested name of the new registry is
637	   "sub-sub-TLV code points for application specific link attributes".
638	   The registration procedure is "Expert Review" as defined in
639	   [RFC8126].  The following assignments are made by this document:

[major] Explicitly indicate where the new sub-registries should be
created.  In this case in the "IS-IS TLV Codepoints" registry.

[minor] Adding a reference to rfc7370 would be useful.  Make it Normative.

[major] If the legacy specifications are extended, is it expected that
new sub-sub-TLVs will also be created?  If so, there need to be
explicit instructions to the DEs in the legacy registries.


...
663	   This document requests a new IANA registry be created, under the
664	   category of "Interior Gateway Protocol (IGP) Parameters", to control
665	   the assignment of Application Identifier Bits.  The suggested name of
666	   the new registry is "Link Attribute Applications".  The registration
667	   policy for this registry is "Standards Action" ([RFC8126] and
668	   [RFC7120]).  The following assignments are made by this document:

[minor] If we're setting up a applications, why not make it generic:
"IGP Applications", for example...??

[minor] s/"Standards Action" ([RFC8126] and [RFC7120])./"Standards
Action" [RFC8126].     Standards Action is only defined in rfc8126;
the reference to rfc7120 is not needed.

670	    Bit #   Name
671	   ---------------------------------------------------------
672	     0      RSVP-TE (R-bit)
673	     1      Segment Routing Traffic Engineering (S-bit)
674	     2      Loop Free Alternate (F-bit)

[major]  Add a line to cover the rest of the range and call it "Unassigned".

676	   This document requests a new IANA registry be created to control the
677	   assignment of sub-TLV types for the application specific SRLG TLV.
678	   The suggested name of the new registry is "Sub-TLVs for TLV 238".
679	   The registration procedure is "Expert Review" as defined in
680	   [RFC8126].  The following assignments are made by this document:

[major] Indicate where this registry should be created.

[minor] Adding a reference to rfc7370 would be useful. Make it Normative.

682	    Value    Description
683	    ---------------------------------------------------------
684	     0-3     Unassigned
685	      4      Link Local/Remote Identifiers (see [RFC5307])
686	      5      Unassigned
687	      6      IPv4 interface address (see [RFC5305])
688	      7      Unassigned
689	      8      IPv4 neighbor address (see [RFC5305])
690	     9-11    Unassigned
691	     12      IPv6 Interface Address (see [RFC6119])
692	     13      IPv6 Neighbor Address (see [RFC6119])
693	    14-255   Unassigned

[major] Do you want the reference to the assignments to be this
document, the RFC where the encoding is defined, or both?  I would
suggest this document.  Please add a column to reflect that: e.g.,
"[RFC5307] [This Document]"...and take out the "(see ...)" text.

695	9.  Security Considerations

697	   Security concerns for IS-IS are addressed in [ISO10589, [RFC5304],
698	   and [RFC5310].

[major] ISO10589 is not in the References section.  It should be a
Normative reference.

[major] rfc5304/rfc5310 only talk about authentication.  What about
vulnerabilities specifically introduced by the new functionality?
This document doesn't define new TE-related attributes, just a new way
to advertise them...perhaps include something like this:

   This document defines a new way to advertise already defined TE Attributes.
   As these TLVs are not used for SPF computation or normal routing, the
   extensions specified here have no direct effect on IP routing.  Tampering
   with the information defined in this document may have an effect on
   applications using it, including potential sub-optimal Traffic Engineering.
   This is not new...see [RFC5305], [RFC5307], [RFC6119], and [RFC8570].

And then there are possible vulnerabilities introduced by this
document.  I put some comments above about the ability of a rogue
router to render the data unusable by setting, for example the L-flag,
or sending conflicting values.  At best, this action could result in
sub-optimal paths (by taking some links out of consideration)...
Consider adding text related to that.