[Lsr] AD Review of draft-ietf-ospf-segment-routing-msd-15

[Alvaro Retana <[e-mail]>]

Dear authors:

I just finished reading this document.  I have several comments and
concerns that I included inline below.

While I do have some significant concerns (see below), I don't think there
are specific items that prevent the start of the IETF LC (they should not
be hard to address), so I'm getting that underway.  However, given the
close relationship to and dependency on
draft-ietf-isis-segment-routing-msd, I will schedule both of them on the
same IESG Telechat.

Thanks!

Alvaro.


...
76 1.  Introduction

78   When Segment Routing(SR) paths are computed by a centralized
79   controller, it is critical that the controller learns the Maximum SID
80   Depth(MSD) that can be imposed at each node/link on a given SR path
81   to insure that the SID stack depth of a computed path doesn't exceed
82   the number of SIDs the node is capable of imposing.

[nit] s/Depth(MSD)/Depth (MSD)

84   The PCEP SR extensions draft [I-D.ietf-pce-segment-routing] signals
85   MSD in SR PCE Capability TLV and METRIC Object.  However, if PCEP is
86   not supported/configured on the head-end of an SR tunnel or a
87   Binding-SID anchor node and controller do not participate in IGP

[nit] s/and controller do not participate/and the controller does not
participate

88   routing, it has no way to learn the MSD of nodes and links.  BGP-LS
89   [RFC7752] defines a way to expose topology and associated attributes
90   and capabilities of the nodes in that topology to a centralized
91   controller.  MSD signaling by BGP-LS has been defined in
92   [I-D.ietf-idr-bgp-ls-segment-routing-msd].  Typically, BGP-LS is
93   configured on a small number of nodes that do not necessarily act as
94   head-ends.  In order for BGP-LS to signal MSD for all the nodes and
95   links in the network MSD is relevant, MSD capabilites should be
96   advertised by every OSPF router in the network.

[nit] s/capabilites/capabilities

...
108   or SIDs associated with another dataplane e.g., IPv6.  Although MSD
109   advertisements are associated with Segment Routing, the
110   advertisements MAY be present even if Segment Routing itself is not
111   enabled.

[minor] Given that you're using Normative language...  It would be nice if
you expanded on the use of the MSD in a non-SR network.  Something simple
such as "a SID and a label are the same thing" would be enough.

113 1.1.  Conventions used in this document

115 1.1.1.  Terminology

[minor] Except for BMI/MSD, the other terms are not definitions, just
expansions.  Some of the abbreviations are already included in the RFC
Editor Abbreviations List [1].  In general, it would be better to just
expand on first use (BGP-LS, for example) is used *before* this section
than to have this section with expansions.

[1] https://www.rfc-editor.org/materials/abbrev.expansion.txt

...
152 2.  Node MSD Advertisement

154   The node MSD TLV within the body of the OSPF RI Opaque LSA is defined

[nit] A reference to rfc7770 would be nice.

155   to carry the provisioned SID depth of the router originating the RI
156   LSA.  Node MSD is the smallest MSD supported by the node on the set
157   of interfaces configured for use by the advertising IGP instance.
158   MSD values may be learned via a hardware API or may be provisioned..

160        0                   1                   2                   3
161        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1

163       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
164       |    Type                       |         Length                |
165       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
166       |         MSD Type and Value ...
167       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ ...

169                          Figure 1: Node MSD TLV

[nit] It would be nicer to display the actual pairs:

        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
        |   MSD-Type    | MSD-Value     |
        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


171   The Type: TBD1

173   Length: variable (minimum of 2, multiple of 2 octets) and represents
174   the total length of value field.

[nit] ...in octets.

176   Value: consists of one or more pairs of a 1 octet MSD-type and 1
177   octet MSD-Value.

[nit] There is no "Value" field illustrated above.  You might want to
reword a little.

...
188   This TLV is applicable to OSPFv2 and to OSPFv3 [RFC5838] and is
189   optional.  The scope of the advertisement is specific to the
190   deployment.

[minor] I'm confused by the reference to rfc5838.  It confuses me because
there are no references to the base specs (rfc2328/rfc5340), but (when it
seems that one is maybe used) you point at rfc5838 here...

192   When multiple Node MSD TLVs are received from a given router, the
193   receiver MUST use the first occurrence of the TLV in the Router
194   Information LSA.  If the Node MSD TLV appears in multiple Router
195   Information LSAs that have different flooding scopes, the Node MSD
196   TLV in the Router Information LSA with the area-scoped flooding scope
197   MUST be used.  If the Node MSD TLV appears in multiple Router
198   Information LSAs that have the same flooding scope, the Node MSD TLV
199   in the Router Information (RI) LSA with the numerically smallest
200   Instance ID MUST be used and subsequent instances of the Node MSD TLV
201   MUST be ignored.  The RI LSA can be advertised at any of the defined
202   opaque flooding scopes (link, area, or Autonomous System (AS)).  For
203   the purpose of Node MSD TLV advertisement, area-scoped flooding is
204   REQUIRED.

