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

[Peter Psenak <[e-mail]>]

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