[prev in list] [next in list] [prev in thread] [next in thread]
List: ms-ospf
Subject: Re: [Lsr] AD Review of draft-ietf-isis-segment-routing-extensions-22
From: "Les Ginsberg (ginsberg)" <ginsberg () cisco ! com>
Date: 2019-03-29 5:55:09
Message-ID: MN2PR11MB36475B77F634FCD144F24BAEC15A0 () MN2PR11MB3647 ! namprd11 ! prod ! outlook ! com
[Download RAW message or body]
Alvaro -
Thanx for the excellent review.
V23 of the draft has been posted
Please review and verify that all comments have been addressed to your satisfaction - \
subject to my responses inline below.
> -----Original Message-----
> From: Alvaro Retana <aretana.ietf@gmail.com>
> Sent: Wednesday, March 20, 2019 3:21 PM
> To: draft-ietf-isis-segment-routing-extensions@ietf.org
> Cc: Uma Chunduri <uma.chunduri@huawei.com>; lsr-chairs@ietf.org;
> lsr@ietf.org
> Subject: AD Review of draft-ietf-isis-segment-routing-extensions-22
>
> 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
[Les:] Done
>
>
> 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
>
[Les:] Done
>
> 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.
>
[Les:] Done.
>
> 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?
>
[Les:] This is not within the purview of the IGP drafts. This is now covered in \
https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-18#section-2.5 and \
it is not appropriate for IGP drafts to discuss this issue nor reference the solution \
as it is not the IGP which is responsible for implementing the solution.
> [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.
>
[Les:] Done.
>
> 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.
>
[Les:] Text reordered as you suggested.
>
> ...
> 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"?
>
[Les:] This text was removed as it is not needed and - as you point out - contradicts \
earlier text which specifies 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.
>
[Les:] You are correct. Changed to the generic term Prefix Reachability.
>
> 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.
>
[Les:] Done.
>
> 197 Length: variable.
>
> [minor] Yes, variable, but it can only have two values: 5 or 6.
>
[Les:] Done.
>
> 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?
>
[Les:] Prevention of looping is done through use of the D-bit as specified in RFC \
5305. R flag has no role in that. It exists to identify a prefix which is not locally \
connected to the advertising router (either leaked between levels or redistributed \
from another protocol).
>
> ...
> 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?
>
[Les:] No. Bit definitions specifically associated with a TLV/sub-TLV dedicated to \
particular functionality historically are NOT managed via an IANA registry. Any \
additional bit definitions required will be specified in a new draft which defines \
the additional functionality.
>
> 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?
>
[Les:] This is explicitly stated at the end of Section 2.1:
" A router receiving a Prefix-SID from a remote node and with an
algorithm value that such remote node has not advertised in the
SR-Algorithm sub-TLV (Section 3.2) MUST ignore the Prefix-SID 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.
>
[Les:] Done.
>
> 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..."
>
[Les:] Done.
> [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...
>
[Les:] Please see https://tools.ietf.org/html/rfc7794#section-2.1
> [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?
>
[Les:] It is legal to have R and N flags set. This occurs when a prefix which has the \
N flag set is leaked into another level.
>
> 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?
>
[Les:] The text has been revised to simply state the preference for RFC 7794 values \
over Prefix-SID values.
> 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).
>
[Les:] Prefixes with N flag set serve as Node Identifiers for the originating node. \
These addresses are then candidates to be used in policies including explicit paths \
used for repair and/or microloop avoidance.
>
> 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
>
[Les:] This text has been removed. What is left is simply the normative description \
of behaviors associated with E/P flags.
>
> ...
> 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.
>
[Les:] Done
> 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).
>
[Les:] This text has been removed.
>
> ...
> 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...
>
[Les:] Done
> 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.
>
[Les:] Text revised. Forwarding plane implementations frequently are address-family \
specific - which is what the text was trying to describe.
>
> 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?
>
>
[Les:] Correct reference is (as you pointed out) to RFC 8402. Text updated and \
reference to RFC8355 removed.
> 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.
>
[Les:] Done.
>
> 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?
>
[Les:] This is explicitly stated in Section 2.1.1.1.
>
> ...
> 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?
>
[Les:] No - similar to previous case answered above.
>
> ...
> 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...
>
[Les:] Offending text removed. B-flag is defined in Section2.2.1
>
> ...
> 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?
>
[Les:] It is possible that implementation of a "dynamically allocated" adj-sid could \
result in the same value being allocated after reboot. However, P-flag set requires \
that behavior whereas P-flag unset allows an implementation to alter the \
adjacency-sid value following reboot but does not require that behavior.
>
> ...
> 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?
>
[Les:] No - as per previous answer.
>
> ...
> 544 System-ID: 6 octets of IS-IS System-ID of length "ID Length" as
> 545 defined in [ISO10589].
>
> [major] Which System-ID?
>
[Les:] Text has been altered to indicate it is the system-id of the neighbor.
> [major] "6 octets of...length "ID Length"" ??
>
[Les:] Done.
>
> 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.
>
[Les:] Unnecessary text removed.
>
> 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...
>
[Les:] Sorry but I chose not to do this. There are other differences in ordering \
between the two specs - in part because the protocol specific nature of the \
advertisements is somewhat different. Reordering just these two sections would not \
result in identical ordering.
> 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.
>
[Les:] Done.
>
> 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...
>
[Les:] Finite perhaps, but certainly quite a few legal values due to the variable \
length of the prefix field. Text unchanged.
>
> 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?
>
[Les:] Added definitions for 3 and 4.
>
> 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.
>
[Les:] Revised text to avoid circular references.
> [minor] Some places use *-Flag and others "* flag", please be consistent.
>
[Les:] Done
>
> 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?
>
[Les:] SHOULD is used because of the option to ignore the S-flag setting and suppress \
inter-level leaking.
>
> 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?
>
[Les:] Please read https://tools.ietf.org/html/rfc7775#section-2 which discusses this \
issue in detail.
> [minor] The text uses Flag, but in some cases bit is used as well. Please
> be consistent.
>
[Les:] Done
>
> ...
> 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.
>
[Les:] Done.
> [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.
>
[Les:] Altered to SHOULD - which works with the MAY/may change you suggested
> [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?
>
[Les:] No - see previous answers.
> 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...
>
[Les:] Corrected.
> 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.
>
[Les:] Done.
> 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".
>
[Les:] Done.
> [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.
>
[Les:] Reformatted to be correct.
But we want to illustrate that the number of bytes in the Prefix is variable based on \
bit length specified.
> [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.
[Les:] Text reordered.
>
> 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.
>
[Les:] Unfortunately this is not possible. What we want to illustrate here is that \
the encoding of the starting prefix itself might be abbreviated based on the prefix \
mask. To do so we MUST use an example prefix of /24 or shorter mask length - which \
means we have to use addresses that are not specified in RFC 5737.
> [minor] It would have been nice to see an IPv6 example (see rfc3849).
>
[Les:] Added.
> 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.
>
[Les:] Done.
>
> ...
> 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.,
>
[Les:] Done.
>
> 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?
>
[Les:] Correct. This has been clarified.
> [nit] s/SubTLV/Sub-TLV (as it is used almost everywhere else)
>
[Les:] Done.
>
> 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?]
>
[Les:] Confusing text removed.
>
> 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?
>
[Les:] Confusing text removed.
>
> 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)?
>
[Les:] "Yes" to all of your questions.
>
> 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]
>
[Les:] Done
> 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]
[Les:] Done
>
> [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.
>
[Les:] Revised text provided.
>
> 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..."
>
[Les:] Unnecessary text removed.
>
> ...
> 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]
>
[Les:] Done.
>
> ...
> 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...
[Les:] This indicates whether the advertising router supports MPLS for IPv4 and/or \
IPv6. Not all implementations support both.
>
>
> ...
> 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
>
[Les:] Done
>
> ...
> 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?
>
[Les:] Text has been added to state how to handle this case.
>
> 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.
>
[Les:] Text has been modified to address the above issues.
>
> 1057 The SR-Algorithm sub-TLV has following format:
>
> [nit] s/has following/has the following
>
[Les:] Done
>
> ...
> 1073 Algorithm: 1 octet of algorithm Section 2.1
>
> [minor] The Algorithm field is described in this section, not in 2.1.
>
[Les:] Done
>
> 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
[Les:] Done.
>
>
> ...
> 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.
>
[Les:] Similar question is relevant to SRGB and the behavior has been specified in \
https://tools.ietf.org/html/draft-ietf-spring-segment-routing-mpls-18#section-2.3 and \
this draft references it. I would do the same here, but the SR MPLS draft does not \
specify behavior for SRLB - even though the referenced section is entitled "Segment \
Routing Global Block and Local Block". Best solution I would think is to update SR \
MPKS draft so that a reference to it could be added here.
>
> 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
>
[Les:] Done.
>
> ...
> 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.
>
[Les:] I have removed it. This change was made a long time ago - I do not think we \
need to continue to be concerned about this interoperability issue.
>
> ...
> 1190 5. IANA Considerations
>
> 1192 This documents request allocation for the following TLVs and subTLVs
_______________________________________________
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic