Re: [Lsr] AD Review of draft-ietf-ospf-te-link-attr-reuse-10

Peter Psenak <ppsenak@cisco.com> Thu, 07 May 2020 07:24 UTC

Return-Path: <ppsenak@cisco.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 973D43A07AA; Thu, 7 May 2020 00:24:45 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -9.601
X-Spam-Level:
X-Spam-Status: No, score=-9.601 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8NwNpc-E-o5X; Thu, 7 May 2020 00:24:32 -0700 (PDT)
Received: from aer-iport-2.cisco.com (aer-iport-2.cisco.com [173.38.203.52]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1F4123A09AF; Thu, 7 May 2020 00:24:11 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=44795; q=dns/txt; s=iport; t=1588836252; x=1590045852; h=subject:from:to:cc:references:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=YR75y7KcTkS9p46L68IH1xIqyVYXFH/kZbsnq4B8LsY=; b=lPrt8641Dtfkbhkc/mfNu5tIcZpj6BYKVCrsksFYQ9TE4BXSOy00fEyo Wg1fjM8sWrju6+fLeeZB+1vgUYqJ2iyt3zZS8+05KXvN/1RLARsejyfA9 UocZPji9rrOekzuNwS74yAEoYjuCENdshYHEkGuDoIHyjQ260VmbqjTcq Y=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0DMAwA3t7Ne/xbLJq1cChwBAQEBAQEHAQESAQEEBAEBQIFHgxhVIBKETYkBh2ItmXeBZwsBAQEOJQoEAQGERAKCKDgTAgMBAQsBAQUBAQECAQUEbYUqBiYMhXEBAQEBAgEaAQgPAQU2BAcFCwsOCgICJgICVwYBDAgBAReDCwGCXCAPA682doEyhDkBhGqBOgaBDiqJeIJmgUE/gRABJ4JpPoJnAQEDgSgNBA+DLIJgBI4bERgqkzKPNH2CUoJwhSiBCoQjTol9Bh2CW4hhBYRPgnGKH5AXiVSTcIFpIoFWMxoIGxU7gjUBNE8YDYoShjkDF4hjhUQ/A2cCBgEHAQEDCZABAiYHghYBAQ
X-IronPort-AV: E=Sophos;i="5.73,362,1583193600"; d="scan'208";a="25965285"
Received: from aer-iport-nat.cisco.com (HELO aer-core-4.cisco.com) ([173.38.203.22]) by aer-iport-2.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 07 May 2020 07:24:09 +0000
Received: from [10.60.140.51] (ams-ppsenak-nitro2.cisco.com [10.60.140.51]) by aer-core-4.cisco.com (8.15.2/8.15.2) with ESMTP id 0477O8dr027878; Thu, 7 May 2020 07:24:08 GMT
From: Peter Psenak <ppsenak@cisco.com>
To: Alvaro Retana <aretana.ietf@gmail.com>, "draft-ietf-ospf-te-link-attr-reuse@ietf.org" <draft-ietf-ospf-te-link-attr-reuse@ietf.org>
Cc: Yingzhen Qu <yingzhen.qu@futurewei.com>, "lsr-chairs@ietf.org" <lsr-chairs@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>
References: <CAMMESszKgJdgm+cbm4OeV9sG12Aic4T1GeT+6RkLYbxHwLWqLQ@mail.gmail.com> <6e1f3647-5c1f-4c31-27b7-82fe576d2c96@cisco.com>
Message-ID: <e899fedf-34f8-13b6-ceb8-29a43c962af2@cisco.com>
Date: Thu, 07 May 2020 09:24:08 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.7.0
MIME-Version: 1.0
In-Reply-To: <6e1f3647-5c1f-4c31-27b7-82fe576d2c96@cisco.com>
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
X-Outbound-SMTP-Client: 10.60.140.51, ams-ppsenak-nitro2.cisco.com
X-Outbound-Node: aer-core-4.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/ipeUXx8a7TSGuo739zAWpgOaoew>
Subject: Re: [Lsr] AD Review of draft-ietf-ospf-te-link-attr-reuse-10
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 07 May 2020 07:24:46 -0000

Hi Alvaro,

the new version which addresses your comments have been posted.

thanks,
Peter

On 05/05/2020 12:08, Peter Psenak wrote:
> Hi Alvaro,
> 
> thanks for your comments.
> 
> 
> I apologize for the delay in responding to your comments.
> 
> I tried to address all of them, some have been resolved during ISIS
> draft review, in which case I took the same resolution for this draf.
> 
> Please see inline, look for ##PP
> 
> 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-isis-te-app-09 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 ISIS 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 §8, §9 and §10.  There is a
> lot of content, but I still made some comments inline 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.
> 
> 
> ##PP
> I have kept "Attribute Advertisements and Enablement" section (moved it
> up) and combined "Deployment Considerations" and "Interoperability,
> Backwards Compatibility and Migration" into single section "Deployment
> Considerations" similar to what has been done for ISIS
> 
> 
> 
> 
> [c] This document does not include equivalent information to what is
> in §7.* in the ISIS draft.  Please consider "importing" it (or at
> least making a reference).
> 
> 
> ##PP
> done
> 
> 
> (B) I was able to identify one significant functional difference which
> warrants a discussion of the reason for it and maybe the pros/cons:
> 
>      §3 talks about the behavior when "both SABM Length and UDABM Length
> are 0".
>      This document makes the use of the attributes mandatory by all
> applications
>      (in conflict with §8.2) while the ISIS draft makes their use
> optional (§4.1).
> 
> 
> ##PP
> removed the text in section 3 and clarified in section "Use of Zero
> Length Application Identifier Bit Masks" to match what is in ISIS draft.
> 
> 
> 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.]
> 
> ...
> 16	Abstract
> 
> 18	   Various link attributes have been defined in OSPF in the context of
> 19	   the MPLS Traffic Engineering (TE) and GMPLS.  Since the original
> 20	   RSVP-TE use case was defined, additional applications (e.g., SRTE,
> 21	   LFA) have been defined which also make use of the link attribute
> 22	   advertisements.  This document defines how to distribute link
> 23	   attributes in OSPFv2 and OSPFv3 for applications other than MPLS TE
> 24	   or GMPLS.
> 
> [c] This abstract is a lot more general, while the ISIS one provides
> more specifics, including talking about RSVP-TE instead of MPLS TE...
> 
> [minor] It may not be obvious to all readers what the relationship is
> between RSVP-TE and  MPLS TE...
> 
> ##PP
> Made it similar to ISIS draft.
> 
> 
> 
> ...
> 89	1.  Introduction
> 
> 91	   Various link attributes have been defined in OSPFv2 [RFC2328] and
> 92	   OSPFv3 [RFC5340] in the context of the MPLS TE and GMPLS.  All these
> 93	   attributes are distributed by OSPFv2 as sub-TLVs of the Link-TLV
> 94	   advertised in the OSPFv2 TE Opaque LSA [RFC3630].  In OSPFv3, they
> 95	   are distributed as sub-TLVs of the Link-TLV advertised in the OSPFv3
> 96	   Intra-Area-TE-LSA as defined in [RFC5329].
> 
> [nit] s/defined in OSPFv2/defined for OSPFv2
> 
> [nit] s/context of the MPLS/context of MPLS
> 
> [minor] "All these attributes..."  Which attributes?  I'm sure the
> references are made later, but since they are mentioned here first,
> please put references here.  [[c] The ISIS draft mentions the relevant
> RFCs from the start.]
> 
> 98	   Many of these link attributes are useful outside of traditional MPLS
> 99	   Traffic Engineering or GMPLS.  This brings its own set of problems,
> 100	   in particular how to distribute these link attributes in OSPFv2 and
> 101	   OSPFv3 when MPLS TE and GMPLS are not deployed or are deployed in
> 102	   parallel with other applications that use these link attributes.
> 
> [nit] "own set of problems"  Are there others you want to highlight
> besides the distribution?
> 
> [minor] What is an application?  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/
> 
> [major] Related to applications...  "distinction between link
> attributes used by TE and link attributes used by other OSPFv2 or
> OSPFv3 applications" [§2.1]  Some of the other applications mentioned
> (SRTE, for example) are also related to Traffic Engineering, so the
> general use of TE and non-TE as a distinction is very confusing and
> seems to assume the type of applications that can use the alternate
> advertisement are limited to "non-TE" -- but I'm sure that is not the
> objective.  Please be clear in the use of general terms like TE.
> 
> [c] The ISIS draft seems to be more careful/explicit when pointing out
> the difference.
> 
> [nit] s/distribute these link attributes...with other applications
> that use these link attributes./distribute these link
> attributes...with other applications that may use them.
> 
> 
> ##PP
> Changed to match the ISIS draft,
> 
> 
> ...
> 124	1.1.  Requirements notation
> 
> 126	   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> 127	   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
> 128	   document are to be interpreted as described in [RFC2119].
> 
> [major] Use the rfc8174 template.
> 
> ##PP
> done
> 
> 
> [c] Before starting with the solution, the ISIS draft has a section
> ("Legacy Advertisements") that talks about the current state.  I
> thought that was very helpful.  An equivalent section in this case
> would review (or at least point to) RFC3630, RFC5329, RFC7471,
> RFC7684, RFC8362...
> 
> 
> ##PP
> done. Moved some of the content from section 2.1 to this new section.
> 
> 
> ...
> 136	2.1.  OSPFv2 Extended Link Opaque LSA and OSPFv3 E-Router-LSA
> 
> 138	   Extended Link Opaque LSAs as defined in [RFC7684] for OSPFv2 and
> 139	   Extended Router-LSAs [RFC8362] for OSPFv3 are used to advertise link
> 140	   attributes that are used by applications other then MPLS TE or GMPLS.
> 141	   These LSAs were defined as a generic containers for distribution of
> 142	   the extended link attributes.  There are several advantages in using
> 143	   them:
> 
> 145	   1.  Advertisement of the link attributes does not make the link part
> 146	       of the TE topology.  It avoids any conflicts and is fully
> 147	       compatible with [RFC3630] and [RFC5329].
> 
> 149	   2.  The OSPFv2 TE Opaque LSA and OSPFv3 Intra-Area-TE-LSA remains
> 150	       truly opaque to OSPFv2 and OSPFv3 as originally defined in
> 151	       [RFC3630] and [RFC5329] respectively.  Their contents are not
> 152	       inspected by OSPF, that acts as a pure transport.
> 
> 154	   3.  There is clear distinction between link attributes used by TE and
> 155	       link attributes used by other OSPFv2 or OSPFv3 applications.
> 
> 
> [major] "used by TE" is ambiguous because SRTE, for example, is a form
> of TE.  Note that the TE term is used extensively throughout the
> document...and it may cause that same type of confusion.  The same
> applies for the mention of "non-TE" later on...  Please be explicit
> and don't simply use TE/non-TE, but as necessary qualify the use.
> 
> 
> ##PP
> fixed
> 
> 157	   4.  All link attributes that are used by other applications are
> 158	       advertised in a single LSA, the Extended Link Opaque LSA in
> 159	       OSPFv2 or the OSPFv3 E-Router-LSA [RFC8362] in OSPFv3.
> 
> 161	   The disadvantage of this approach is that in rare cases, the same
> 162	   link attribute is advertised in both the TE Opaque and Extended Link
> 163	   Attribute LSAs in OSPFv2 or the Intra-Area-TE-LSA and E-Router-LSA in
> 164	   OSPFv3.  Additionally, there will be additional standardization
> 165	   effort.  However, this could also be viewed as an advantage as the
> 166	   non-TE use cases for the TE link attributes are documented and
> 167	   validated by the LSR working group.
> 
> [minor] "...both the TE Opaque and Extended Link Attribute LSAs in
> OSPFv2 or the Intra-Area-TE-LSA and E-Router-LSA in OSPFv3."  It might
> be good to introduce (maybe in the §2 introduction) how the
> information is advertised today so that this makes sense to the casual
> reader.
> 
> ##PP
> done
> 
> 
> [nit] "Additionally, there will be additional standardization effort."
>    Not really a technical argument...  Hopefully when attributes are
> standardized in one case they will at the same time standardized for
> using the alternate advertisement mechanism.
> 
> ##PP
> removed
> 
> [major] "non-TE use cases"  Same comment as above: SRTE is a form of
> TE.  Please either be specific and use MPLS TE/RSVP-TE, or clearly
> define the terms somewhere so that readers understand that non-TE
> still includes TE mechanisms like SRTE...
> 
> ##PP
> done
> 
> [nit] The last sentence is unnecessary..  The registration policy for
> the registries is set at "IETF Review or IESG Approval", which means
> that any RFC from any WG would be ok to register a new value (IETF
> Review), and that there may even exist cases when an RFC is not needed
> (IESG Approval)...and there are also ranges for Experimental Use and
> FCFS.  IOW, there is no guarantee that new attributes, or even the
> rehashing of existing ones for "non-TE use cases" will even go through
> the IETF, much less the lsr WG.
> 
> 
> ##PP
> removed
> 
> 
> 
> 
> 
> 169	   Extended Link Opaque LSA [RFC7684] and E-Router-LSA [RFC8362] are
> 170	   used to advertise any link attributes used for non-TE applications in
> 171	   OSPFv2 or OSPFv3 respectively, including those that have been
> 172	   originally defined for TE applications.
> 
> [minor] Please include a forward reference to §4 somewhere.
> 
> ##PP
> done
> 
> [nit] This is the 3rd time that you mention this...
> 
> ##PP
> It is important :)
> 
> 
> 
> 174	   TE link attributes used for RSVP-TE/GMPLS continue to use OSPFv2 TE
> 175	   Opaque LSA [RFC3630] and OSPFv3 Intra-Area-TE-LSA [RFC5329].
> 
> [nit] Also mentioned above...
> 
> ##PP
> We need to make this very clear. I rather keep it in multiple places
> 
> 
> 
> 
> 177	   The format of the link attribute TLVs that have been defined for TE
> 178	   applications will be kept unchanged even when they are used for non-
> 179	   TE applications.  Unique code points will be allocated for these TE
> 180	   link attribute TLVs from the OSPFv2 Extended Link TLV Sub-TLV
> 181	   Registry [RFC7684] and from the OSPFv3 Extended LSA Sub-TLV Registry
> 182	   [RFC8362].  For each reused TLV, the code point will be defined in an
> 183	   IETF document along with the expected use-case(s).
> 
> 
> [minor] s/Unique code points will be allocated.../Unique code points
> are allocated...
> Please also add a pointer to the IANA Considerations section here.
> 
> ##PP
> done
> 
> 
> [major] "For each reused TLV, the code point will be defined in an
> IETF document along with the expected use-case(s)."  It sounds as if
> the intent of this sentence is to set a requirement for future work.
> However, as I mentioned above, the Registration Policy defined for the
> registries don't support asking for any type of IETF documentation.
> 
> ##PP
> removed
> 
> 
> 
> 185	3.  Advertisement of Application Specific Values
> 
> 187	   To allow advertisement of the application specific values of the link
> 188	   attribute, a new Application Specific Link Attributes (ASLA) sub-TLV
> 189	   is defined.  The ASLA sub-TLV is a sub-TLV of the OSPFv2 Extended
> 190	   Link TLV [RFC7471] and OSPFv3 Router-Link TLV [RFC8362].
> 
> [minor] s/RFC7471/RFC7684
> 
> ##PP
> fixed
> 
> 
> ...
> 221	      SABM Length: Standard Application Identifier Bit-Mask Length.  It
> 222	      MUST be a multiple of 4 bytes.  If the Standard Application Bit-
> 223	      Mask is not present, the Standard Application Bit-Mask Length MUST
> 224	      be set to 0.
> 
> [c] The ISIS draft defines the L-flag to indicate that the
> "applications listed...MUST use the legacy advertisements".  There is
> no equivalent functionality here.  Please see my comments about this
> in §4.1 (of the ISIS draft).
> 
> ##PP
> That is correct, there is no L-flag in OSPF. We do not plan to use TE
> Opaque LSA for anything else than what they have been defined for.
> 
> 
> 
> 
> 
> ...
> 231	      Standard Application Identifier Bit-Mask: Optional set of bits,
> 232	      where each bit represents a single standard application.  Bits are
> 233	      defined in [I-D.ietf-isis-te-app], which also request a new IANA
> 234	      "Link Attribute Applications" registry under "Interior Gateway
> 235	      Protocol (IGP) Parameters" for them.  The bits are repeated here
> 236	      for informational purpose:
> 
> [minor] s/Bits are defined in [I-D.ietf-isis-te-app], which also
> request a new IANA "Link Attribute Applications" registry under
> "Interior Gateway Protocol (IGP) Parameters" for them./Bits are
> defined in [I-D.ietf-isis-te-app].
> 
> ##PP
> done
> 
> 238	         Bit-0 (R-bit): RSVP TE
> 
> [nit] s/RSVP TE/RSVP-TE
> 
> ##PP
> done
> 
> 240	         Bit-1 (S-bit): Segment Routing TE
> 
> 242	         Bit-2 (F-bit): Loop Free Alternate (LFA).  Includes all LFA
> 243	         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...
> 
> 
> ##PP
> has been closed with ISIS draft.
> 
> 
> ...
> 248	   Standard Application Identifier Bits are defined/sent starting with
> 249	   Bit 0.  Additional bit definitions that are defined in the future
> 250	   SHOULD be assigned in ascending bit order so as to minimize the
> 251	   number of octets that will need to be transmitted.
> 
> [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/MUST (?) 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.
> 
> 
> #PP
> removed this text, as bits and the registry that defines them is
> specified in ISIS draft.
> 
> 
> 253	   User Defined Application Identifier Bits have no relationship to
> 254	   Standard Application bits and are NOT managed by IANA or any other
> 255	   standards body.  It is recommended that bits are used starting with
> 256	   Bit 0 so as to minimize the number of octets required to advertise
> 257	   all of them.
> 
> [nit] "all of them", what?  s/all of them/any User Defined Applications.
> 
> ##PP
> Changed to "all UDAs"
> 
> 
> 259	   Undefined bits in both Bit-Masks MUST be transmitted as 0 and MUST be
> 260	   ignored on receipt.  Bits that are NOT transmitted MUST be treated as
> 261	   if they are set to 0 on receipt.
> 
> [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 unknown (= 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?
> 
> ##PP
> Just like unknown TLVs, unknown bits MUST be ignored.
> I have added more text, similar to ISIS.
> 
> Note that below the case is presented for when "neither the Standard
> Application Bits nor the User Defined Application bits are set (i.e.,
> both SABM Length and UDABM Length are 0)", but the condition is
> different because the bits are not present (see my comment below).
> 
> [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 potential new Deployment Considerations
> Section.
> 
> ##PP
> For Standard Applications it is obvious that we need to agree on the
> meaning of the bits. That's why we are standardizing them. For User
> Defined bits, I would rather avoid specifying anything, it's up to the
> user what it does.
> 
> [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..."?
> 
> ##PP
> aligned with ISIS draft.
> 
> 
> ...
> 271	   The advantage of not making the Application Bit-Masks part of the
> 272	   attribute advertisement itself is that we can keep the format of the
> 273	   link attributes that have been defined previously and reuse the same
> 274	   format when advertising them in the ASLA sub-TLV.
> 
> [nit] s/we can keep the format of the link attributes that have been
> defined previously and reuse the same format when advertising them in
> the ASLA sub-TLV./the format of any previously defined link attributes
> can be kept and reused when advertising them in the ASLA sub-TLV.
> 
> 
> ##PP
> done
> 
> 
> 276	   When neither the Standard Application Bits nor the User Defined
> 277	   Application bits are set (i.e., both SABM Length and UDABM Length are
> 278	   0) in the ASLA sub-TLV, then the link attributes included in it MUST
> 279	   be considered as being applicable to all applications.
> 
> [major] s/bits are set/bits are present
> "set" gives the impression of them being "1"; it's different to have
> them all be 0 than to have then not be there.
> 
> [major] The specification of what to do when "both SABM Length and
> UDABM Length are 0" is inconsistent with §8.2.  Please specify the
> behavior only once.
> 
> 
> ##PP
> I have removed this text and added the text to "Deployment consideration
> " section. The text matches what is in the ISIS draft.
> 
> [c] The ISIS draft leaves open the possibility for applications to use
> the attributes while this document makes it mandatory. I think this is
> a significant functional difference.
> 
> ##PP
> I have aligned with ISIS.
> 
> 
> 281	   If, however, another advertisement of the same link attribute
> 282	   includes any Application Bit-Mask in the ASLA sub-TLV, applications
> 283	   that are listed in the Application Bit-Masks of such ASLA sub-TLV
> 284	   SHOULD use the attribute advertisement which has the application
> 285	   specific bit set in the Application Bit-Masks.
> 
> 287	   If the same application is listed in the Application Bit-Masks of
> 288	   more then one ASLA sub-TLV, the application SHOULD use the first
> 289	   advertisement and ignore any subsequent advertisements of the same
> 290	   attribute.  This situation SHOULD be logged as an error.
> 
> [major] For the first two SHOULDs (in the last two paragraphs: "SHOULD
> use the attribute advertisement" and "SHOULD use the first
> advertisement").  When is it ok to not do as specified?  For example,
> are there specific considerations related to not selecting the first
> advertisement?  Why would a receiver not use the attribute
> advertisement which has the application specific bit set?  IOW, why
> are these not MUSTs?
> 
> ##PP
> Changed to MUST.
> 
> 
> 
> 
> 
> 292	   This document defines the initial set of link attributes that MUST
> 293	   use ASLA sub-TLV if advertised in the OSPFv2 Extended Link TLV or in
> 294	   the OSPFv3 Router-Link TLV.  If the ASLA sub-TLV includes any link
> 295	   attribute(s) NOT listed below, they MUST be ignored.  Documents which
> 296	   define new link attributes MUST state whether the new attributes
> 297	   support application specific values and as such MUST be advertised in
> 298	   an ASLA sub-TLV.  The link attributes that MUST be advertised in ASLA
> 299	   sub-TLVs are:
> 
> [nit] s/use ASLA sub-TLV/use the ASLA sub-TLV
> 
> ##PP
> fixed
> 
> 
> 
> [major] "initial set of link attributes that MUST use [the] ASLA
> sub-TLV if advertised..."    The second paragraph in this section says
> that "ASLA sub-TLV is an optional sub-TLV" -- that is a true
> statement.  However, the ASLA sub-TLV is (according to this paragraph)
> mandatory when the link attributes are used.
> 
> Suggestion (for the second paragraph)>
> 
>      The ASLA sub-TLV is an optional sub-TLV and can appear multiple
> times in the
>      OSPFv2 Extended Link TLV and OSPFv3 Router-Link TLV.  It MUST be
> present if
>      any of the link attributes listed at the end on this section are
>      advertised.  It has the following format:
> 
> 
> ##PP
> done
> 
> 
> 301	      - Shared Risk Link Group
> 
> 303	      - Unidirectional Link Delay
> 
> 305	      - Min/Max Unidirectional Link Delay
> 
> 307	      - Unidirectional Delay Variation
> 
> 309	      - Unidirectional Link Loss
> 
> 311	      - Unidirectional Residual Bandwidth
> 
> 313	      - Unidirectional Available Bandwidth
> 
> 315	      - Unidirectional Utilized Bandwidth
> 
> 317	      - Administrative Group
> 
> 319	      - Extended Administrative Group
> 
> 321	      - TE Metric
> 
> [minor]  Please include references for the definition of these attributes.
> 
> ##PP
> done
> 
> 
> 323	4.  Reused TE link attributes
> 
> 325	   This section defines the use case and code points from the OSPFv2
> 326	   Extended Link TLV Sub-TLV Registry and OSPFv3 Extended LSA Sub-TLV
> 327	   Registry for some of the link attributes that have been originally
> 328	   defined for TE or GMPLS.
> 
> [minor] "defines the...code points"  The code points are
> assigned/defined by IANA later on..   Perhaps: "This section defines
> the use cases and indicates the code points (see Section 12)..."
> 
> ##PP
> done
> 
> 
> ...
> 408	4.4.  TE Metric
> 
> 410	   [RFC3630] defines TE Metric.
> 
> [minor] rfc3630 calls it "Traffic Engineering Metric"
> 
> ##PP
> fixed
> 
> 
> ...
> 417	5.  Maximum Link Bandwidth
> 
> [nit] Include the application independent attributes in a single
> section -- a sub-section each.
> 
> 419	   Maximum link bandwidth is an application independent attribute of the
> 420	   link that is defined in [RFC3630].  Because it is an application
> 421	   independent attribute, it MUST NOT be advertised in ASLA sub-TLV.
> 422	   Instead, it MAY be advertised as a sub-TLV of the Extended Link
> 423	   Opaque LSA Extended Link TLV in OSPFv2 [RFC7684] or sub-TLV of OSPFv3
> 424	   E-Router-LSA Router-Link TLV in OSPFv3 [RFC8362].
> 
> [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.
> 
> ##PP
> added a similar section as the one that has been added to ISIS draft.
> 
> 
> [1] https://mailarchive.ietf.org/arch/msg/ospf/xl8HWDeQqYNcv_X_568-xv6rfvk
> 
> [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.
> 
> [c] A related point was made when reviewing the ISIS draft [1].
> 
> [1]
> https://mailarchive.ietf.org/arch/msg/isis-wg/2M2pho1EZcVTkOOQssLQUVO0HY4
> 
> 
> ##PP
> I hope the added section makes it clear.
> 
> 
> ...
> [minor][c] The ISIS draft also calls out the Maximum Reservable Link
> Bandwidth and Unreserved Bandwidth as attributes specific to
> RSVP-TE...but this document doesn't [§4.2.2].  Why?  I think the
> answer has to do with this text from §2.1:
> 
>      TE link attributes used for RSVP-TE/GMPLS continue to use OSPFv2 TE
>      Opaque LSA [RFC3630] and OSPFv3 Intra-Area-TE-LSA [RFC5329].
> 
> Is that correct?
> 
> ##PP
> more precisely, Maximum Reservable Link Bandwidth and Unreserved
> Bandwidth as RSVP-TE specific attributes are not allowed in ASLA TLV.
> These attributes MUST use TE Opaque LSAs.
> 
> If so, it wasn't completely clear to me that the sub-TLVs defined in
> rfc3630 and rfc5329 continue to be advertised when using
> RSVP-TE/GMPLS.
> 
> ##PP
> - all TLVs defined in rfc3630 and rfc5329 can continue to be advertised
> in legacy TE Opaque LSAs when used with RSVP-TE/GMPLS
> 
> - Maximum Reservable Link and Bandwidth and Unreserved Bandwidth as
> RSVP-TE specific attributes MUST only use the TE Opaque LSAs and MUST
> never be advertised in ASLA.
> 
> - we do support app bit in ASLA TLV for RSVP-TE. This allows attributes
> that are allowed to be advertised in ASLA TLV to be associated with RSVP-TE.
> 
> - It might be worth including stronger wording in §2.1.
> 
> 
> ##PP
> there can not be a stronger wording.
> 
> - Given that some of the sub-TLVs (from rfc3630 and rfc5329) are
> reused (§4.3 through §7), maybe it is worth providing some guidance on
> not using those if RSVP-TE is the only application, or avoiding their
> inclusion in the original LSAs.
> 
> ##PP
> I added a new subsection in the "Deployment Considerations" to describe
> the above RSVP case and the case of Maximum Reservable Link Bandwidth
> and Unreserved Bandwidth/
> 
> 
> 
> 434	6.  Local Interface IPv6 Address Sub-TLV
> 
> [major] The Local/Remote Interface IPv6 Address Sub-TLVs (rfc5329) can
> include multiple addresses for a link.  It seems to me that it could
> be possible for different applications to use different addresses.  If
> that is the case, then it seems that these sub-TLVs should not be
> application independent.  Why are they not considered to be
> application specific?
> 
> [minor] Why are IPv4 addresses not considered?
> 
> ##PP
> IPv4 local address is part of the basic spec.
> IPv6 remote address has been added in rfc8379.
> 
> 
> ...
> 460	8.1.  Use of TE LSA Advertisements
> 
> [c] I made the same comments in §5.1 of the ISIS draft.
> 
> 462	   Bit Identifers for Standard Applications are defined in Section 3.
> 463	   All of the identifiers defined in this document are associated with
> 464	   applications which were already deployed in some networks prior to
> 465	   the writing of this document.  Therefore, such applications have been
> 466	   deployed using the TE LSA advertisements.  The Standard Applications
> 467	   defined in this document MAY continue to use TE LSA advertisements
> 468	   for a given link so long as at least one of the following conditions
> 469	   is true:
> 
> [nit] s/Identifers/Identifiers
> 
> ##PP
> fixed
> 
> [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
> 
> ##PP
> I believe this has been closed during ISIS review.
> I also added a sentence at the end of this section to match ISIS.
> 
> 
> 
> 471	      The application is RSVP-TE
> 
> 473	      The application is SRTE or LFA and RSVP-TE is not deployed
> 474	      anywhere in the network
> 
> 476	      The application is SRTE or LFA, RSVP-TE is deployed in the
> 477	      network, and both the set of links on which SRTE and/or LFA
> 478	      advertisements are required and the attribute values used by SRTE
> 479	      and/or LFA on all such links is fully congruent with the links and
> 480	      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.
> 
> 482	   Under the conditions defined above, implementations which support the
> 483	   extensions defined in this document have the choice of using TE LSA
> 484	   advertisements or application specific advertisements in support of
> 485	   SRTE and/or LFA.  This will require implementations to provide
> 486	   controls specifying which type of advertisements are to be sent/
> 487	   processed on receive for these applications.  Further discussion of
> 488	   the associated issues can be found in Section 10.
> 
> [] To generalize: s/in support of SRTE and/or LFA/in support of other
> applications
> 
> 490	   New applications which future documents define to make use of the
> 491	   advertisements defined in this document MUST NOT make use of TE LSA
> 492	   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.
> 
> 
> ##PP
> again, this has been closed with ISIS. I updated accordingly.
> 
> 
> 
> 494	8.2.  Use of Zero Length Application Identifier Bit Masks
> 
> [minor] This section is partially redundant with the behavior
> specified in §3.  Please make the specification only once.
> 
> ##PP
> removed from 3.
> 
> 
> 496	   If link attributes are advertised associated with zero length
> 497	   Application Identifier Bit-Masks for both standard applications and
> 498	   user defined applications, then that set of link attributes MAY be
> 499	   used by any application.  If support for a new application is
> 500	   introduced on any node in a network in the presence of such
> 501	   advertisements, these advertisements MAY be used by the new
> 502	   application.  If this is not what is intended, then existing
> 503	   advertisements MUST be readvertised with an explicit set of
> 504	   applications specified before a new application is introduced.
> 
> [major] s/MAY be used/may be used
> It is just a statement of fact...
> 
> ##PP
> updated to match ISIS
> 
> 
> 506	9.  Attribute Advertisements and Enablement
> 
> [c] This section is pretty much the same in both drafts.  Consider
> making the specification only once (maybe in the ISIS draft -- because
> the applications are defined there) and then referencing it.  Note the
> generalization comment below.
> 
> ##PP
> I believe it's good to keep it for readability. I made it same as in
> ISIS draft.
> 
> 
> ...
> 517	   In the case of RSVP-TE, the advertisement of application specific
> 518	   link attributes implies that RSVP is enabled on that link.  The
> 519	   absence of RSVP-TE application specific link attributes in
> 520	   combination with the absence of legacy advertisements implies that
> 521	   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.
> 
> 523	   In the case of SRTE, advertisement of application specific link
> 524	   attributes does NOT indicate enablement of SRTE.  The advertisements
> 525	   are only used to support constraints which may be applied when
> 526	   specifying an explicit path.  SRTE is implicitly enabled on all links
> 527	   which are part of the Segment Routing enabled topology independent of
> 528	   the existence of link attribute advertisements.
> 
> 530	   In the case of LFA, advertisement of application specific link
> 531	   attributes does NOT indicate enablement of LFA on that link.
> 532	   Enablement is controlled by local configuration.
> 
> 534	   If, in the future, additional standard applications are defined to
> 535	   use this mechanism, the specification defining this use MUST define
> 536	   the relationship between application specific link attribute
> 537	   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.
> 
> 539	   This document allows the advertisement of application specific link
> 540	   attributes with no application identifiers i.e., both the Standard
> 541	   Application Identifier Bit-Mask and the User Defined Application Bit
> 542	   Mask are not present (See Section 3).  This supports the use of the
> 543	   link attribute by any application.  In the presence of an application
> 544	   where the advertisement of link attribute advertisements is used to
> 545	   infer the enablement of an application on that link (e.g., RSVP-TE),
> 546	   the absence of the Application Identifier leaves ambiguous whether
> 547	   that application is enabled on such a link.  This needs to be
> 548	   considered when making use of the "any application" 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.
> 
> ##PP
> I hope all the above comments were addressed by reusing ISIS text, which
> has been approved already.
> 
> 
> 550	10.  Backward Compatibility
> 
> [major] This section talks about the case where legacy advertisements
> are already in use.  This document cannot Normatively define what
> those deployments do, because by definition they are not necessarily
> implementing this specification.  IOW, phrases like "OSPF routers in
> that domain SHOULD continue to advertise" are out of place.
> 
> [] Instead of trying to fix the text, I believe that the §8.1 already
> covers the scenarios discussed here.  Also, please consider
> "importing" the 7.* sub-sections from the ISIS draft.
> 
> ##PP
> done
> 
> ...
> 556	   In fact, there is at least one OSPF implementation that utilizes the
> 557	   link attributes advertised in TE Opaque LSAs [RFC3630] for Non-RSVP
> 558	   TE applications.  For example, this implementation of LFA and remote
> 559	   LFA utilizes links attributes such as Shared Risk Link Groups (SRLG)
> 560	   [RFC4203] and Admin Group [[RFC3630] advertised in TE Opaque LSAs.
> 561	   These applications are described in [RFC5286], [RFC7490], [RFC7916]
> 562	   and [RFC8102].
> 
> [nit] s/Non-RSVP TE/Non-RSVP-TE
> 
> 564	   When an OSPF routing domain includes routers using link attributes
> 565	   from the OSPFv2 TE Opaque LSAs or the OSPFv3 Intra-Area-TE-LSA for
> 566	   Non-RSVP TE applications defined in this document (i.e.  SRTE and
> 567	   LFA), OSPF routers in that domain SHOULD continue to advertise such
> 568	   OSPFv2 TE Opaque LSAs or the OSPFv3 Intra-Area-TE-LSA.  In such a
> 569	   deployment, the advertised attributes SHOULD be the same and Non-
> 570	   RSVP application access to link attributes is a matter of local
> 571	   policy.
> 
> [nit] s/Non-RSVP TE/Non-RSVP-TE
> 
> [nit] s/Non-RSVP/Non-RSVP-TE
> 
> 573	   When advertising link-attributes for any new applications other then
> 574	   RSVP-TE, SRTE or LFA, OSPF routers MUST NOT use TE Opaque LSA or
> 575	   OSPFv3 Intra-Area-TE-LSA.  Instead, advertisement in the OSPFv2
> 576	   Extended Link Attributes LSAs or OSPFv3 E-Router-LSA MUST be used.
> 
> [major] (A similar statement was made in §8.1.)  Why?  Just as the
> cases from §8.1, the application may run my itself on the network,
> and/or the attributes may be fully congruent.
> 
> 578	   It is RECOMMENDED to advertise link-attributes for RSVP-TE in the
> 579	   existing TE LSAs.
> 
> [major] When is it ok to not do so?  IOW, why isn't it "REQUIRED"?
> I'm asking this question as a matter of process, but the conditions
> laid out in §8.1 seems to already answer the question.  As I mentioned
> above, the cases in this section seem to already be covered in §8.1.
> 
> 
> ##PP
> I hope all the above comments were addressed by reusing and modifying
> ISIS pieces as needed.
> 
> 
> 
> 581	11.  Security Considerations
> ...
> 591	   Implementations MUST assure that malformed TLV and Sub-TLV defined in
> 592	   this document are detected and do not provide a vulnerability for
> 593	   attackers to crash the OSPF router or routing process.  Reception of
> 594	   a malformed TLV or Sub-TLV SHOULD be counted and/or logged for
> 595	   further analysis.  Logging of malformed TLVs and Sub-TLVs SHOULD be
> 596	   rate-limited to prevent a Denial of Service (DoS) attack (distributed
> 597	   or otherwise) from overloading the OSPF control plane.
> 
> [major] "Implementations MUST assure that malformed TLV and Sub-TLV
> defined in this document are detected and do not provide a
> vulnerability for attackers to crash the OSPF router or routing
> process."  How can this be Normatively enforced?  How is the
> individual ability of an implementation to avoid crashes required for
> interoperability?  IOW, I don't think you can use "MUST" in the
> sentence above.  s/MUST/must
> 
> ##PP
> Above text is used in several OSPF RFCs -  rfc8476 being an example. I
> change to "must".
> 
> 
> [major] What about vulnerabilities introduced by this document?  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 the ASLA sub-TLV is 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 other possible vulnerabilities introduced by the
> new encodings...  For example, a rogue router can render the data
> unusable by setting the incorrect bit-mask...  At best, this action
> could result in sub-optimal paths (by taking some links out of
> consideration)...  Consider adding text related to that.
> 
> ##PP
> I added the text similar to what is in the ISIS draft.
> 
> 
> 
> 599	12.  IANA Considerations
> 
> 601	12.1.  OSPFv2
> 
> 603	   OSPFv2 Extended Link TLV Sub-TLVs registry [RFC7684] defines sub-TLVs
> 604	   at any level of nesting for OSPFv2 Extended Link TLVs.  This
> 605	   specification updates OSPFv2 Extended Link TLV sub-TLVs registry with
> 606	   the following TLV types:
> 
> [nit] s/OSPFv2 Extended Link TLV/The OSPFv2 Extended Link TLV
> 
> ##PP
> done
> 
> [major] s/This specification updates OSPFv2 Extended Link TLV sub-TLVs
> registry with the following TLV types:/IANA has assigned the following
> TLV types:
> 
> ##PP
> done
> ...
> 633	12.2.  OSPFv3
> 
> 635	   OSPFv3 Extended LSA Sub-TLV Registry [RFC8362] defines sub-TLVs at
> 636	   any level of nesting for OSPFv3 Extended LSAs.  This specification
> 637	   updates OSPFv3 Extended LSA Sub-TLV Registry with the following TLV
> 638	   types:
> 
> [nit] s/OSPFv3 Extended LSA Sub-TLV/The OSPFv3 Extended LSA Sub-TLV
> 
> ##PP
> done
> 
> [major] s/This specification updates OSPFv3 Extended LSA Sub-TLV
> Registry with the following TLV types:/IANA has assigned the following
> TLV types:
> 
> ##PP
> done
> 
> 
> ...
> 737	15.2.  Informative References
> 
> [major] These references should be Normative:
> 
> ##PP
> done.
> 
> thanks,
> Peter
> 
> 
> 
> 739	   [I-D.ietf-isis-te-app]
> 740	              Ginsberg, L., Psenak, P., Previdi, S., Henderickx, W., and
> 741	              J. Drake, "IS-IS TE Attributes per application", draft-
> 742	              ietf-isis-te-app-08 (work in progress), October 2019.
> 
> 744	   [RFC2328]  Moy, J., "OSPF Version 2", STD 54, RFC 2328,
> 745	              DOI 10.17487/RFC2328, April 1998,
> 746	              <https://www.rfc-editor.org/info/rfc2328>.
> 
> 748	   [RFC4203]  Kompella, K., Ed. and Y. Rekhter, Ed., "OSPF Extensions in
> 749	              Support of Generalized Multi-Protocol Label Switching
> 750	              (GMPLS)", RFC 4203, DOI 10.17487/RFC4203, October 2005,
> 751	              <https://www.rfc-editor.org/info/rfc4203>.
> ...
> 776	   [RFC7471]  Giacalone, S., Ward, D., Drake, J., Atlas, A., and S.
> 777	              Previdi, "OSPF Traffic Engineering (TE) Metric
> 778	              Extensions", RFC 7471, DOI 10.17487/RFC7471, March 2015,
> 779	              <https://www.rfc-editor.org/info/rfc7471>.
> 
>       [Lsr] AD Review of draft-ietf-ospf-te-link-attr-r…  Alvaro Retana
> 
> 
> 
>