Reviewer: Adrian Farrel Review result: Has Nits Reviewer: Adrian Farrel Document: draft-ietf-lsr-multi-tlv-08 Review result: Ready Hi, The Responsible AD has requested a further Routing Area Directorate review during IETF Last Call. I am the designated reviewer. This document addresses the challenge of IS-IS TLVs exceeding the 255 octet limit. It describes an existing deployed mechanism for using multiple TLVs to carry the content of the over-large TLVs. The draft is well written and easy to understand from the perspective of someone more familiar with IS-IS. There are fewer than 10 pages of substance in this draft, the remainder being tables of codepoints, references, and boilerplate. There are, of course, some nits, but I believe the draft is ready to progress. Resolving the nits is not essential but would, I think, improve the utility of the document, while nothing technical changes. Adrian === = Nits = It may be helpful (although obvious?) to add to the Abstract and Introduction *why* this document codifies a common mechanism. Clearly, a number of existing implementations already do this and don't need any further guidance. The answer (of course) is to allow future implementations to interoperate and to make it clear what is happening when these multiple TLVs are seen in the field. --- Introduction Some TLV definitions have addressed this by explicitly stating that a TLV may appear multiple times inside of an LSP. A reference or two would be nice (but not essential as you explicitly list them later). --- Introduction The intent of this document is to clarify and codify the situation by explicitly making multiple occurences of a TLV the mechanism for scaling TLV contents, except where otherwise explicitly stated. The last clause is ambiguous. Otherwise stated in this document, or in other documents? --- Introduction As of this writing, the authors are aware of the following TLVs that fall into this category: This text doesn't age well. I would suggest cutting it to: The following TLVs fall into this category: --- 3.1 The RFC editor will ask you to expand BFD, MTID, and NLPID on first use. --- 3.1 It is not clear from reading this document whether you are describing the previously defined behaviour for type 148. RFC 6213 is not in the list in section 1, so we might assume that you are describing what this document delivers. But the text is very definitive as though the mechanism already exists. Perhaps: OLD If more than 255 octets is required to advertise all of the MTID/NLPID pairs, multiple BFD-Enabled TLVs would be required. NEW` If more than 255 octets is required to advertise all of the MTID/NLPID pairs, they cannot be carried in a single TLV, but using the mechanism described in this document, multiple BFD-Enabled TLVs can be present. END --- 4. The RFC editor will ask you to expand IIH on first use. --- 4. Pedantically... OLD In the absence of MP-TLV support, when a router receives an MP-TLV, it treats the TLVs as replacements for each other i.e., the receiver chooses which TLV will be processed and which TLV will be ignored. NEW In the absence of MP-TLV support, as specified in [ISO10589], when a router receives an MP-TLV, it treats one of the TLVs as a replacement for the others, i.e., the receiver chooses which TLV will be processed and which TLVs will be ignored. END --- 4. In the presence of MP-TLV support, when a router receives an MP-TLV, it treats the TLVs as complementary i.e., information from all the TLVs is processed. But the previous paragraph says that there is a legitimate case where either a sender makes an error by including multiple versions of a TLV or in the case of transmitting a TLV from one LSP to another. These legitimate cases now run into the MP-TLV handling and must be resolved. I think the duplication is covered by the concatenation processing rules. That is, after concatenation of accidental/erroneous duplicate TLVs it will be discovered that the TLV contains duplicate sub-TLVs and so it will be declared malformed and discarded. As with other points in this review, it is sort-of OK that everything can be worked out like this, but it would be much nicer if the spec was explicit. --- 5. When processing MP-TLVs, implementations MUST NOT impose a minimum length check. The next text gives context to this (i.e., do not require that all bar one TLV in an MP-TLV set have length 255), this text is ambiguous because it appears to say that a TLV with length 0 should be allowed. Is it? Further, surely the length must be large enough that the key (if applicable) is present. Perhaps, given the subsequent text, this sentence is not needed? --- 5. The contents of a multi-part TLV MUST be processed as if they were concatenated. This is probably the most important sentence in the document! Combined with: ...and... If the internals of the TLV contain key information, then replication of the key information MUST be taken to indicate that subsequent data MUST be processed as if the subsequent data were concatenated after a single copy of the key information. ...we can deduce a lot. 1. Information place in component TLVs must be present such that the concatenation may be done in any order. That is, you can't just fill up the 255 bytes and then start a new TLV. 2. If a TLV has a fixed part (which may contain a key) it must be present in each component TLV such that the component TLV is correctly composed in its own right. I looked for this guidance in 8.2, but did not find it. While the text is acceptable as it stands (after all, this can be worked out) it would not hurt to make it clear by calling out these points explicitly either in 5 or 8.2. --- 5. It is also possible that information which is not part of the fixed part of a TLV can be duplicated e.g., the same sub-TLV could appear multiple times whether within the same TLV or in different parts of an MP-TLV. This is also an error. Surely append "unless the TLV's base definition allows multiple sub-TLVs of the same type to be present." ? --- 5. Specifying how to handle such cases is the responsibility of the document which defines the TLV. If such a document is not explicit in how to handle such cases, it is RECOMMENDED that the first occurrence in the lowest numbered LSP be used. In the case of IIHs, it is RECOMMENDED that the first occurrence in the IIH be used. This seems broken (although the intent is clear). Since MP-TLV is intended to be applicable to TLVs that are already defined, it seems impossible that the documents that defined them can describe how to handle this situation. They should, of course, already cover how to handle duplicate TLVs and duplicate sub-TLVs within a single TLV. --- You have "Multi-part TLV" and "Multi-Part TLV". Pick one. But you also seem to interchange "MP-TLV" and "Multi-part TLV". --- 7. Nodes which support MP-TLV for codepoints for which existing specifications do not explicitly define such support, but for which MP-TLV is applicable, SHOULD include this sub-TLV in a Router Capability TLV. Not clear why this is not a MUST. I don't think it needs to be, but an implementation might need help to understand why it MAY do otherwise. But later you say "it is only for information" which is a good enough reason to leave it out. And so you could... OLD As an aid to network operators, a new sub-TLV of the IS-IS Router CAPABILITY TLV [RFC7981] is defined: NEW As an aid to network operators when diagnosing such situations, a new sub-TLV of the IS-IS Router CAPABILITY TLV [RFC7981] is defined: END -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
