[Lsr] RtgDir Review: draft-ietf-isis-segment-routing-msd-15

[Julien Meuric <[e-mail]>]

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

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-isis-segment-routing-msd-15
Reviewer: Julien Meuric (with some inputs from Bruno Decraene)
Review Date: 2018-09-07
IETF LC End Date: 2018-09-12
Intended Status: ST

*Summary:*
I have some minor concerns about this document that I think should be
resolved before publication.

*Comments:"
The document is well focused on a solution to a clearly scoped problem.
The defined encodings are simple and clear. All the same, there are a
few loose wordings, implicit assumptions or fuzzy zones which require
clarification before publication.

*Minor issues:*

- The abstract uses the acronym "SID", however:
    * It should be expanded at first use,
    * It should be defined, e.g. by adding a (normative) reference to
RFC 8402 (at least in the introduction),
    * The SR context is missing and should be explicitly mentioned
(adding a phrase such as "in a Segment Routing-enabled network" would be
enough).

- OLD
   Path Computation Element Protocol (PCEP) SR extensions draft
   [I-D.ietf-pce-segment-routing] signals MSD in SR Path Computation
   Element (PCE) Capability TLV and METRIC Object.
  NEW
   The Path Computation Element communication Protocol (PCEP) SR
   extension document [I-D.ietf-pce-segment-routing] defines how to
   signal MSD using the SR-PCE-CAPABILITY sub-TLV (per node) and
   the METRIC object (per request).

- The introduction says that the solution to complement BGP-LS lies in
IS-IS (the OSPF draft claiming the counterpart on its side). This is a
bit rough, the sentence 2 paragraph below already does the necessary
scoping job: "This document defines an extension to <your favorite IGP
here>". I suggest to temper the early sentence by rephrasing the
beginning of page 3 into: "MSD capabilities should be advertised by
every router in the network involved in the IGP."

- The wording of the following sentence is not clear: "Note that in a
non-SR MPLS network, label depth is what is defined by the MSD
advertisements." Is it intended to say: "Note that MSD advertisements
are applicable beyond SR-enabled networks and may refer to label depth
in MPLS networks."?

- "SID" may deserve to join the terminology section.

- s/SIDs a node or a link on a node can support./SIDs supported by a
node or a link on a node./

- The same ASCII art is used for the 2 figures (sections 2 and 3). It
would be nice to specialize each one a little bit by explicitly adding
their respective codepoint in the type field (23 and 15).

- Section 3 says "any other value represents that of the link when used
as an outgoing link." Is it not excessive to be that prescriptive about
outgoing vs. incoming? E.g., if an implementation was to advertise ERLD
as a link MSD, that would rather refer to a capability of the
incominglink. This specification could be left to each MSD-Type definition.

- In section 4: s/the Link MSD MUST take preference/the Link MSD MUST
take precedence/

- In section 5, BMI-MSD is defined as "the total number of MPLS  labels
which can be imposed" (which is OK when the incoming packet is
unlabled). When the incoming packet is labeled (e.g. use of segment
routing binding SID), if the incoming label is to be "replaced" by N
outgoing labels, what processing model is assumed when advertising the MSD:
 * one swap and one imposition of N-1 labels?
 * one pop and one imposition of N labels?

- For the BMI-MSD use case, the draft seems to clearly target a value
attached to the outgoing link. However, this capability on a router is
highly hardware-dependent: some implementations may push a stack on the
egress linecard while some may perform it on the ingress linecard. The
I-D seems to make some implicit hardware assumption and the current link
MSD advertisement is not suited to describe these possible combinations
of hardware implementations. This needs to be addressed somehow.

- Section 7 says that "an MSD that is incorrect, may result in [...]
instantiation of a path that can't be supported". It would be more
accurate to mention "instantiation attempt", as a proper state control
mechanism is expected to deny unsupported instantiation (e.g. PCEP
errors for "invalid objects" include "Unsupported number of SR-ERO
subobjects").

*Nits:*
- s/(IS-IS) Router to advertise/(IS-IS) router to advertise/
- s/link a given SR path/each node/link of a given SR path/
- s/path doesn't exceed/path does not exceed/
- s/and associated attributes and capabilities/as well as associated
attributes and capabilities/
- s/it is expected, that/it is expected that/
---
- s/an IANA managed registry/an IANA-managed registry/
- s/defined by this document/defined by this document:/
- s/path that can't be/path that cannot be/
---

Regards,

Julien