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

["Les Ginsberg (ginsberg)" <[e-mail]>]

Alvaro -

Thanx for your patience.
We have published V10 of the draft addressing your comments.

Note that the authors of the IS-IS and OSPF drafts are "synchronizing" as there is a lot of similarity in your comments regarding the two drafts. We have decided to close all issues with you for the IS-IS draft first - then hopefully the equivalent changes can be made for the OSPF draft can be made and resolved more quickly.

Detailed responses inline.

> -----Original Message-----
> From: Alvaro Retana <[e-mail]>
> Sent: Thursday, January 09, 2020 9:17 AM
> To: [e-mail]
> Cc: Acee Lindem (acee) <[e-mail]>; [e-mail]; [e-mail]
> Subject: AD Review of draft-ietf-isis-te-app-09
> 
> 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.
> 
> 
[Les:] This comment covers three sections:

5.  Deployment Considerations 
6.  Attribute Advertisements and Enablement 
7.  Interoperability, Backwards Compatibility and Migration
       Concerns

Of these I think 5 and 7 have been combined and fall under the heading of "Deployment Considerations".
Section 6 is discussing a particular aspect of the protocol extensions - what the advertisement of link attributes associated with a particular applications says about the state of that application on that link. This isn't a deployment consideration and has been retained as a separate section.

> (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).
> 
[Les:] Both drafts use the word "MAY" in the respective sections entitled
"Use of Zero Length Application Identifier Bit Masks".

"MAY" - or perhaps "may" - makes sense to me because in the absence of SABM
bits there is no way to know if a given application will have any reason to use 
the attributes advertised. The text is intended to say use by any application
is "permitted".  I have changed the text in this way.

