incorporated and new version pushed many thanks for careful review
-- tony On Tue, Sep 20, 2022 at 10:16 PM John Scudder <jgs= [email protected]> wrote: > WG, > > I’d like to highlight that I’ve filled in (what I believe was) a missing > registry creation in the IANA Considerations section, and proposed that the > registration procedures be Expert Review like the other IS-IS registries. I > don’t think this will be controversial (it just follows the established > pattern) but I do want to highlight it. > ack > > Dear Authors, > > Thanks for your patience. Here’s my review of your document. > > For the most part I’ve supplied my questions and comments in the form of > an edited copy of the draft. Minor editorial suggestions I’ve made in place > without further comment, more substantive questions and comments are done > in-line and prefixed with “jgs:”. You can use your favorite diff tool to > review them; I’ve attached the iddiff output for your convenience if you’d > like to use it. I’ve also pasted a traditional diff below in case you want > to use it for in-line reply. I’d appreciate feedback regarding whether you > found this a useful way to receive my comments as compared to a more > traditional numbered list of comments with selective quotation from the > draft. > > Thanks, > > —John > > --- draft-ietf-lsr-isis-flood-reflection-08.txt 2022-09-20 > 15:29:50.000000000 -0400 > +++ draft-ietf-lsr-isis-flood-reflection-08-jgs-comments.txt 2022-09-20 > 16:07:33.000000000 -0400 > @@ -18,15 +18,15 @@ > > Abstract > > - This document describes a backwards compatible, optional IS-IS > + This document describes a backward-compatible, optional IS-IS > extension that allows the creation of IS-IS flood reflection > topologies. Flood reflection permits topologies in which L1 areas > provide transit forwarding for L2 using all available L1 nodes > internally. It accomplishes this by creating L2 flood reflection > adjacencies within each L1 area. Those adjacencies are used to flood > L2 LSPDUs and are used in the L2 SPF computation. However, they are > - not utilized for forwarding within the flood reflection cluster > - except in pathological cases mentioned in Section 7. This > + not ordinarily utilized for forwarding within the flood reflection > cluster. > + This > ack > arrangement gives the L2 topology significantly better scaling > properties than traditionally used flat designs. As an additional > benefit, only those routers directly participating in flood > @@ -34,6 +34,10 @@ > incremental deployment of scalable L1 transit areas in an existing, > previously flat network design, without the necessity of upgrading > all routers in the network. > + > +jgs: Note I suggested removing the forward reference to Section 7. > +Although accurate, ISTM that this level of detail doesn't go in the > +Abstract. > ack > > Requirements Language > > @@ -130,7 +134,7 @@ > Due to the inherent properties of link-state protocols the number of > IS-IS routers within a flooding domain is limited by processing and > flooding overhead on each node. While that number can be maximized > - by well written implementations and techniques such as exponential > + by well-written implementations and techniques such as exponential > ack > back-offs, IS-IS will still reach a saturation point where no further > routers can be added to a single flooding domain. In some L2 > backbone deployment scenarios, this limit presents a significant > @@ -148,7 +152,7 @@ > Unfortunately, in its simplest implementation, this requires the > inclusion of most, or all, of the transit L1 routers as L1/L2 to > allow traffic to flow along optimal paths through those transit > - areas. Consequently, such approach fails to reduce the number of L2 > + areas. Consequently, such an approach fails to reduce the number of L2 > ack > routers involved and with that fails to increase the scalability of > the L2 backbone as well. > > @@ -364,7 +368,7 @@ > time introducing control plane scaling benefits provided by L2 flood > reflectors. > > - The remainder of this document defines remaining extensions necessary > + The remainder of this document defines the remaining extensions > necessary > for a complete flood reflection solution: > ack > > * It defines a special 'flood reflector adjacency' built for the > @@ -412,12 +416,12 @@ > The following terms are used in this document. > > Flood Reflector: > - Node configured to connect L2 only to flood reflector clients and > + Node configured to connect L2-only clients to flood reflector > clients and > reflect (reflood) IS-IS L2 LSPs amongst them. > you changed the meaning, a FR is NOT only L2 > > Flood Reflector Client: > Node configured to build Flood Reflector Adjacencies to Flood > - Reflector, and normal adjacencies to other clients and L2 nodes > + Reflectors, and normal adjacencies to other clients and L2 nodes > not participating in flood reflection. > ack > > Flood Reflector Adjacency: > @@ -453,7 +457,7 @@ > Tunnel Endpoint: > An endpoint that allows the establishment of a bi-directional > tunnel carrying IS-IS control traffic or alternately, serves as > - the origin of such tunnel. > + the origin of such a tunnel. > ack > > L1 shortcut: > A tunnel between two clients visible in L1 only that is used as a > @@ -461,8 +465,10 @@ > mode. > > Hot-Potato Routing: > - In context of this document routing paradigm where L2->L1 routes > + In the context of this document, a routing paradigm where L2->L1 > routes > are less preferred than L2 routes [RFC2966]. > + > +jgs: Should the reference be to RFC 5302 instead? > correct, updated > > 3. Further Details > > @@ -676,7 +682,7 @@ > > further attributes necessary for tunnel establishment as defined > in [RFC9012]. The Protocol Type sub-TLV as defined in [RFC9012] > - MAY be included. In case such sub-TLV is included and F flag is > + MAY be included. In case such a sub-TLV is included and the F flag > is > ack > set (i.e. the resulting tunnel is a flood reflector adjacency) > this sub-TLV MUST include a type that allows to carry encapsulated > IS-IS frames. Furthermore, such tunnel type MUST be able to > @@ -696,6 +702,10 @@ > specified tunnel endpoints to automatically establish one or more > tunnels that will serve as L1 tunnel shortcuts to the clients > advertising the endpoints. > + > +jgs: Should that be "multiple" or "one or more"? That is, there > +doesn't *have* to be a multiplicity of them (although I imagine > +there almost always will be). > one or more is more specific. more than one may be needed for ECMP implementations e.g. > > In case of automatic flood reflection adjacency tunnels and in case > IS-IS adjacencies are being formed across L1 shortcuts all the > @@ -842,9 +852,13 @@ > Internet-Draft draft-ietf-lsr-isis-flood-reflection September 2022 > > > - A flood reflector MUST only form flood reflection adjacencies with > - flood reflector clients with matching Cluster ID. A flood reflector > + A flood reflector MUST NOT form flood reflection adjacencies with > + flood reflector clients with a different Cluster ID. A flood reflector > MUST NOT form any traditional L2 adjacencies. > + > +jgs: For consistency and clarity, I've suggested inverting the sense of > +the "MUST only" that was in the above, to follow the style you use in > +the "MUST NOT" paragraph below. > ack > > Flood reflector clients MUST NOT form flood reflection adjacencies > with flood reflectors with a different Cluster ID. > @@ -904,7 +918,7 @@ > routes are distributed into the area, normally as L2->L1 routes. Due > to the rules in Section 4.6 the computation in the resulting topology > is relatively simple, the L2 SPF from a flood reflector client is > - guaranteed to reach within a single hop the Flood Reflector and in > + guaranteed to reach the Flood Reflector within a single hop, and in > ack > the following hop the L2 egress to which it has a forwarding tunnel. > All the flood reflector tunnel nexthops in the according L2 route can > hence be removed and if the L2 route has no other ECMP L2 nexthops, > @@ -912,7 +926,7 @@ > less preferred L2->L1 route to be used to forward traffic towards the > advertising egress. > > - In the particular case the client has L2 routes which are not route > + In the particular case the client has L2 routes which are not flood > ack > reflected, those will be naturally preferred (such routes normally > "hot-potato" packets out of the L1 area). However in the case the L2 > route through the flood reflector egress is "shorter" than such > @@ -934,7 +948,7 @@ > not have any L2 flood reflector adjacencies MUST NOT redistribute L2 > routes into the cluster. > > - The L2 prefixes advertisements redistributed into L1 with flood > + The L2 prefix advertisements redistributed into an L1 that contains > flood > ack > reflectors SHOULD be normally limited to L2 intra-area routes (as > defined in [RFC7775]), if the information exists to distinguish them > from other L2 prefix advertisements. > @@ -942,7 +956,7 @@ > On the other hand, in topologies that make use of flood reflection to > hide the structure of L1 areas while still providing transit > forwarding across them using tunnels, we generally do not need to > - redistribute L1 prefixes advertisements into L2. > + redistribute L1 prefix advertisements into L2. > ack > > > > @@ -995,11 +1009,12 @@ > 8. IANA Considerations > > This document requests allocation for the following IS-IS TLVs and > - Sub-TLVs. > + Sub-TLVs, and requests creation of a new registry. > ack > > 8.1. New IS-IS TLV Codepoint > > - This document requests the following IS-IS TLV: > + This document requests the following IS-IS TLV under the > + IS-IS Top-Level TLV Codepoints registry: > > > ack > > @@ -1014,10 +1029,10 @@ > ----- --------------------------------- --- --- --- ----- > 161 Flood Reflection y n n n > > -8.2. Sub TLVs for TLV 242 > +8.2. Sub-TLVs for IS-IS Router CAPABILITY TLV > ack > > - This document request the following registration in the "sub-TLVs for > - TLV 242" registry. > + This document request the following registration in the "IS-IS Sub-TLVs > + for IS-IS Router CAPABILITY TLV" registry. > ack > > Type Description > ---- ----------- > @@ -1026,18 +1041,24 @@ > > 8.3. Sub-sub TLVs for Flood Reflection Discovery sub-TLV > > - This document request the following registration in the "sub-sub-TLVs > - for Flood Reflection Discovery sub-TLV" registry. > + This document requests creation of a new registry named > + "Sub-sub TLVs for Flood Reflection Discovery sub-TLV" under the > + "IS-IS TLV Codepoints" grouping. The Registration Procedures > + for this registry are Expert Review, following the common > + expert review guidance given for the grouping. > + > + The range of values in this registry is 0-255. The registry > + should be seeded with the following initial registration: > ack > > Type Description > ---- ----------- > 161 Flood Reflection Discovery Tunnel Encapsulation Attribute > > > -8.4. Sub TLVs for TLV 22, 23, 25, 141, 222, and 223 > +8.4. Sub-TLVs for TLVs Advertising Neighbor Information > > - This document requests the following registration in the "sub-TLVs > - for TLV 22, 23, 25, 141, 222, and 223" registry. > + This document requests the following registration in the "IS-IS > + Sub-TLVs for TLVs Advertising Neighbor Information" registry. > ack > > Type Description 22 23 25 141 222 223 > ---- -------------------------------- --- --- --- --- --- --- > @@ -1048,8 +1069,8 @@ > > This document uses tunnels to carry IS-IS control traffic. If an > attacker should be able to inject traffic into such a tunnel, the > - consequences could be in most extreme case the complete subversion of > - the IS-IS level 2 information. Therefore, at tunnel level steps > + consequences could be in the most extreme case, the complete > subversion of > + the IS-IS level 2 information. Therefore, at the tunnel level steps > should be taken to prevent such injection. Since the available > security procedures will vary by deployment and tunnel type, the > details of securing tunnels are beyond the scope of this document. > @@ -1067,12 +1088,12 @@ > ack > > > attacks or could even completely subvert the IS-IS level 2 routing. > - Therefore, steps should be taken to prevent such capture at tunnel > + Therefore, steps should be taken to prevent such capture at the tunnel > level. Since the available security procedures will vary by > deployment and tunnel type, the details of securing tunnels are > beyond the scope of this document. > ack > > - Additionally, usual IS-IS security mechanisms [RFC5304] SHOULD be > + Additionally, the usual IS-IS security mechanisms [RFC5304] SHOULD be > deployed to prevent misrepresentation of routing information by an > attacker in case a tunnel is compromised if the tunnel itself does > not provide mechanisms strong enough guaranteeing the integrity of > ack > > _______________________________________________ > Lsr mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/lsr >
_______________________________________________ Lsr mailing list [email protected] https://www.ietf.org/mailman/listinfo/lsr
