Dear authors: This is my review of draft-ietf-isis-mpls-elc-10. I reviewed this document alongside draft-ietf-ospf-mpls-elc-12, so many comments are the same/similar. Thank you for the work in both of them!
Besides the in-line comments, I want to point out here that this specification is incomplete. It needs to have (1) a formal description of the new MSD-Type (similar to §5/rfc8491), and (2) a discussion of the interaction with the BMI-MSD. I will progress both documents together, so I will wait for both of them to address my comments before starting their IETF LC. Thanks!! Alvaro. [Line numbers from idnits.] ... 18 Abstract 20 Multiprotocol Label Switching (MPLS) has defined a mechanism to load- 21 balance traffic flows using Entropy Labels (EL). An ingress Label 22 Switching Router (LSR) cannot insert ELs for packets going into a 23 given Label Switched Path (LSP) unless an egress LSR has indicated 24 via signaling that it has the capability to process ELs, referred to 25 as Entropy Label Capability (ELC), on that tunnel. In addition, it 26 would be useful for ingress LSRs to know each LSR's capability for 27 reading the maximum label stack depth and performing EL-based load- 28 balancing, referred to as Entropy Readable Label Depth (ERLD). This 29 document defines a mechanism to signal these two capabilities using 30 IS-IS. These mechanisms are particularly useful, where label 31 advertisements are done via protocols like IS-IS. [nit] s/as Entropy Label Capability/as the Entropy Label Capability [minor] "protocols like IS-IS" That last sentence sounds as if there were other options; for example advertise labels with OSPF and then use the extensions here. It's just a minor point, but I think that maybe that last sentence is not needed. ... 81 1. Introduction 83 [RFC6790] describes a method to load-balance Multiprotocol Label 84 Switching (MPLS) traffic flows using Entropy Labels (EL). "The Use 85 of Entropy Labels in MPLS Forwarding" [RFC6790] introduces the 86 concept of Entropy Label Capability (ELC) and defines the signalings 87 of this capability via MPLS signaling protocols. Recently, 88 mechanisms have been defined to signal labels via link-state Interior 89 Gateway Protocols (IGP) such as IS-IS 90 [I-D.ietf-isis-segment-routing-extensions]. In such scenarios, the 91 defined signaling mechanisms are inadequate. This draft defines a 92 mechanism to signal the ELC using IS-IS. This mechanism is useful 93 when the label advertisement is also done via IS-IS. [nit] s/"The Use of Entropy Labels in MPLS Forwarding" [RFC6790]/It also [nit] s/signalings/signaling [nit] "In such scenarios, the defined signaling mechanisms are inadequate." Take this sentence out: the rest of the paragraph is enough. 95 In addition, in the cases where LSPs are used for whatever reasons 96 (e.g., SR-MPLS [I-D.ietf-spring-segment-routing-mpls]), it would be 97 useful for ingress LSRs to know each intermediate LSR's capability of 98 reading the maximum label stack depth and performing EL-based load- 99 balancing. This capability, referred to as Entropy Readable Label 100 Depth (ERLD) as defined in [I-D.ietf-mpls-spring-entropy-label] may 101 be used by ingress LSRs to determine the position of the EL label in 102 the stack, and whether it's necessary to insert multiple ELs at 103 different positions in the label stack. [nit] s/in the cases where LSPs are used for whatever reasons/in cases where LSPs are used 105 2. Terminology 107 This memo makes use of the terms defined in [RFC6790], [RFC4971] and 108 [I-D.ietf-mpls-spring-entropy-label]. [minor] I'm not sure why rfc4971 is referenced here; what terminology is needed from it? ... 116 3. Advertising ELC Using IS-IS 118 Even though ELC is a property of the node, in some cases it is 119 advantageous to associate and advertise the ELC with a prefix. In a 120 multi-area network, routers may not know the identity of the prefix 121 originator in a remote area, or may not know the capabilities of such 122 originator. Similarly in a multi-domain network, the identity of the 123 prefix originator and its capabilities may not be known to the 124 ingress LSR. [minor] Is there a difference that are you trying to highlight between multi-area and multi-domain? The last two sentences seem redundant to me; using "domain" should be enough. 126 One bit of the "Bit Values for Prefix Attribute Flags Sub-TLV" 127 registry defined in [RFC7794] (Bit 3 is desired) is to be assigned by 128 the IANA for the ELC. If a router has multiple line cards, the 129 router MUST NOT announce the ELC for any prefixes that are locally 130 attached unless all of its line-cards are capable of processing ELs. 131 If a router supports ELs on all of its line-cards, it SHOULD set the 132 ELC for every local host prefix it advertises in IS-IS. [major] The first sentence is not needed because IANA has already assigned the bit, and any requests should be in the IANA Considerations section. Perhaps change to something like: Bit 3 in the Prefix Attribute Flags [RFC7794] is used as the ECL Flag (E-flag), as shown in Figure 1. [major] From a general router architecture point of view, I understand what you mean by line-card. But, strictly speaking from a specification point of view, what is a line-card? Would using "interface" instead be an acceptable generalization? [minor] Is there a difference between "prefixes that are locally attached" and a "local host prefix"? Are all locally-attached prefixes host prefixes (/32 or /128)? [major] "it SHOULD set the ELC for every local host prefix" If ELs are supported in all the interfaces, when would a router not set the ELC? IOW, why is "MUST" not used instead of "SHOULD"? [major/related] The last two sentences seem to be redundant -- I think that only the second one is needed; suggestion (assuming my interpretation of the questions above): If a router supports ELs on all of its interfaces, it MUST set the E-flag (ELC Flag) for every local prefix it advertises. 134 0 1 2 3 4 5 6 7... 135 +-+-+-+-+-+-+-+-+... 136 |X|R|N|E| ... 137 +-+-+-+-+-+-+-+-+... 138 Figure 1: Prefix Attribute Flags 140 E-flag: ELC Flag (Bit 3) 141 Set for local host prefix of the originating node 142 if it supports ELC. [nit] Justify the description in line with the text (move it to the left). 144 When a router leaks a prefix between two levels (upwards or 145 downwards), it MUST preserve the ELC signaling for this prefix. [nit] Going up is not really leaking. ;-) [minor] An Informative reference to rfc5302 would be nice. Suggestion> When a router distributes a prefix between two levels [RFC5302] it MUST preserve the E-flag setting. 147 When redistributing a prefix between two IS-IS protocol instances or 148 redistributing from another protocol to an IS-IS protocol instance, a 149 router SHOULD preserve the ELC signaling for that prefix. The exact 150 mechanism used to exchange ELC between protocol instances running on 151 an ASBR is outside of the scope of this document and is 152 implementation specific. [minor] s/ELC signaling/ELC setting [nit] Please expand ASBR. [nit] s/ and is implementation specific./. 154 4. Acknowledgements [major] Move this section to just before the References. ... 161 5. Advertising ERLD Using IS-IS [major] draft-ietf-mpls-spring-entropy-label says that "To advertise an ERLD value, a SPRING router: MUST be entropy label capable". This requirement must be translated to this document so that the ERLD is only advertised if the ELC is also advertised. I'm assuming that the ERLD should be ignored if the ELC is not advertised -- but that should be explicitly defined as well. If the ELC is advertised, but the ERLD isn't, what value should be assumed, 0? 163 A new MSD-type of the Node MSD ((Maximum SID Depth) sub-TLV 164 [RFC8491], called ERLD is defined to advertise the ERLD of a given 165 router. As shown in Figure 2, it is formatted as described in 166 [RFC8491] with a new MSD-Type code to be assigned by IANA (the type 167 code of 2 is desired) and the Value field is set to the ERLD in the 168 range between 0 to 255. The scope of the advertisement depends on 169 the application. If a router has multiple line-cards with different 170 capabilities of reading the maximum label stack depth, the router 171 MUST advertise the smallest one. [minor] "new MSD-type...called ERLD is defined to advertise the ERLD" I suggest that you call the new MSD ERLD-MSD, to differentiate ERLD from ERLD. ;-) [major] s/a new MSD-Type code to be assigned by IANA (the type code of 2 is desired)/the MSD-Type set to 2 IANA already assigned. [minor] s/Value/MSD-Value ... 180 When the ERLD MSD-Type is received in the Link MSD Sub-TLV, it MUST 181 be ignored. [nit] s/When the/If the 183 6. Signaling ELC and ERLD in BGP-LS ... 188 The ELC Flag included in the Prefix Attribute Flags sub-TLV, as 189 defined in Section 3, is advertised using the Prefix Attribute Flags 190 TLV (TLV 1170) of the BGP-LS IPv4/IPv6 Prefix NLRI Attribute as 191 defined in section 2.3.2 of 192 [I-D.ietf-idr-bgp-ls-segment-routing-ext]. [nit] Suggestion> The ELC is advertised using the Prefix Attribute Flags TLV as defined in [I-D.ietf-idr-bgp-ls-segment-routing-ext]. 194 The ERLD MSD-type introduced for IS-IS in Section 5 is advertised 195 using the Node MSD TLV (TLV 266) of the BGP-LS Node NLRI Attribute as 196 defined in section 3 of [I-D.ietf-idr-bgp-ls-segment-routing-msd]. [nit] Suggestion> The ERLD-MSD is advertised using the Node MSD TLV as defined in [I-D.ietf-idr-bgp-ls-segment-routing-msd]. 198 7. IANA Considerations 200 IANA is requested to allocate the E-bit (bit position 3 is desired) 201 from the "Bit Values for Prefix Attribute Flags Sub-TLV" registry. 203 IANA is requested to allocate a MSD type (the type code of 2 is 204 desired) from the "IGP MSD Types" registry for ERLD. [major] IANA has already assigned the values. Suggestion> Early allocation has been done by IANA for this document as follows: - Bit 3 in the Bit Values for Prefix Attribute Flags Sub-TLV registry has been assigned to the ELC Flag. IANA is asked to update the registry to reflect the name used in this document: ECL Flag (E-flag). - Type 2 in the IGP MSD-Types registry has been assigned for the ERLD-MSD. IANA is asked to update the registry to reflect the name used in this document: ERLD-MSD. 206 8. Security Considerations 208 The security considerations as described in [RFC4971] nd 209 [I-D.ietf-mpls-spring-entropy-label] are applicable to this document. [minor] Why? Also, I think that some of the other references should be added here. Suggestion: This document specifies the ability to advertise additional node capabilities using IS-IS and BGP-LS. As such, the security considerations as described in [RFC4971], [RFC7752], [RFC7794], [RFC8491], [I-D.ietf-idr-bgp-ls-segment-routing-ext], [I-D.ietf-idr-bgp-ls-segment-routing-msd] and [I-D.ietf-mpls-spring-entropy-label] are applicable to this document. 211 Incorrectly setting the E flag (ELC capable) (during origination, 212 leaking or redistribution) may lead to black-holing of the traffic on 213 the egress node. [minor] s/E flag (ELC capable)/E flag [minor] s/during origination, leaking or redistribution/during origination or redistribution [major] "...may lead to black-holing of the traffic on the egress node." I'm not sure I understand how, but the ELC advertisement should be accompanied by the ERLD-MSD -- see my questions at the beginning of §5. 215 Incorrectly setting of the ERLD value may lead to poor load-balancing 216 of the traffic. [minor] "may lead to poor load-balancing" If the ERLD is low, then the traffic may not be load balanced at all...that is not "poor", it is "0". ... 244 10.1. Normative References ... 259 [I-D.ietf-mpls-spring-entropy-label] 260 Kini, S., Kompella, K., Sivabalan, S., Litkowski, S., 261 Shakir, R., and J. Tantsura, "Entropy label for SPRING 262 tunnels", draft-ietf-mpls-spring-entropy-label-12 (work in 263 progress), July 2018. == Outdated reference: draft-ietf-mpls-spring-entropy-label has been published as RFC 8662 265 [I-D.ietf-spring-segment-routing-mpls] 266 Bashandy, A., Filsfils, C., Previdi, S., Decraene, B., 267 Litkowski, S., and R. Shakir, "Segment Routing with MPLS 268 data plane", draft-ietf-spring-segment-routing-mpls-22 269 (work in progress), May 2019. == Outdated reference: draft-ietf-spring-segment-routing-mpls has been published as RFC 8660 [minor] This reference can be Informative. ... 276 [RFC4971] Vasseur, JP., Ed., Shen, N., Ed., and R. Aggarwal, Ed., 277 "Intermediate System to Intermediate System (IS-IS) 278 Extensions for Advertising Router Information", RFC 4971, 279 DOI 10.17487/RFC4971, July 2007, 280 <https://www.rfc-editor.org/info/rfc4971>. ** Obsolete normative reference: RFC 4971 ** Replace with rfc7981 ... 307 10.2. Informative References 309 [I-D.ietf-isis-segment-routing-extensions] 310 Previdi, S., Ginsberg, L., Filsfils, C., Bashandy, A., 311 Gredler, H., and B. Decraene, "IS-IS Extensions for 312 Segment Routing", draft-ietf-isis-segment-routing- 313 extensions-25 (work in progress), May 2019. == Outdated reference: draft-ietf-isis-segment-routing-extensions has been published as RFC 8667 _______________________________________________ Lsr mailing list [email protected] https://www.ietf.org/mailman/listinfo/lsr