[major] The text above ends by saying that for the "Node MSD TLV
advertisement, area-scoped flooding is REQUIRED".  I take that to mean that
other scopes are not valid, is that true?

If so, then the rules seem to contradict themselves; specifically: "If the
Node MSD TLV appears in multiple Router Information LSAs that have the same
flooding scope..." seems to imply that receiving the Node MSD TLV in an
non-area scoped RI LSAs is ok.

206 3.  Link MSD sub-TLV
...
223   Type:

225   For OSPFv2, the Link level MSD value is advertised as an optional
226   Sub-TLV of the OSPFv2 Extended Link TLV as defined in [RFC7684], and
227   has value of TBD2.

229   For OSPFv3, the Link level MSD value is advertised as an optional
230   Sub-TLV of the E-Router-LSA TLV as defined in [RFC8362], and has
231   value of TBD3.

[nit] For both OSPFv2/OSPFv3, it would be clearer if "has a type of TBD"
(instead of "value") was used.  The "value" is used to mean different
things in the same sentence.

233   Length: variable and similar to that, defined in Section 2.

[major] Similar or the same?

235   Value: consists of one or more pairs of a 1 octet MSD-type and 1
236   octet MSD-Value.

[nit] There is no "Value" field illustrated above.  You might want to
reword a little.

...
248   Other MSD Types are reserved for future extensions.

[major] The MSD Types are not defined in this document...so they shouldn't
be reserved here...

250   If this TLV is advertised multiple times in the same OSPFv2 Extended
251   Link Opaque LSA, only the first instance of the TLV is used by
252   receiving OSPFv2 routers.  This situation SHOULD be logged as an
253   error.

[nit] s/this TLV/this sub-TLV

[major] Can we make the specification stronger?  Maybe "only the first
instance MUST be used"...

255   If this TLV is advertised multiple times for the same link in
256   different OSPFv2 Extended Link Opaque LSAs originated by the same
257   OSPFv2 router, the OSPFv2 Extended Link TLV in the OSPFv2 Extended
258   Link Opaque LSA with the smallest Opaque ID is used by receiving
259   OSPFv2 routers.  This situation may be logged as a warning.

[nit] s/this TLV/this sub-TLV

[major] Can we make the specification stronger?  Maybe "...with the
smallest Opaque ID MUST be used...".

[minor] Why is the importance of this last situation less, where a warning
may (non-normative) be logged?

[major] What about multiples in OSPFv3?

261 4.  Using Node and Link MSD Advertisements

[major] After reading this section, I still don't know how do use the
advertisements.  What should a receiver do with the values?  Maybe the use
is constrained to a controller...maybe the exact operation is our of the
scope of this document.  Either way, please say something.

...
279 5.  Base MPLS Imposition MSD

[minor] Even with the pointer to I-D.ietf-isis-segment-routing-msd, this
section is not needed in this document because the definition of the
MSD-Types is done elsewhere.  Please remove it.

...
301 7.  Security Considerations

303   Security concerns for OSPF are addressed in [RFC7474].  Further

[minor] That's a bold statement!  I'm sure those are not the only security
concerns...  It may be better to just point at the base specifications...

304   security analysis for OSPF protocol is done in [RFC6863] including
305   analysis of both the above documents.  Security considerations, as

[minor] Which documents?

306   specified by [RFC7770], [RFC7684] and [RFC8362] are applicable to
307   this document.

309   Advertisement of an incorrect MSD value may result: in a path
310   computation failing and the service unavailable or instantiation of a
311   path that can't be supported by the head-end (the node performing the
312   imposition).

[major] The paragraph above can also applied to modified information.  What
about privacy?  What are the issues with disclosure of the MSDs?

...
329 10.1.  Normative References

[minor] I don't think that rfc7474 needs to be Normative.

...
342   [RFC7474]  Bhatia, M., Hartman, S., Zhang, D., and A. Lindem, Ed.,
343              "Security Extension for OSPFv2 When Using Manual Key
344              Management", RFC 7474, DOI 10.17487/RFC7474, April 2015,
345              <https://www.rfc-editor.org/info/rfc7474>.

...
366 10.2.  Informative References

[nit] rfc8126 is not used.

...
403   [RFC8126]  Cotton, M., Leiba, B., and T. Narten, "Guidelines for
404              Writing an IANA Considerations Section in RFCs", BCP 26,
405              RFC 8126, DOI 10.17487/RFC8126, June 2017,
406              <https://www.rfc-editor.org/info/rfc8126>.