> 
> 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/7zkoaLfUe19JxTOWaKWO51wK9
> gg
> 
> 
> [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.
> 
[Les:] DONE

> 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...
> 
[Les:] I have shortened the abstract - though the text is still somewhat different than the OSPF draft because in IS-IS we provide a way to migrate RSVP-TE to the new advertisements whereas in OSPF the recommendation is NOT to migrate OSPF-TE. This difference exists largely because in IS-IS we are more sensitive to the total amount of space advertisements consume.

> 
> ...
> 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
> 
[Les:] Done.

> [minor] Please add a reference for RSVP-TE.
> 
[Les:] Done.

> 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.
> 
[Les:] Done.

> [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.
> 
[Les:] Done.

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

> 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.
> 
[Les:] Done.

> [nit] A Table could be nice.

[Les:] Done.

> 
> 
> ...
> 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)
> 
[Les:] Done.

> [minor] Include a forward reference to the IANA Considerations.
> 
[Les:] I have decided not to do this. I don't think it is needed and I don't
see it as clarifying. This applies to the other cases where you suggested this as well.

> [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
> 
[Les:] In theory it is possible for a User Defined Application to perform the same functions as a standard application (such as SRTE). However, if it did so it
would not be "Standard SRTE" and the bit could NOT be considered as the same as
"standard SRTE".

> [c] The OSPF draft doesn't make the same emphasis.
> 
[Les:] The OSPF draft does not have the equivalent paragraph - maybe it should. But I do not see anything incorrect in the current text.
> 
> ...
> 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.
> 
[Les:] I have removed this text and referenced the next Section 4.2 which
contains a more complete description of how the content of the sub-TLV
is used.

> [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.
> 
[Les:] The revised Deployment considerations section now addresses this.

> [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?
> 
[Les:] The revised Deployment considerations section now addresses this.

> 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
> 
[Les:] Done.

> 
> ...
> 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
> 
[Les:] Done.

> 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
> 
[Les:] Done

> 
> ...
> 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...
> 
[Les:] I don't see the need - but if it arises a new application bit
can be defined.

> 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
> 
[Les:] As per earlier response, I have changed this to use "permitted".
This will (hopefully) avoid anyone thinking that "may" should have been "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.
>
[Les:] Answered above.

 
> [minor] The same behavior is specified (again!) in §5.2; please
> specify it only once.
> 
[Les:] Section 5.2 is the winner.

> 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.
> 
[Les:] Done.

> [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?
> 
[Les:] The most likely case (which isn't very likely) is that the designated
experts are aware of some currently non-standardized use of a bit which they
anticipate will be submitted to the WG for standardization and they are
"nice people" and temporarily skip a bit.
But do you REALLY think this is a MAJOR concern?

> [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.
> 
[Les:] Done

> [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?
> 
[Les:] Just like unknown TLVs, unknown bits MUST be ignored.
I have added text.

> [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.
> 
[Les:] Standard Applications by definition have bits which are assigned 
by IANA. There is therefore no need to say routers have to agree
on their meaning.
User Defined Applications are - by definition - outside the scope of this
specification. They can do whatever they want.

> [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..."?
> 
[Les:] Please define "omitted". The term suggests to me that they could be
in the middle of a transmitted stream - which isn't possible here.
For me there are the bits that were transmitted - and the ones that weren't.
???

> 
> ...
> 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...
> 
[Les:] Base protocol rules (absent RFC 7356 extensions) limit TLVs to
255 bytes in length. It is already possible to generate more information
about a link than will fit in a single TLV. In such cases implementations
already do send multiple TLVs for the same link. I do not believe that
we need to document base protocol behavior yet again for this new
TLV.

> 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.
> 
[Les:] Done.

> (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?
> 
[Les:] Good catch. Fixed.

> [?] 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?
> 
[Les:] Correct - for that specific link.

> 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...
> 
[Les:] The text says:

"Link attribute sub-sub-TLVs for the corresponding link attributes MUST NOT be advertised for the set of applications specified in the Standard/User Application Identifier Bit Masks and all such advertisements MUST be ignored on receipt."

There is no reason to send multiple sub-TLVs w L-bit set on a given link
because by definition such a sub-TLV MUST NOT have any sub-sub-TLVs.

Authentication (as always) is our protection against attacks.


> 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...
> 
[Les:] As noted above, decided against this.

> [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.
> 
[Les:] I have added text in the IANA section.

> 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
> 
[Les:] Addressed in new Section 4.2.3

> 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
> 
[Les:] Added a sub-section in Section 4.

> 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.
> 
[Les:] As always, authentication is your friend.
A rogue who has access to the network can insert all manner of
bogus TLVs - nothing special about these advertisements.

> 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
> 
[Les:] Done.

> [nit] s/RSVP-TE(R-bit)/RSVP-TE (R-bit)
> 
[Les:] Done.

> [major] It seems to me that "the L-flag MUST be set" is missing above.
> 
[Les:] The text is describing "When advertised using the Application Specific Link Attributes sub-TLV..." - which means the L-bit is NOT set. If the L-bit were set then the attribute sub-sub-TLV would NOT be present.

> 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],
> 
[Les:] Done.

> 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...
> 
[Les:] See reply above.

> 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.
> 
[Les:] Done.

> [minor] Instead of the text above, include a forward reference to the
> IANA Considerations where the whole registry in included.
> 
[Les:] As mentioned above, chose not to do this.

> 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
> 
[Les:] Done.

> 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?
> 
[Les:] I believe the text is clear. Since there are two IPv4 and two IPv6 sub-TLVs
you need two sub-TLVs in that case, but as the Link Local/Remote Identifiers are combined in one sub-TLV you only need one in that case.

> 
> ...
> 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
> 
[Les:] Done.

> [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
> 
[Les:] The existing applications have the special property that they are
already deployed using legacy advertisements. This cannot be said for
any as yet to be defined applications. You can insist in the documents
that define new applications that they discuss whether legacy advertisements
can/cannot be used, but this document cannot speak to new applications
without being clairvoyant.

> 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.
> 
[Les:] If we allow new applications to use legacy, this further complicates deployment of new applications as implementations have to deal with cases where advertisements are in legacy exclusively, in new sub-TLV exclusively, and a transition between the two. Such complexity is unavoidable for the existing applications as they are already deployed using legacy. But such complexity is unnecessary in the case of new applications. I have added some text in this regard.

> 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.
> 
[Les:] Done.

> 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...
> 
[Les:] Done.

> 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.
> 
[Les:] For codepoints this makes sense - but for readability it is better to have the text in both drafts.
> 
> ...
> 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?
> 
[Les:] This is the existing behavior based on experience. 
No existing document (to my knowledge) specified what constituted 
enablement and it isn't the role of this document to do so.
We are just documenting existing behavior.
Figure 1 in https://tools.ietf.org/html/draft-hegde-isis-advertising-te-protocols-03#section-1 is informative here.
(Thanx to Chris Bowers for investigating this.)

> 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.
> 
[Les:] AS  discussed above, this seems inappropriate for this document and so was not done.

> 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.
> 
[Les:] The discussion of enablement (now in Section 5) I think covers all the points clearly. I prefer the text we currently have.

> 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.
> 
[Les:] If RSVP-TE uses application specific advertisements, then it behaves just
like any other application - meaning it can use advertisements with an empty
application bit mask. But this introduces ambiguity as far as enablement - 
which we state in the text.
We could prohibit applications which infer enablement from advertisement from using advertisements with 0 length bit masks???

> 
> ...
> 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
> 
[Les:] Done

> [nit] s/with L-bit set/with the L-flag set
> 
[Les:] Done

> 582	7.2.  Multiple Applications: All Attributes Not Shared w RSVP-TE
> 
> [nit] s/w/with
> 
[Les:] Done

> 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?
> 
[Les:] I have added a discussion of this in the new Section 6.
> 
> ...
> 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
> 
[Les:] Done.

> 607	   2)Readvertise all legacy link attributes using application specific
> 608	   advertisements with L-bit clear and R-bit set.
> 
> [minor] s/Readvertise/Advertise
> 
[Les:] Done.

> 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