Invalid TLV Handling in IS-IS
draft-ietf-lsr-isis-invalid-tlv-03
Yes
(Alvaro Retana)
No Objection
(Barry Leiba)
(Deborah Brungard)
(Martin Vigoureux)
Note: This ballot was opened for revision 02 and is now closed.
Erik Kline
No Objection
Comment
(2020-07-10 for -02)
Not sent
[ section 2 ] * s/PDUS/PDUs/
Murray Kucherawy
No Objection
Comment
(2020-07-13 for -02)
Sent
In the Abstract: * "... in order to insure that interoperability is maximized." -- That should be "ensure".
Roman Danyliw
No Objection
Comment
(2020-07-13 for -02)
Sent
I'm glad to see language clarifying error handling. Thanks for the work on it. Section 3.2. Per “It is RECOMMENDED that implementations provide controls for the enablement of behaviors that are not backward compatible”, I want to double check that I’m understanding this sentence correctly. RFC5304 provides normative guidance that isn’t backward compatible with ISO10589. RFC6233 provide guidance that isn’t backward compatible with either RFC5304 or ISO10589. Is the initial sentence effectively saying that implementations should support deployments in configurations that are not backward compatible (i.e., those using the newer TLVs)? As these changes are covering security matters, I read “controls” in the cyber mitigation sense -- they prevent an action, not enable one.
Éric Vyncke
No Objection
Comment
(2020-07-13 for -02)
Not sent
Minor nits in the abstract: Suggest using ';' and more commas in "Although there are explicit statements in existing specifications, deployment experience has shown that there are inconsistencies in the behavior when a TLV which is disallowed in a particular Protocol Data Unit (PDU) is received."
Alvaro Retana Former IESG member
Yes
Yes
(for -02)
Unknown
Benjamin Kaduk Former IESG member
Yes
Yes
(2020-07-13 for -02)
Sent
The shepherd writeup is a bit unclear as to why Proposed Standard is the right document status (vs., e.g., Informational). I guess it's desired to have the Updates: relationship to the indicated documents, which essentially forces it to be standards-track. On the other hand, perhaps the sense that ignoring a TLV that is specifically disallowed (as opposed to "merely" unrecognized) is itself a newly specified requirement, in which case the status as Proposed Standard makes sense for that reason. It might be worth a little more clarity on which (if either) of these scenarios are intended. Section 1 A corollary to ignoring unknown TLVs is having the validation of PDUs be independent from the validation of the TLVs contained in the PDU. nit: this doesn't exactly seem like a "corollary" specifically, but rather a choice that [ISO10589] made (and which keeps some overall sense of consistency between PDU and TLV handling). Section 3.1 [ISO10589] defines the behavior required when a PDU is received containing a TLV which is "not recognised". It states (see Sections 9.3 - 9.13): This is Sections 9.5 (not 9.3) to 9.13 in the copy I have. Section 3.2 Similarly, the extensions defined by [RFC6233] are not compatible with the behavior defined in [RFC5304], therefore can only be safely enabled when all nodes support the extensions. nit: I'd argue that technically the extensions are *defined* by 6232, even though 6233 is what makes their nature (as "allowed in purge") easily discoverable. It is RECOMMENDED that implementations provide controls for the enablement of behaviors that are not backward compatible. We also specifically want the ability to generate but not process/require at least some of them, right? Is that worth calling out in addition to just "controls for the enablement"? Section 3.3 a given sub-TLV is allowed. Section 2 of [RFC5305] is updated by the following sentence: "As with TLVs, it is required that sub-TLVs which are disallowed MUST be ignored on receipt.". Do we want to say where this logical insertion occurs? Section 3.4 The correct setting for "LSP" is "n". This document updates [RFC6232] by correcting that error. It's slightly interesting that there doesn't seem to be a corresponding Errata Report (but may not be worth doing anything about given that this update is about to be approved). Section 8.1 It's not entirely clear that RFC 7356 is referenced in a normative manner.
Barry Leiba Former IESG member
No Objection
No Objection
(for -02)
Not sent
Deborah Brungard Former IESG member
No Objection
No Objection
(for -02)
Not sent
Martin Duke Former IESG member
No Objection
No Objection
(2020-07-11 for -02)
Sent
It might be helpful to define “ignore” as “skip the number of octets indicated by the length field.” An alternate interpretation might skip the number of bytes implied by the type code, if the type is known. Similarly, I take it that a length value beyond the end of the message ends processing of the PDU, but the PDU as a whole MUST NOT be discarded.
Martin Vigoureux Former IESG member
No Objection
No Objection
(for -02)
Not sent
Robert Wilton Former IESG member
No Objection
No Objection
(2020-07-14 for -02)
Sent
The document is short and easy to read, thanks! However, I was sure whether I should put a DISCUSS on this document for section 3.4. I find that the quoted paragraph from RFC6232 to be badly worded: "The POI TLV SHOULD be found in all purges and MUST NOT be found in LSPs with a non-zero Remaining Lifetime." Taking a strict reading of this, my interpretation is that an implementation is not compliant to RFC 6232 if it happens to receive a POI TLV in an LSP with non-zero remaining lifetime! Further, this text then arguably conflicts with earlier parts of this document regarding how unrecognized or bad TLVs should be handled. Hence, given that RFC6232 is being updated, I would prefer it if this document also updated RFC6232 to clarify the above paragraph to something like: "The POI TLV SHOULD be sent in all purges and MUST NOT be sent in LSPs with a non-zero Remaining Lifetime." One other minor comment: It is RECOMMENDED that implementations provide controls for the enablement of behaviors that are not backward compatible. Is this covered by the existing ISIS YANG model, or included in the latest update to that YANG model? Regards, Rob