[Lsr] Benjamin Kaduk's No Objection on draft-ietf-isis-reverse-metric-16: (with COMMENT)
Benjamin Kaduk <email@example.com> Fri, 16 November 2018 22:08 UTC
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id CDAA6127333; Fri, 16 Nov 2018 14:08:10 -0800 (PST)
Content-Type: text/plain; charset="utf-8"
From: Benjamin Kaduk <firstname.lastname@example.org>
To: "The IESG" <email@example.com>
Cc: firstname.lastname@example.org, Christian Hopps <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com
Date: Fri, 16 Nov 2018 14:08:10 -0800
Subject: [Lsr] Benjamin Kaduk's No Objection on draft-ietf-isis-reverse-metric-16: (with COMMENT)
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:firstname.lastname@example.org?subject=unsubscribe>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:email@example.com?subject=subscribe>
X-List-Received-Date: Fri, 16 Nov 2018 22:08:11 -0000
Benjamin Kaduk has entered the following ballot position for draft-ietf-isis-reverse-metric-16: No Objection When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-isis-reverse-metric/ ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- Section 1 Perhaps expand IIH on first usage? Section 1.2 (We could perhaps use "virtual network" instead of "VPN", to avoid ambiguity about whether its traffic is encrypted. This is quite tangential to the actual document, so feel free to ignore.) Section 1.5 Using an offset implies a need to do bounds checking and/or clamping. Section 2 discusses this to some extent, though it may be worth an additional mention here or in the security considerations. Section 2 The Reverse Metric TLV is a new TLV to be used inside IS-IS Hello PDU. This TLV is used to support the IS-IS Reverse Metric mechanism that allows a "reverse metric" to be send to the IS-IS neighbor. nits: "inside an IS-IS Hello PDU"; "sent" Do we need to say "network byte order" for the Metric field? The W bit seems to allow one node to modify the metric for traffic to adjacent nodes, which is a slightly broader permission than what is described in the security considerations and could merit mention therein. (The existing text there on authentication should suffice even for this permission, though.) U bit (0x02): The "Unreachable" bit specifies that the metric calculated by addition of the reverse metric value to the "default metric" is limited to (2^24-1). Does this mean that 2^24-1 is the maximum value (as opposed to the maximum of 2^24-2 when the bit is not set) or that it is the only allowed value? [...] This sub-TLV is optional, if it appears more than once then the entire Reverse Metric TLV MUST be ignored. nit: comma splice Section 3.2 When an M-ISIS router receives a Reverse Metric TLV, it MUST add the received Metric value to its Default Metric in all Extended IS Reachability TLVs for all topologies. [...] (I assume this is still scoped to the link in question. I don't know whether there's any need to add more text to clarify that, though.) Section 3.3 If a DIS is configured to apply Traffic Engineering over a link and it receives metric sub-TLV in a Reverse Metric TLV, it should update the Traffic Engineering Default Metric sub-TLV value of the corresponding Extended IS Reachability TLV or insert a new one if not present. Is "metric sub-TLV" short for "Traffic Engineering Default Metric sub-TLV"? In the case of multi-access LANs, the "W" Flags bit is used to signal from a non-DIS to the DIS whether to change the metric and, optionally Traffic Engineering parameters for all nodes in the Pseudonode LSP or solely the node on the LAN originating the Reverse Metric TLV. nit: comma after "optionally" When the DIS receives the Reverse Metric TLV with the 'W' bit set, does that mean it ignores any such TLVs it receives without the 'W' bit set, or would both the global and router-specific offsets be applied? Section 3.5 Please expand CSPF on first use. (Also, nit-level, I am inferring that "a CSPF recalculation" would be more grammatical, but am not entirely sure.) Also, SHOULD and RECOMMENDED have the same strength, so from an editorial perspective the current text could be consolidated. But perhaps the intent was to have the 2^24-1 case be a stronger requirement, in which case MUST could be used? It is RECOMMENDED that implementations provide a capability to disable any IS-IS metric changes by Reverse Metric mechanism through neighbor's Hello PDUs. It can be to a node's individual interface Default Metric or Traffic Engineering parameters based upon receiving a properly formatted Reverse Metric TLVs. I'm failing to parse this last sentence. The first sentence seems to be saying that implementations should provide a knob to ignore received Reverse Metric TLVs, so maybe the second sentence is trying to say what the behavior would be in the case that the received Reverse Metric TLV is ignored? I'm not sure. Section 4 The enhancement in this document makes it possible for one IS-IS router to manipulate the IS-IS Default Metric and, optionally, Traffic Engineering parameters of adjacent IS-IS neighbors. [...] Just to check my understanding: this manipulation is for parameters either for links directed at the IS-IS router sending Reverse Metric, or for links directed to multicast neighbors on a multi-access LAN (or both)? Section 5 It's a little surprising to me to see a new registry created with a single value allocated, the value of which matches the sub-TLV of the same name in the "Sub-TLVs for TLVs 22, 23, 25, 141, 222, and 223 (Extended IS reachability, IS Neighbor Attribute, L2 Bundle Member Attributes, inter-AS reachability information, MT-ISN, and MT IS Neighbor Attribute TLVs)" registry, but I do not object to doing it this way. Appendix B The risks of misidentifying one side of a point-to-point link or one or more interfaces attached to a multi-access LAN and subsequently increasing its IS-IS metric and potentially increased latency, jitter or packet loss. [...] nit: I'm not sure this is a complete sentence. I think "The risks of..." implies there will be another clause following, and also that "subsequently increasing" and "potentially increased" have mismatched verb tense, but I am not entirely certain I am working towards the intended meaning.
- [Lsr] Benjamin Kaduk's No Objection on draft-ie... Benjamin Kaduk