[Lsr] AD Review of draft-ietf-isis-segment-routing-extensions-22
Alvaro Retana <aretana.ietf@gmail.com> Wed, 20 March 2019 22:20 UTC
Return-Path: <aretana.ietf@gmail.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 63A2812798C; Wed, 20 Mar 2019 15:20:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.996
X-Spam-Level:
X-Spam-Status: No, score=-1.996 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, NORMAL_HTTP_TO_IP=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, THIS_AD=0.001, UNPARSEABLE_RELAY=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id a4UoZ6mb1a5R; Wed, 20 Mar 2019 15:20:45 -0700 (PDT)
Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6921A13118F; Wed, 20 Mar 2019 15:20:45 -0700 (PDT)
Received: by mail-ot1-x329.google.com with SMTP id q24so3727781otk.0; Wed, 20 Mar 2019 15:20:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc; bh=6lYKVLknMaUhOvWE4/5JgVQS9g7IkvRQoYgU4ijXuoU=; b=Pz7qjpHpxVIZrSC83MsCOO+wVzJ0YefZIr0Fb9vCJqMjK4Y8CA9TnHBsWXqRpUW6pT oPH0aEBGxcta94AB0qYpeBe/VHGQR1jGJ5F9bJTI6TUkd9tDf8v+OSm2e2DMiF9TJT/q 5NMLLNCBy2sacGe4GUo6gwpdmqwRl8iMP7qnaZq5/+gi0nCwHYxP/VK7WTDyHKYJSFQe +K7q1fDWGb/ULgFs/nIyjMM81t29k1LcsmfKrmJDmLGnVKp3RGFx77zVq6sNTDeAfppq 8OY9qezTQe6prWO4qsbuVzW5Trz0LBd5xN3vm0nqifSMvALMhKaCpaw2Zn5L9SjdO0EW KJbQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc; bh=6lYKVLknMaUhOvWE4/5JgVQS9g7IkvRQoYgU4ijXuoU=; b=NWctglBc8EZimdpILg/e7SoPDFdRSm/TDvKAXw95UYVp54veLI2S4xBAfz79eS+q+P /7dSGjNXN/OfGSMyTmW5hw5hzxmwMh+zkt0+2mTnMJGxiHYwpI6fEnX5YYZc6lZV0jfh CznaJCRbr+y+WGV8xkMe+Qua1d68A7loUZMgy9cVRcjOMvth8eb2750z2ZNlaGR4gdep vVGrI/eE+7S7V3aeKBHMBtw4TtEK6em0cW0Tf+2crbE93aWIKcj7RgNbgIlztfE6ZgS6 16POTab0eurYPuq+z/FIiPucKzMAdKBWgQ+EntazB6MdjWoTOXXUDYjL7M+CgYLYYT9K n8AQ==
X-Gm-Message-State: APjAAAUkV63l5FVaSHMo2g2gwSfrWjLMB10mHtAPSY75sDU4QiJkOGUG AjvXCi+nDiMuXByxF5w+DM8wCtKDvlYsKHVOo3PxMg==
X-Google-Smtp-Source: APXvYqxC3/iA2GSX0R7dhRopAS1DEkBkXJD7f86tccVYVy4GQXWwSIwVIiYFWrqD2Z4w3uY+E/Bkj6fH8Mei6LemszU=
X-Received: by 2002:a9d:6348:: with SMTP id y8mr299966otk.287.1553120443555; Wed, 20 Mar 2019 15:20:43 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Wed, 20 Mar 2019 15:20:42 -0700
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Wed, 20 Mar 2019 15:20:42 -0700
Message-ID: <CAMMESsz7nvqpSHY78j_3Gkksbf=SiyMfv8pV83iRN9GUfJJ_YA@mail.gmail.com>
To: draft-ietf-isis-segment-routing-extensions@ietf.org
Cc: Uma Chunduri <uma.chunduri@huawei.com>, lsr-chairs@ietf.org, lsr@ietf.org
Content-Type: multipart/alternative; boundary="00000000000054e5c605848e084d"
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/trLsjLG0hE5hBgY2xn9l6zJHPv0>
Subject: [Lsr] AD Review of draft-ietf-isis-segment-routing-extensions-22
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 20 Mar 2019 22:20:53 -0000
Dear authors: I just finished reviewing this document. Please take a look at the in-line comments below. In general, I think that some more work is needed. I will wait for that before starting the IETF Last Call. There is a significant issue that I want to highlight here (there are also related comments in-line): - §2.1.1.2 talks about interaction with rfc7794 and a transition period which apparently ends in this specification not being used. Am I reading that correctly? Thanks! Alvaro. [Line numbers come from idnits.] ... 107 1. Introduction 109 Segment Routing (SR) allows for a flexible definition of end-to-end 110 paths within IGP topologies by encoding paths as sequences of 111 topological sub-paths, called "segments". These segments are 112 advertised by the link-state routing protocols (IS-IS and OSPF). 113 Prefix segments represent an ecmp-aware shortest-path to a prefix (or 114 a node), as per the state of the IGP topology. Adjacency segments 115 represent a hop over a specific adjacency between two nodes in the 116 IGP. A prefix segment is typically a multi-hop path while an 117 adjacency segment, in most of the cases, is a one-hop path. SR's 118 control-plane can be applied to both IPv6 and MPLS data-planes, and 119 do not require any additional signaling (other than the regular IGP). 120 For example, when used in MPLS networks, SR paths do not require any 121 LDP or RSVP-TE signaling. Still, SR can interoperate in the presence 122 of LSPs established with RSVP or LDP. [nit] s/ecmp-aware/ECMP-aware 124 There are additional segment types, e.g., Binding SID defined in 125 [RFC8402]. This draft also defines an advertisement for one type of 126 BindingSID: the Mirror Context segment. [nit] s/BindingSID/Binding SID 128 This draft describes the necessary IS-IS extensions that need to be 129 introduced for Segment Routing operating on an MPLS data-plane. 131 Segment Routing architecture is described in [RFC8402]. [nit] s/Segment Routing architecture/The Segment Routing architecture 133 Segment Routing use cases are described in [RFC7855]. 135 2. Segment Routing Identifiers 137 Segment Routing architecture ([RFC8402]) defines different types of 138 Segment Identifiers (SID). This document defines the IS-IS encodings 139 for the IGP-Prefix-SID, the IGP-Adjacency-SID, the IGP-LAN-Adjacency- 140 SID and the Binding-SID. [nit] s/([RFC8402])/[RFC8402] [nit] s/Segment Routing architecture/The Segment Routing architecture [major] rfc8402 doesn't use the names above. Please be consistent. 142 2.1. Prefix Segment Identifier (Prefix-SID Sub-TLV) 144 A new IS-IS sub-TLV is defined: the Prefix Segment Identifier sub-TLV 145 (Prefix-SID sub-TLV). 147 The Prefix-SID sub-TLV carries the Segment Routing IGP-Prefix-SID as 148 defined in [RFC8402]. The 'Prefix SID' MUST be unique within a given 149 IGP domain (when the L-flag is not set). The 'Prefix SID' MUST carry 150 an index (when the V-flag is not set) that determines the actual SID/ 151 label value inside the set of all advertised SID/label ranges of a 152 given router. A receiving router uses the index to determine the 153 actual SID/label value in order to construct forwarding state to a 154 particular destination router. [major] What if the Prefix SID is not unique? What should the receiver do? [major] "...MUST carry an index...that determines the actual SID/label value..." What are you trying to specify with this "MUST"? The description of the V-flag (below) already points at the value being an index, but the qualification ("that determines...") makes the enforcement of this "MUST" more complex: the receiver has to verify that the index actually results in a SID/label inside a range for it to be valid. What should the receiver do if that is not the case? Suggestion: Rearrange the text so that the TLV and its fields are described first. I think that then some of the text above won't be needed. 156 In many use-cases a 'stable transport' IP Address is overloaded as an 157 identifier of a given node. Because the IP Prefixes may be re- 158 advertised into other levels there may be some ambiguity (e.g. 159 Originating router vs. L1L2 router) for which node a particular IP 160 prefix serves as identifier. The Prefix-SID sub-TLV contains the 161 necessary flags to disambiguate IP Prefix to node mappings. 162 Furthermore if a given node has several 'stable transport' IP 163 addresses there are flags to differentiate those among other IP 164 Prefixes advertised from a given node. [minor] Again, this text may be clearer if the TLV description was presented first. If you decide not to rearrange the text, at least put forward references so readers can look forward to find out how the statement above is achieved. ... 180 When the IP Reachability TLV is propagated across level boundaries, 181 the Prefix-SID sub-TLV SHOULD be kept. [major] When would this sub-TLV not be kept? IOW, why is that not a "MUST"? §2.1.2: "The Prefix-SID sub-TLV MUST be preserved when the IP Reachability TLV gets propagated across level boundaries." I'm not saying that one way is correct...either one may be. Just be consistent. [minor] I'm assuming that "IP Reachability TLV" means one of the TLVs above, true? It may be worth clarifying that. 183 The Prefix-SID sub-TLV has the following format: 185 0 1 2 3 186 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 187 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 188 | Type | Length | Flags | Algorithm | 189 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 190 | SID/Index/Label (variable) | 191 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 193 where: 195 Type: TBD, suggested value 3 [major] This value (and all the others in the document) are not "suggested values", they have been already allocated by IANA. Please update the individual descriptions and the IANA Considerations section accordingly. 197 Length: variable. [minor] Yes, variable, but it can only have two values: 5 or 6. 199 Flags: 1 octet field of following flags: 201 0 1 2 3 4 5 6 7 202 +-+-+-+-+-+-+-+-+ 203 |R|N|P|E|V|L| | 204 +-+-+-+-+-+-+-+-+ 206 where: 208 R-Flag: Re-advertisement flag. If set, then the prefix to 209 which this Prefix-SID is attached, has been propagated by the 210 router either from another level (i.e., from level-1 to level-2 211 or the opposite) or from redistribution (e.g.: from another 212 protocol). [major] Should it be used to avoid looping as well? Or is there no risk of that in this case? ... 236 Other bits: MUST be zero when originated and ignored when 237 received. [major] Do you expect IANA to handle the assignment of those other bits? 239 Algorithm: the router may use various algorithms when calculating 240 reachability to other nodes or to prefixes attached to these 241 nodes. Algorithms identifiers are defined in Section 3.2. 242 Examples of these algorithms are metric based Shortest Path First 243 (SPF), various sorts of Constrained SPF, etc. The algorithm field 244 of the Prefix-SID contains the identifier of the algorithm the 245 router has used in order to compute the reachability of the prefix 246 to which the Prefix-SID is associated. 248 At origination, the Prefix-SID algorithm field MUST be set to 0 or 249 to any value advertised in the SR-Algorithm sub-TLV (Section 3.2). [major] What should a receiver do if the Algorithm value doesn't match what has been advertised in the SR-Algorithm Sub-TLV? ... 282 2.1.1.2. R and N Flags 284 The R-Flag MUST be set for prefixes that are not local to the router 285 and either: 287 advertised because of propagation (Level-1 into Level-2); 289 advertised because of leaking (Level-2 into Level-1); 291 advertised because of redistribution (e.g.: from another 292 protocol). 294 In the case where a Level-1-2 router has local interface addresses 295 configured in one level, it may also propagate these addresses into 296 the other level. In such case, the Level-1-2 router MUST NOT set the 297 R bit. The R-bit MUST be set only for prefixes that are not local to 298 the router and advertised by the router because of propagation and/or 299 leaking. [minor] This last sentence is redundant with the start of this section. Please specify the behavior just once. 301 The N-Flag is used in order to define a Node-SID. A router MAY set 302 the N-Flag only if all of the following conditions are met: 304 The prefix to which the Prefix-SID is attached is local to the 305 router (i.e., the prefix is configured on one of the local 306 interfaces, e.g., a 'stable transport' loopback). 308 The prefix to which the Prefix-SID is attached MUST have a Prefix 309 length of either /32 (IPv4) or /128 (IPv6). [major] "The prefix...MUST have a Prefix length..." This condition is not worded as one; perhaps: "The prefix...has a Prefix length..." [minor] Why is the N-Flag needed? If the setting is optional ("MAY"), then not setting it doesn't imply that the conditions are not met... [major] It is clearly an error if both the R-Flag and N-Flag are set, right? What should the receiver do in that case? 311 The router MUST ignore the N-Flag on a received Prefix-SID if the 312 prefix has a Prefix length different than /32 (IPv4) or /128 (IPv6). 314 [RFC7794] also defines the N and R flags and with the same semantics 315 of the equivalent flags defined in this document. There will be a 316 transition period where both sets of flags will be used and 317 eventually only the flags of the Prefix Attributes will remain. 318 During the transition period implementations supporting the N and R 319 flags defined in this document and the N and R flags defined in 320 [RFC7794] MUST advertise and parse all flags. In case the received 321 flags have different values, the value of the flags defined in 322 [RFC7794] prevails. [major] "...transition period where both sets of flags will be used and eventually only the flags of the Prefix Attributes will remain" To be honest, you lost me here? Please explain what will the transition period be between? [major] If "both sets of flags will be used and eventually only the [already defined] flags of the Prefix Attributes will remain", why do we need the new ones? [major] "MUST advertise and parse all flags" What does this mean? By "all flags" I guess you mean the N and R flags...but if both this document and rfc7794 are implemented, it seems to me that the specification above is really meaningless. [major] "In case the received flags have different values..." For a second it seemed to me that what you really wanted was for the flags in this document to behave the same way as the flags defined in rfc7794...the easy solution to that (and to avoid my questions listed before rfc7794 was mentioned) would have been to just point at rfc7794 as the place where the Flags are defined. However, it looks like the definitions (at least for the R-Flag) are not the same. Is that on purpose or is it just an oversight? One piece of information that might make this discussion relevant is understanding the significance of these Flags...but neither document offers any specific use, beyond simply "it may be useful for other routers to know" (rfc7794, §2.2). 324 2.1.1.3. E and P Flags 326 When calculating the outgoing label for the prefix, the router MUST 327 take into account E and P flags advertised by the next-hop router, if 328 next-hop router advertised the SID for the prefix. This MUST be done 329 regardless of next-hop router contributing to the best path to the 330 prefix or not. [major] "the router MUST take into account" What normative action is standardized with that phrase? It would be clearer/better if the behavior is explicitly specified in function of the flags (as sone below). s/MUST/must [major] "This MUST be done regardless of next-hop router contributing to the best path to the prefix or not." This what? I'm assuming the text refers to "take into account"... Same comment as before: s/MUST/must ... 366 2.1.2. Prefix-SID Propagation 368 The Prefix-SID sub-TLV MUST be preserved when the IP Reachability TLV 369 gets propagated across level boundaries. [major] §2.1 says that it SHOULD. Please be consistent...and, specify behavior only once. 371 The level-1-2 router that propagates the Prefix-SID sub-TLV between 372 levels MUST set the R-flag. 374 If the Prefix-SID contains a global index (L and V flags unset) and 375 it is propagated as such (with L and V flags unset), the value of the 376 index MUST be preserved when propagated between levels. 378 The level-1-2 router that propagates the Prefix-SID sub-TLV between 379 levels MAY change the setting of the L and V flags in case a local 380 label value is encoded in the Prefix-SID instead of the received 381 value. [major] Up to now, I had been assuming that propagating and preserving meant not changing the sub-TLV (except for the R-Flag). But these last paragraphs indicate that not only can some flags be changed, but the contents can completely change. It would be ideal if the valid operations on the sub-TLV were discussed (or at least referenced) when the preserving behavior was first introduced (I think in §2.1). ... 409 2.2.1. Adjacency Segment Identifier (Adj-SID) Sub-TLV 411 The following format is defined for the Adj-SID sub-TLV: 413 0 1 2 3 414 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 415 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 416 | Type | Length | Flags | Weight | 417 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 418 | SID/Label/Index (variable) | 419 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 421 where: 423 Type: TBD, suggested value 31 425 Length: variable. [minor] Yes, variable, but it can only have two values... 427 Flags: 1 octet field of following flags: 429 0 1 2 3 4 5 6 7 430 +-+-+-+-+-+-+-+-+ 431 |F|B|V|L|S|P| | 432 +-+-+-+-+-+-+-+-+ 434 where: 436 F-Flag: Address-Family flag. If unset, then the Adj-SID refers 437 to an adjacency with outgoing IPv4 encapsulation. If set then 438 the Adj-SID refers to an adjacency with outgoing IPv6 439 encapsulation. [major] This document describes the operation on the MPLS data plane, right? The F-Flag talks about the outgoing encapsulation...but that could simply be the next label in the MPLS stack, right? What am I missing? I get the impression that there's a relationship between what this Flag points to and something else...but I just don't know what. 441 B-Flag: Backup flag. If set, the Adj-SID is eligible for 442 protection (e.g.: using IPFRR or MPLS-FRR) as described in 443 [RFC8355]. [major] Because the specification of the B-Flag depends directly on rfc8355, it should be a Normative reference. Looking at rfc8355, I couldn't find a place where it talks about eligibility for protection...just text about potential strategies, but (honestly) nothing that I would call Normative. Please clarify. Perhaps rfc8402 might be a better reference. [major] I'm assuming that this description (pointing at rfc8355), plus the text below about allocation of Adj-SIDs should result in some action related to the protection...what is that? IOW, presented with the B-Flag set, what action should a node take? 445 V-Flag: Value flag. If set, then the Adj-SID carries a value. 446 By default the flag is SET. [minor] Is the description the same at the V-Flag in §2.1? If so, then indicating that, or at least also pointing out here that the value is carried instead of an index would be helpful. 448 L-Flag: Local Flag. If set, then the value/index carried by 449 the Adj-SID has local significance. By default the flag is 450 SET. [major] Looking back at §2.1.1.1, what are the conditions for which the combination of the V and L-Flags are valid/invalid? Do the same conditions apply? ... 460 Other bits: MUST be zero when originated and ignored when 461 received. [major] Do you expect IANA to handle the assignment of those other bits? ... 469 An SR capable router MAY allocate an Adj-SID for each of its 470 adjacencies and SHOULD set the B-Flag when the adjacency is 471 eligible for protection (IP or MPLS). [major] If eligible for protection, when would the router not set the B-Flag? IOW, why is that not a MUST? Beyond that, I think that the use of SHOULD is in conflict with the definition of the B-Flag (above): "If set, the Adj-SID is eligible for protection...", which implies that it isn't eligible if it's not set...but the SHOULD opens the door for not setting the Flag... ... 479 When the P-flag is not set, the Adj-SID MAY be persistent. When 480 the P-flag is set, the Adj-SID MUST be persistent. [major] If the adjacency is persistent, why/when would the P-Flag not be set? ... 488 2.2.2. Adjacency Segment Identifiers in LANs ... 537 Other bits: MUST be zero when originated and ignored when 538 received. [major] Do you expect IANA to handle the assignment of those other bits? ... 544 System-ID: 6 octets of IS-IS System-ID of length "ID Length" as 545 defined in [ISO10589]. [major] Which System-ID? [major] "6 octets of...length "ID Length"" ?? 547 SID/Index/Label as defined in Section 2.1.1.1. 549 Multiple LAN-Adj-SID sub-TLVs MAY be encoded. Note that this sub-TLV 550 MUST NOT appear in TLV 141. 552 When the P-flag is not set, the LAN-Adj-SID MAY be persistent. When 553 the P-flag is set, the LAN-Adj-SID MUST be persistent. 555 In case one TLV-22/23/222/223 (reporting the adjacency to the DIS) 556 can't contain the whole set of LAN-Adj-SID sub-TLVs, multiple 557 advertisements of the adjacency to the DIS MUST be used and all 558 advertisements MUST have the same metric. 560 Each router within the level, by receiving the DIS PN LSP as well as 561 the non-PN LSP of each router in the LAN, is capable of 562 reconstructing the LAN topology as well as the set of Adj-SID each 563 router uses for each of its neighbors. 565 A label is encoded in 3 octets (in the 20 rightmost bits). 567 An index is encoded in 4 octets. [minor] Some of the information above (for example, the paragraph about the P-Flag and these last 2 sentences) seems unnecessary given that the description of these fields is already done somewhere else. 569 2.3. SID/Label Sub-TLV 571 The SID/Label sub-TLV may be present in the following TLVs/sub-TLVs 572 defined in this document: 574 SR-Capabilities Sub-TLV (Section 3.1) 576 SR Local Block Sub-TLV (Section 3.3) 578 SID/Label Binding TLV (Section 2.4) 580 Multi-Topology SID/Label Binding TLV (Section 2.5) [nit] It might have been nice, in line with the OSPF work, to flip Sections 2 and 3. Just a nit... 582 Note that the code point used in all of the above cases is the SID/ 583 Label Sub-TLV code point assigned by IANA in the "sub-TLVs for TLV 584 149 and 150" registry. [major] Because this document is setting up that new registry, we don't have to wait for IANA to assign anything: the document can specify it. IOW, the paragraph above is not needed. 586 The SID/Label sub-TLV contains a SID or a MPLS Label. The SID/Label 587 sub-TLV has the following format: 589 0 1 2 3 590 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 591 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 592 | Type | Length | 593 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 594 | SID/Label (variable) | 595 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 597 where: 599 Type: TBD, suggested value 1 601 Length: variable [major] Yes, variable...but right now only one value is defined. There's a finite number of possibilities... 602 SID/Label: if length is set to 3 then the 20 rightmost bits 603 represent a MPLS label. [major] Is that it..? What about the SID? 605 2.4. SID/Label Binding TLV ... 619 The SID/Label Binding TLV has Type TBD (suggested value 149), and has 620 the following format: [nit] No need to talk about the Type twice. 622 0 1 2 3 623 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 624 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 625 | Type | Length | Flags | RESERVED | 626 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 627 | Range | Prefix Length | Prefix | 628 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 629 // Prefix (continued, variable) // 630 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 631 | SubTLVs (variable) | 632 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 634 Figure 1: SID/Label Binding TLV format 636 o Type: TBD, suggested value 149 ... 657 2.4.1. Flags ... 671 M-Flag: Mirror Context flag. Set if the advertised SID 672 corresponds to a mirrored context. The use of the M flag is 673 described in [RFC8402]. [major] Where? Note that rfc8402 says that "The Mirror SID is advertised using the Binding segment defined in SR IGP protocol extensions [ISIS-SR-EXT].", pointing back at this document. [minor] Some places use *-Flag and others "* flag", please be consistent. 675 S-Flag: If set, the SID/Label Binding TLV SHOULD be flooded across 676 the entire routing domain. If the S flag is not set, the SID/ 677 Label Binding TLV MUST NOT be leaked between levels. This bit 678 MUST NOT be altered during the TLV leaking. [major] If the S-Flag is set, when would the TLV not be flooded across the entire domain? IOW, why is the SHOULD not a MUST? 680 D-Flag: when the SID/Label Binding TLV is leaked from level-2 to 681 level-1, the D bit MUST be set. Otherwise, this bit MUST be 682 clear. SID/Label Binding TLVs with the D bit set MUST NOT be 683 leaked from level-1 to level-2. This is to prevent TLV looping 684 across levels. [minor] What about leaking from L2 to L1 with the Flag set? [minor] The text uses Flag, but in some cases bit is used as well. Please be consistent. ... 695 An implementation MAY decide not to honor the S-flag in order not 696 to leak Binding TLV's between levels (for policy reasons). In all 697 cases, the D flag MUST always be set by any router leaking the 698 Binding TLV from level-2 into level-1 and MUST be checked when 699 propagating the Binding TLV from level-1 into level-2. If the D 700 flag is set, the Binding TLV MUST NOT be propagated into level-2. [major] s/implementation MAY decide/implementation may decide There's no Normative action there. [major] Regardless of whether MAY/may is used, the statement in the first sentence contradicts the "MUST NOT be leaked" Normative statement. Given that there are exceptions, maybe "SHOULD NOT" would be more appropriate. [major] This last paragraph seems to contain the same information as the text describing the S-Flag. Please don't duplicate specifications. 702 Other bits: MUST be zero when originated and ignored when 703 received. [major] Do you expect IANA to handle the assignment of those other bits? 705 2.4.2. Range 707 The 'Range' field provides the ability to specify a range of 708 addresses and their associated Prefix SIDs. This advertisement 709 supports the SRMS functionality. It is essentially a compression 710 scheme to distribute a continuous Prefix and their continuous, 711 corresponding SID/Label Block. If a single SID is advertised then 712 the range field MUST be set to one. For range advertisements > 1, 713 the number of addresses that need to be mapped into a Prefix-SID and 714 the starting value of the Prefix-SID range. [nit] The last sentence sounds incomplete... 716 Example 1: if the following router addresses (loopback addresses) 717 need to be mapped into the corresponding Prefix SID indexes. [minor] Suggestion: move the examples to the end of §2.4...after all the fields have been discussed. 719 Router-A: 192.0.2.1/32, Prefix-SID: Index 1 720 Router-B: 192.0.2.2/32, Prefix-SID: Index 2 721 Router-C: 192.0.2.3/32, Prefix-SID: Index 3 722 Router-D: 192.0.2.4/32, Prefix-SID: Index 4 724 0 1 2 3 725 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 726 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 727 | Type | Length |0|0| | RESERVED | 728 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 729 | Range = 4 | /32 | 192 | 730 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 731 | .0 | .2 | .1 |Prefix-SID Type| 732 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 733 | sub-TLV Length| Flags | Algorithm | | 734 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 735 | 1 | 736 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ [nit] The Prefix Length should be "32", not "/32". [nit] The Prefix itself is one field (in the description), but it looks like 4 separate fields above...and the numbers should not have "." in them. [minor] It looks like the fields starting with "Prefix-SID Type" belong to the Prefix-SID Sub-TLV, but that is not explained anywhere (yet). The Algorithm field is not set...nor are the Flags. 738 Example-2: If the following prefixes need to be mapped into the 739 corresponding Prefix-SID indexes: 741 10.1.1/24, Prefix-SID: Index 51 742 10.1.2/24, Prefix-SID: Index 52 743 10.1.3/24, Prefix-SID: Index 53 744 10.1.4/24, Prefix-SID: Index 54 745 10.1.5/24, Prefix-SID: Index 55 746 10.1.6/24, Prefix-SID: Index 56 747 10.1.7/24, Prefix-SID: Index 57 [major] Please use addresses in the rfc5737 reserved ranges. [minor] It would have been nice to see an IPv6 example (see rfc3849). 749 0 1 2 3 750 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 751 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 752 | Type | Length |0|0| | RESERVED | 753 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 754 | Range = 7 | /24 | 10 | 755 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 756 | .1 | .1 |Prefix-SID Type| sub-TLV Length| 757 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 758 | Flags | Algorithm | | 759 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 760 | 51 | 761 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ [] Same comments as above. ... 778 2.4.3. Prefix Length, Prefix ... 784 The 'Prefix Length' field contains the length of the prefix in bits. 785 Only the most significant octets of the Prefix are encoded. (i.e., 1 786 octet for prefix length 1 up to 8, 2 octets for prefix length 9 to 787 16, 3 octets for prefix length 17 up to 24 and 4 octets for prefix 788 length 25 up to 32, ...., 16 octets for prefix length 113 up to 128). [nit] s/encoded. (i.e.,/encoded (i.e., 790 2.4.4. Mapping Server Prefix-SID 792 The Prefix-SID sub-TLV (suggested value 3) is defined in Section 2.1 793 and contains the SID/index/label value associated with the prefix and 794 range. The Prefix-SID SubTLV MUST be present in the SID/Label 795 Binding TLV unless the M-flag is set in the Flags field of the parent 796 TLV. [major] Does it then mean that the Prefix-SID SubTLV must not be present if the M-Flag is not set? [nit] s/SubTLV/Sub-TLV (as it is used almost everywhere else) 798 A node receiving an SRMS entry for a prefix MUST check the existence 799 of such prefix in its link-state database prior to consider and use 800 the associated SID. [major] What does it mean for the prefix to exist in the link-state database? Does it mean that it must have been advertised in a "normal" routing TLV, or something else? I ask not only to clarify, but because §2.4.3 says that the "'Prefix' does not need to correspond to a routable prefix of the originating node". [Ah...I think I understand. Because "the mapping server does not specify the originator of a prefix" (§2.4.4.2), then the prefix is obviously not routable in the context of the originating node (the server)...but it should be routable in general. Is that it?] 802 2.4.4.1. Prefix-SID Flags 804 The Prefix-SID flags are defined in Section 2.1. The Mapping Server 805 MAY advertise a mapping with the N flag set when the prefix being 806 mapped is known in the link-state topology with a mask length of 32 807 (IPv4) or 128 (IPv6) and when the prefix represents a node. The 808 mechanisms through which the operator defines that a prefix 809 represents a node are outside the scope of this document (typically 810 it will be through configuration). [major] §2.1.1.2 says that the "router MAY set the N-Flag only if... the prefix to which the Prefix-SID is attached MUST have a Prefix length of either /32 (IPv4) or /128 (IPv6)." That is similar to what the text above says (but the text talks about "the prefix...known in the link-state topology"), and it is related to the last question in the previous section. Does the prefix being in the link-state database/topology mean that the match is exact (to the prefix advertised), or is a supernet ok? 812 The other flags defined in Section 2.1 are not used by the Mapping 813 Server and MUST be ignored at reception. [major] How does the receiver know that the sub-TLV was originated by a Mapping Server? Is it the case that it would originate a Binding TLV? IOW, would the other flags always be ignored when the sub-TLV is included in the Binding TLV (but not other TLVs)? Does this apply also to the Multi-Topology SID/Label Binding TLV (§2.5)? 815 2.4.4.2. PHP Behavior when using Mapping Server Advertisements ... 823 o A prefix reachability advertisement for the prefix has been 824 received which includes the Extended Reachability Attribute Flags 825 sub-TLV ([RFC7794]). [nit] s/([RFC7794])/[RFC7794] 827 o X and R flags are both set to 0 in the Extended Reachability 828 Attribute Flags sub-TLV. 830 In the absence of an Extended Reachability Attribute Flags sub-TLV 831 ([RFC7794]) the A flag in the binding TLV indicates that the 832 originator of a prefix reachability advertisement is directly 833 connected to the prefix and thus PHP MUST be done by the neighbors of 834 the router originating the prefix reachability advertisement. Note 835 that A-flag is only valid in the original area in which the Binding 836 TLV is advertised. [nit] s/([RFC7794])/[RFC7794] [major] §2.4.1 says that "if the Binding TLV is leaked...the A-flag MUST be cleared", which is not the same as not considering it valid (ignoring, as described above). In any case, please be specific about the functionality in the description. 838 2.4.4.3. Prefix-SID Algorithm 840 The algorithm field contains the identifier of the algorithm the 841 router MUST use in order to compute reachability to the range of 842 prefixes. Use of the algorithm field is described in Section 2.1. [major nit] At first glance, it looks like this section is not needed because it repeats what §2.1 already says about the algorithm. But this paragraph refers to "the algorithm the router MUST use in order to compute reachability", in contrast to the definition of the field in §2.1: "The algorithm field...contains the identifier of the algorithm the router has used in order to compute the reachability..." ... 901 3.1. SR-Capabilities Sub-TLV ... 909 The Router Capability TLV specifies flags that control its 910 advertisement. The SR Capabilities sub-TLV MUST be propagated 911 throughout the level and MUST NOT be advertised across level 912 boundaries. Therefore Router Capability TLV distribution flags are 913 set accordingly, i.e., the S flag in the Router Capability TLV 914 ([RFC7981]) MUST be unset. [nit] s/([RFC7981])/[RFC7981] ... 943 I-Flag: MPLS IPv4 flag. If set, then the router is capable of 944 processing SR MPLS encapsulated IPv4 packets on all interfaces. 946 V-Flag: MPLS IPv6 flag. If set, then the router is capable of 947 processing SR MPLS encapsulated IPv6 packets on all interfaces. [minor] Just curious: How are these flags used? Given that the data plane we're dealing with is MPLS... ... 981 The originating router MUST ensure the order is same after a graceful 982 restart (using checkpointing, non-volatile storage or any other 983 mechanism) in order to guarantee the same order before and after GR. [nit] s/order is same/order is the same ... 1015 3.2. SR-Algorithm Sub-TLV ... 1046 The SR-Algorithm sub-TLV is optional, it MAY only appear a single 1047 time inside the Router Capability TLV. [major] "MAY only appear a single time" means that it can appear more than once. If it does show up more than once, what should the the receiver do? 1049 When the originating router does not advertise the SR-Algorithm sub- 1050 TLV, then all the Prefix-SIDs advertised by the router MUST have 1051 algorithm field set to 0. Any receiving router MUST assume SPF 1052 algorithm (i.e., Shortest Path First). [minor] It seems to me that this part of the specification would fit better in §2.1. [major] Besides maybe being redundant, what is specified in the last sentence? If the indication of the algorithm is explicit, then I don't understand what "MUST assume" is trying to mandate. 1054 When the originating router does advertise the SR-Algorithm sub-TLV, 1055 then algorithm 0 MUST be present while algorithm 1 MAY be present. [nit] I think that it is clear that algorithm 1 is optional. I would take that last part out just because other algorithms may be defined in the future...and the important part at this point is the inclusion of algorithm 0. 1057 The SR-Algorithm sub-TLV has following format: [nit] s/has following/has the following ... 1073 Algorithm: 1 octet of algorithm Section 2.1 [minor] The Algorithm field is described in this section, not in 2.1. 1075 3.3. SR Local Block Sub-TLV 1077 The SR Local Block (SRLB) Sub-TLV contains the range of labels the 1078 node has reserved for local SIDs. Local SIDs are used, e.g., for 1079 Adjacency-SIDs, and may also be allocated by other components than 1080 IS-IS protocol. As an example, an application or a controller may 1081 instruct the router to allocate a specific local SID. Therefore, in 1082 order for such applications or controllers to know what are the local 1083 SIDs available in the router, it is required that the router 1084 advertises its SRLB. [nit] s/than IS-IS protocol/than the IS-IS protocol ... 1122 The originating router MUST NOT advertise overlapping ranges. [major] What should the receiver do if the ranges overlap? I'm assuming the same thing as what was specified before...please be specific. 1124 It is important to note that each time a SID from the SRLB is 1125 allocated, it SHOULD also be reported to all components (e.g.: 1126 controller or applications) in order for these components to have an 1127 up-to-date view of the current SRLB allocation and in order to avoid 1128 collision between allocation instructions. 1130 Within the context of IS-IS, the reporting of local SIDs is done 1131 through IS-IS Sub-TLVs such as the Adjacency-SID. However, the 1132 reporting of allocated local SIDs may also be done through other 1133 means and protocols which mechanisms are outside the scope of this 1134 document. [major] "SHOULD also be reported to all components" Because that is outside the scope, then it shouldn't be a Normative statement. s/SHOULD/should ... 1170 4. Non backward compatible changes with prior versions of this document [major] I find this section unnecessary. If you want to keep it, please move it to an Appendix and don't use Normative language. ... 1190 5. IANA Considerations 1192 This documents request allocation for the following TLVs and subTLVs. [minor] IANA may be ok, but I find the formatting below a little confusing. A table may be a better option. ... 1313 This document creates the following sub-TLV Registry: ... [major] Please indicate what the range is. 1333 6. Security Considerations 1335 With the use of the extensions defined in this document, IS-IS 1336 carries information which will be used to program the MPLS data plane 1337 [RFC3031]. In general, the same types of attacks that can be carried 1338 out on the IP/IPv6 control plane can be carried out on the MPLS 1339 control plane resulting in traffic being misrouted in the respective 1340 data planes. However, the latter may be more difficult to detect and 1341 isolate. Existing security extensions as described in [RFC5304] and 1342 [RFC5310] apply to these segment routing extensions. [nit] Maybe break up the paragraph into multiple ones with independent points. [major] With the last sentence, are you implying that authentication is required when IS-IS is carrying SR extensions? If so, please be explicit. Why would these extensions be different than any other extension? [minor] The companion OSPF documents added the following paragraph to the Security Considerations as a result of one of the reviews...perhaps consider including something similar: 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 OSPFv2 router or routing process. Reception of malformed TLV or Sub-TLV SHOULD be counted and/or logged for further analysis. Logging of malformed TLVs and Sub-TLVs SHOULD be rate-limited to prevent a Denial of Service (DoS) attack (distributed or otherwise) from overloading the OSPF control plane. ... 1415 9.1. Normative References ... [minor] I think that these references can be Informative: 1463 [RFC5305] Li, T. and H. Smit, "IS-IS Extensions for Traffic 1464 Engineering", RFC 5305, DOI 10.17487/RFC5305, October 1465 2008, <https://www.rfc-editor.org/info/rfc5305>. 1467 [RFC5308] Hopps, C., "Routing IPv6 with IS-IS", RFC 5308, 1468 DOI 10.17487/RFC5308, October 2008, 1469 <https://www.rfc-editor.org/info/rfc5308>. ... 1527 Les Ginsberg (editor) 1528 Cisco Systems, Inc. 1529 IT [nit] I didn't know you had moved. ;-)
- [Lsr] AD Review of draft-ietf-isis-segment-routin… Alvaro Retana
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Acee Lindem (acee)
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Les Ginsberg (ginsberg)
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Alvaro Retana
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Les Ginsberg (ginsberg)
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Les Ginsberg (ginsberg)
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Alvaro Retana
- Re: [Lsr] AD Review of draft-ietf-isis-segment-ro… Les Ginsberg (ginsberg)