Hi John, Thanks for your detailed review. We've incorporated the editorial changes suggested by you. Please check inline below for responses.
An update with these changes has also been posted: https://datatracker.ietf.org/doc/html/draft-ietf-lsr-ospf-reverse-metric-06 On Thu, Sep 1, 2022 at 2:28 AM John Scudder <[email protected]> wrote: > Dear Authors, > > Thanks for your patience. Here’s my review. > > I’ve supplied my comments in the form of an edited copy of the draft. > Minor editorial suggestions I’ve made in place without further comment, > more substantive comments are done in-line and prefixed with “jgs:”. You > can use your favorite diff tool to review them; I’ve attached a PDF of 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. > > Thanks, > > —John > > > --- draft-ietf-lsr-ospf-reverse-metric-05.txt 2022-08-30 > 14:25:14.000000000 -0400 > +++ draft-ietf-lsr-ospf-reverse-metric-05-jgs-comments.txt 2022-08-31 > 16:54:21.000000000 -0400 > @@ -1,4 +1,18 @@ > - > +jgs: Nit, the doc is inconsistent on what case to use for "hello" > +(sometimes spelled Hello, sometimes hello) and also mixes "Hello > +packets" and "hello messages". Can you pick one and stick with it? (RFC > +2328 calls them "Hello packets".) > KT> Ack. Changed to "Hello packet" for consistency. > + > +jgs: This document makes fairly liberal use of SHOULD. Please review > +these. For each one, please consider whether it can be a MUST instead, > +or the plain English word "should", or some other construction (e.g. a > +statement like "A router receiving a Hello packet from its neighbor that > +contains the Reverse Metric TLV on a link uses the RM value to derive > +the metric for the link..."). If the RFC 2119 SHOULD is really the > +appropriate tool, please add some context about when it would be > +appropriate for the implementor to make a different choice than what is > +specified, and perhaps what that choice might be. (Same point applies > +to your use of RECOMMENDED.) > > KT> Updated the text to clarify in a few instances. I believe others should be clear; if not, we can discuss specifics. > > > @@ -22,7 +36,7 @@ > signaling of this reverse metric, to be used on the link to the > signaling router, allows a router to influence the amount of traffic > flowing towards itself and in certain use cases enables routers to > - maintain symmetric metric on both sides of a link between them. > + maintain symmetric metrics on both sides of a link between them. > > Status of This Memo > > @@ -88,12 +102,12 @@ > > 1. Introduction > > - Routers running the Open Shortest Path First (OSPFv2) [RFC2328] and > - OSPFv3 [RFC5340] routing protocols originate a Router-LSA (Link State > + A router running the Open Shortest Path First (OSPFv2) [RFC2328] and > + OSPFv3 [RFC5340] routing protocols originates a Router-LSA (Link-State > Advertisement) that describes all its links to its neighbors and > - includes a metric that indicates its "cost" of reaching the neighbor > + includes a metric that indicates its "cost" to reach the neighbor > over that link. Consider two routers R1 and R2 that are connected > - via a link. The metric for this link in direction R1->R2 is > + via a link. The metric for this link in the direction R1->R2 is > configured on R1 and in the direction R2->R1 is configured on R2. > Thus the configuration on R1 influences the traffic that it forwards > towards R2 but does not influence the traffic that it may receive > @@ -114,9 +128,14 @@ > Internet-Draft OSPF Reverse Metric April 2022 > > > - results in the desired change in the traffic distribution that R1 > + may result in the desired change in the traffic distribution that R1 > wanted to achieve towards itself over the link from R2. > > +jgs: I changed "results" to "may result" because, of course, you might > +get the outcome you want by twiddling the metric and you might not. > +About the only case where you can guarantee you get the outcome you > +hoped for is when you're drying out a link by setting max-metric. No? > KT> Ack > + > This document describes extensions to OSPF Link-Local Signaling (LLS) > [RFC5613] to signal OSPF reverse metrics. Section 4 specifies the > LLS Reverse Metric TLV and Section 5 specifies the LLS Reverse TE > @@ -132,9 +151,9 @@ > > 2. Use Cases > > - This section describes certain use cases that OSPF reverse metric > + This section describes certain use cases that the OSPF reverse metric > helps address. The usage of the OSPF reverse metric need not be > - limited to these cases and is intended to be a generic mechanism. > + limited to these cases; it is intended to be a generic mechanism. > > Core Network > ^ ^ > @@ -156,8 +175,8 @@ > > Figure 1: Reference Dual Hub and Spoke Topology > > - Consider a deployment scenario where, as shown in Figure 1, a bunch > - of routers R1 through RN, are dual-home connected to AGGR1 and AGGR2 > + Consider a deployment scenario where, as shown in Figure 1, > + routers R1 through RN are dual-home connected to AGGR1 and AGGR2 > that are aggregating their traffic towards a core network. > > > @@ -182,6 +201,15 @@ > successfully, this allows the operator to confidently perform > disruptive augmentation, fault diagnosis, or repairs on a link > without disturbing ongoing communications in the network. > + > +jgs: I'm surprised by the claim that a metric change will "gradually" > shift > +traffic and that it will have "little or no disruption". I guess it all > +depends on how you define those terms, and on the network -- I can pretty > +easily draw you a network where the shift will be neither gradual, nor > +non-disruptive. What it will be, is quick, and any disruption will be > +brief. Anyway, ISTM that this claim is red meat for people (like me) to > +disagree with and is not important to the use case, so it could just be > +edited out or cut way down to something drier but easier to support? > KT> Made it "drier". Please check if the changes work. > > In deployments such as a hub and spoke topology as shown in Figure 1, > it is quite common to have routers with several hundred interfaces > @@ -194,17 +222,21 @@ > when a large number of CE routers connect to a PE router, an > additional challenge related to coordinating access to the CE routers > may arise when the CEs are not managed by the provider. > + > +jgs: The world is a big and wonderful place so there may be a provider > that > +allows routers they don't manage to participate in their IGP, but it seems > +like a pretty weird case. > KT> These are cases where OSPF is run as a PE-CE protocol. > > The OSPF reverse metric mechanism helps address these challenges. > The operator can set the link on one of the routers (generally the > - hub like AGGR1 or a PE) in a "maintenance mode". This causes the > + hub like AGGR1 or a PE) to a "maintenance mode". This causes the > router to advertise the maximum metric for that link and also to > signal its neighbor on the same link to advertise maximum metric via > the reverse metric signaling mechanism. Once the link maintenance is > completed and the "maintenance mode" is turned off, the router > returns to using its provisioned metric for the link and also stops > the signaling of reverse metric on that link resulting in its > - neighbor to also revert to its provisioned metric for that link. > + neighbor also reverting to its provisioned metric for that link. > > 2.2. Adaptive Metric Signaling > > @@ -233,7 +265,7 @@ > adaptive metric changes to be applied on the AGGR1 as opposed to > being provisioned on a possibly large number of R1-RN routers. > > - The reverse metric mechanism may also be similarly applied between > + The reverse metric mechanism may be similarly applied between > spine and leaf nodes in a CLOS topology deployment. > > 3. Solution > @@ -282,7 +314,7 @@ > Internet-Draft OSPF Reverse Metric April 2022 > > > - Length: 4 octet > + Length: 4 octets > > MTID : the multi-topology identifier of the link ([RFC4915]) > > @@ -370,13 +402,17 @@ > When the O flag is set, the metric value to be advertised is derived > by adding the value in the TLV to the provisioned metric for the > link. When the O flag is clear, the metric value to be advertised is > - derived directly from the value in the TLV. When the H flag is set > - and the O flag is clear, the metric value to be advertised is derived > + > +jgs: shouldn't the above say something about being careful about when > +provisioned + advertised > max-metric? > KT> Ack > + > + copied directly from the value in the TLV. When the H flag is set > + and the O flag is clear, the metric value to be advertised is copied > directly from the value in the TLV only when the RM value signaled is > higher than the provisioned metric for the link. > > A router stops including the Reverse Metric TLV in its hello messages > - when it needs its neighbors to go back to using their own provisioned > + when it needs its neighbor to go back to using its own provisioned > metric values. When this happens, a router that had modified its > metric in response to receiving a Reverse Metric TLV from its > neighbor should revert to using its provisioned metric value. > @@ -422,15 +458,17 @@ > When using multi-topology routing with OSPF [RFC4915], a router MAY > include multiple instances of the Reverse Metric TLV in the LLS block > of its hello message - one for each of the topologies for which it > - desires to signal the reserve metric. > + desires to signal the reverse metric. > > In certain scenarios, the OSPF router may also require the > modification of the TE metric being advertised by its neighbor router > towards itself in the inbound direction. The Reverse TE Metric TLV, > - using similar procedures as described above, MAY be used to signal > + using similar procedures to those described above, MAY be used to > signal > the reverse TE metric for router links. The neighbor SHOULD use the > reverse TE metric value to derive the TE metric advertised in the TE > - Metric sub-TLV of the Link TLV in its TE Opaque LSA [RFC3630]. > + Metric sub-TLV of the Link TLV in its TE Opaque LSA [RFC3630]. The > + rules for doing so are analogous to those given above for the > + Router-LSA. > > 7. Operational Guidelines > > @@ -441,6 +479,11 @@ > message, this will assist in rapidly identifying the node in the > network that is advertising an OSPF metric or TE metric different > from that which is configured locally on the device. > + > +jgs: It seems to me that "send a syslog message" is being too specific -- > +don't you actually mean something more general like "log the event" or > +"notify system administration"? Surely it doesn't matter if > +Syslog [RFC5424] is the way it's done? > KT> Ack > > > > @@ -463,10 +506,28 @@ > If an implementation enables this mechanism by default, it is > RECOMMENDED that it be disabled by the operators when not explicitly > using it. > + > +jgs: In the above, it feels like what you really want to say is that an > +implementation SHOULD disable this mechanism by default. Why not just > +say that? As written, you've put extra work onto the operator, and to > +what end? > KT> Ack. Have updated the text. > > For the use case in Section 2.1, a router SHOULD limit the period of > advertising reverse metric towards a neighbor only for the duration > of a network maintenance window. > + > +jgs: This reads as though you're addressing the routing software > +implementor ("a router SHOULD"). If so it seems as though you're > +implicitly defining what would normally be an implementation detail -- > +"give me a feature that allows me to specify a temporary metric on a > +link, with associated begin/end times". If you really want to require > +that, I think it needs more specification than what's here, because it > +would be relatively easy to overlook. On the other hand if this is > +really advice to the network operator... it seems kind of obvious, as > +though it doesn't need to be said. > + > +My own feeling is that this paragraph could be removed, but we can > +discuss. > KT> It was meant for the operator and we have rephrased the text. > > 8. Backward Compatibility > > @@ -513,6 +574,10 @@ > > Receiving a malformed LLS Reverse Metric or Reverse TE Metric TLVs > MUST NOT result in a hard router or OSPF process failure. The > + > +jgs: Is there some way for me to read that MUST NOT as anything other > +than "you must not have bugs in your software"? > + > KT> No other way ;-) and I am borrowing this text from previous RFCs. Thanks, Ketan > reception of malformed LLS TLVs or sub-TLVs SHOULD be logged, but > such logging MUST be rate-limited to prevent denial-of-service (DoS) > attacks. > >
_______________________________________________ Lsr mailing list [email protected] https://www.ietf.org/mailman/listinfo/lsr
