Last Call Review of draft-ietf-teas-lsp-diversity-07
review-ietf-teas-lsp-diversity-07-rtgdir-lc-decraene-2017-05-23-00

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir<http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir>

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.

Document: draft-ietf-teas-lsp-diversity-07
Reviewer: Bruno Decraene
Review Date: 2017/05/23
IETF LC End Date: not initiated AFAIK
Intended Status: Standard Track
Summary:
I have some minor concerns about this document that I think should be resolved
before publication. Comments: I have only very basic knowledge of RSVP-TE and
none of GMPLS. I found the document very clear. Thank you. Major Issues:  None
Minor Issues: To provide diversity, my understanding is that the signaling of
the second LSP is extended to detect the crossing of the first path, and then
is re-routed to avoid it. In some cases, I guess that providing path diversity
is not possible without re-routing the first path. It's not clear to me how
this is possible with this proposition. If it’s not, this limitation should
probably be highlighted somewhere in the document. (and if it is, sorry for
having missed it). Especially since this case seem to be in scope as per the
Introduction: "Similarly, an LSP from EN2 to EN3 traversing CN1 needs to be
diverse from an LSP from EN2 to EN3 going via CN4. [...]  This document
addresses these diversity requirements "
-----
§1.1
"There are scenarios in which the ENs have the following requirements"
OK. Are there other scenario? (seem so as per the wording). What are their
requirements and why aren’t they considered?
-----
"-  Both client and server understand the identifier."
Not sure what is meant by "understand". At this step, it's not clear why the
identifier can't be an any number/string that no one understand but is treated
as an opaque value/identifier.
-----
"The identifier is to be stable for a long period of time."
Easy comment but "long" is not very specific and different person may have
different interpretation. I guess that the goal is that the identifier be
stable for the duration of the diversity requirement.
-----
" These requirements are met by using the LSP identifier. The LSP
      identifier uniquely identifies an LSP in the network and
      comprises of the following fields: IPv4/IPv6 tunnel sender
      address, IPv4/IPv6 tunnel end point address, Tunnel ID, LSP ID,
      and Extended Tunnel ID. These fields are defined in [RFC3209],
      sections 4.6.1.1 and 4.6.2.1."
It's not clear to me that this choice meet the requirements. As per my quick
reading of RFC 3209, Tunnel ID remains constant for the lifetime of a tunnel.
If the node (e.g. EN1) reboot, it seems plausible that this Tunnel ID be
changed. That seems to be an issue given that this identifier seems to be
statically configured on the other node (e.g. EN2)
----
"In order to
      maintain diversity between these two connections within the core
      network, it is assumed that the core network implements Crankback
      Signaling [RFC4920]."
Don't you mean :s/assume/REQUIRED  ?
----
§1.2 PCE-allocated Identifier
If PCE is used to compute the paths, paths diversity needs to be handled by the
PCE (to compute diverse paths). In which case, the problem seems to be solved
with no need for further RSVP-TE extensions. Figure 2 seem to illustrate that
this section seem to consider the case where PCE is only partially used (in one
domain). This is not stated at the beginning of 1.2 hence this does not help
the reader to understand the case really being considered.
----
§1.3
"the concept of a Path Affinity Set (PAS) is defined for abstracting SRLG
information." It's not clear to me how the second node (EN2 using the example
from the introduction) is communicated this PAS from EN1, and how it's kept up
to date as the path used by EN1 may change. IOW, this PAS does not seem to
fulfill the 3 latest requirements listed in 1.1: "       -  It is necessary to
be able to reference the identifier even if the LSP referenced by it is not yet
signaled.
        -  The identifier is to be stable for a long period of time.
        -  The identifier is to be stable even when the referenced LSP is
        rerouted."

It's not clear to me what benefits this PAS brings compared to the client
initiated identifier. While it adds the above disadvantages.
---
"The means by which the processing node determines the path corresponding to
the PAS is beyond the scope of this document." But this doesn't remove the
problem to be solved. You seem to assume that there is one single database,
typically centralized. An alternative option may have been to cypher the
"detailled SRLG list". Why has this option been discarded?

"The means to distribute the PAS information within the core network is beyond
the scope of this document. For example, the
      PAS and the associated SRLG information can be distributed within the
      core network by an Interior Gateway Protocol (IGP) or by other means such
      as configuration."
I don't think the use of the IGP would be such a good fit, in term of frequency
of update (ok, this is deployment dependent) and scalability (a priori the
number of PAS is o(N^2), N being Core Nodes.)
---
Abstract: " This document specifies three new route exclusion types."
From the IANA section and the document ToC, I'm seen only 2 types: "IPv4
Diversity subobject", "IPv6 Diversity subobject"
---
"IANA section"
You do not define a registry nor a registration policy for the "DI Type"
---
§2.1
The A-Flags field is 4 bits longs and this document already allocates 4 flags,
meaning that there is no room for extension (although there is a 4 bits
Reserved field available)
---
§2.1
If A-Flags "0x01 = Destination node exception" and "0x04 = Penultimate node
exception" are both set, it seems that the penultimate link (between these 2
nodes) should also be excluded from the exclusion list. If so this should be
specified.
---
"When the diversity identifier type is set to "IPv4/ IPv6
              Network Assigned Identifier", the value MUST be set to the
              IPv4/ IPv6 address of the node publishing the Path
              Affinity Set (PAS)."
Given that the way the PAS is advertised (I read 'publish') is out of scope of
this document, I'd rather not use the term "publishing". I guess "allocating"
would be better and probably more accurate.
---
§3
"the diversity subobject must be kept while other subobjects may be removed."
Do you mean :s/must/MUST  ? (it looks to me that this is required for interop,
hence a MUST)
---
"all Diversity subobjects in an XRO/ EXRS MUST contain the same Diversity
Identifier Type." Could you clarify the reason? What if someone wants to be
diverse with 2 others LSP: one intra-domain using a client-Initiated
Identifier, and another one inter-domain using a "PCE-allocated Identifier"?
---
§2.3
"it MUST return a PathErr with the error code TBA3 "Routing Problem" and error
value of "Unsupported Diversity Identifier Type" I think I would propose "code
"Routing Problem" (24) and error value of "Unsupported Diversity Identifier
Type (TBA3)"
----
§2.3
"The transit nodes in a domain and the domain egress node SHOULD NOT process
the signaled diversity information." This does not seem to match " In order to
      maintain diversity between these two connections within the core
      network, it is assumed that the core network implements Crankback
      Signaling [RFC4920]."
As I understand, by the latter, that all RSVP-TE nodes need to process the
diversity information. But this may comes from my lack of knowledge of RSVP-TE.
---
Nits:
Impressive list of contributors: 1,5 pages, 16 persons (in additions to 4
authors)

Thanks,
Regards,
--Bruno