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 mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
