Adrian - Thanx for your prompt and detailed review of the latest version. Please see inline. > -----Original Message----- > From: Adrian Farrel via Datatracker <noreply@xxxxxxxx> > Sent: Thursday, February 13, 2025 10:16 AM > To: rtg-dir@xxxxxxxx > Cc: draft-ietf-lsr-multi-tlv.all@xxxxxxxx; last-call@xxxxxxxx; lsr@xxxxxxxx > Subject: Rtgdir last call review of draft-ietf-lsr-multi-tlv-08 > > 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? > [LES:] The intent is to allow a document defining some new codepoint to choose a different mechanism for supporting more than 255 bytes. We aren't encouraging this - quite to the contrary - but we don’t want to preclude an alternative that might be appropriate for some use case that we currently do not foresee. I will update the text to clarify this. BTW, I prefer to leave the abstract as is. > --- > > 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: [LES:] Well, when one starts w V0, it is hard to know how long it will take before the document gets published and what other protocol extensions may be standardized in the meantime. But I get your point. How about: "Some examples of this are..." ?? > > --- > > 3.1 > > The RFC editor will ask you to expand BFD, MTID, and NLPID on first use. > [LES:] Ack > --- > > 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 [LES:] The TLV is not in the earlier list because RFC 6213 is silent on whether multiple TLVs can be sent. This section is providing an example of a codepoint where there is no key, yet more than 255 bytes of information may need to be advertised. Given that RFC 6213 is silent on this point, the text "would be required" seems very accurate to me. Whereas "can be present" suggests that this has already been stated in an existing RFC. The actual standardization of applicability of MP-TLV to this codepoint is formally present in Section 9. So, I think the existing text should remain as is. > > --- > > 4. > > The RFC editor will ask you to expand IIH on first use. > [LES:] Ack > --- > > 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 > [LES:] ISO 10589 is not relevant here. None of the TLVs defined in ISO 10589 are candidates for MP. The potential for MP really came with the introduction of sub-TLVs (RFC 5305). So, I prefer to leave text unchanged. > --- > > 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. > [LES:] I am not sure what textual change you are suggesting. The current text seems to me to be appropriate and clear. More detailed receive processing guidelines are discussed in Section 5. This section is just providing a high level contrast between MP disabled/MP enabled processing. > --- > > 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? > [LES:] There is no intention to suggest any of what you state. The intent is to say that although MP-TLVs should only be sent when more than 255 bytes is required, the receiver should not reject MP -TLVs if they happen to have (for example) 100 bytes each. I will revise the text to make this more clear. > --- > > 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. [LES:] This is explicitly stated in Section 5: "The receiving node must then process this as having key information K and sub-TLVs A, B, C, D, E, F, or, because ordering is irrelevant, sub-TLVs D, E, F, A, B, C, or any other permutation." > 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. > [LES:] Section 5 also says: "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." > 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. [LES:] As I have pointed out, Section 5 already has explicit text on these points. 8.2 is talking about "generation". Section 5 and your comments on it are about receive processing. I do not want to confuse the two. I suspect what you are after is that the description of receive processing implies some requirements on how the originator of the TLV encodes it. This is "loosely true" i.e., in a perfect world all MP-TLVs would be generated in strict accordance with rules (mentioned in 8.2). But in the spirit of "be strict in what you send/generous in what you receive" we want receivers to be more forgiving. Sending MP-TLVs when they are not required (less than 255 bytes of info) is sub-optimal and undesirable, but we intentionally stop short of declaring this "illegal". This point was discussed at some length on the list and we (the authors) were/are firm in our commitment to this approach. At this point I am not sure how to address your concerns or if they even need to be addressed. I'll think on this some more, but if after reading my response you have a suggestion, please share. > > --- > > 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." ? > [LES:] I'll revise this text. What we are talking about is duplicated information. > --- > > 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. > [LES:] There are existing cases where MP-TLV support was defined but a different behavior was specified for handling duplicated information. For example: https://datatracker.ietf.org/doc/rfc7981/ Section 3 (last paragraph) states: " Where a receiving system has two copies of an IS-IS Router CAPABILITY TLV from the same system that have conflicting information for a given sub-TLV, the procedure used to choose which copy shall be used is undefined." We are choosing not to revise or override explicit guidance for a given codepoint. > --- > > You have "Multi-part TLV" and "Multi-Part TLV". Pick one. > But you also seem to interchange "MP-TLV" and "Multi-part TLV". > [LES:] ack > --- > > 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 [LES:] OK > -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
