Hi, Adrian:
I admire your efforts to try to improve the readiness of this document. But, SURPRINGLY, you get the wrong conclusion-----“in particular clarifying what is in the field and ensuring that others can interoperate safely.” We are discussing where is the “field(what constitute a key)” to “ensuing that others can interoperate safely”. Would you like to follow/chime in the discussions on another thread (https://mailarchive.ietf.org/arch/msg/lsr/eDZWKm-bcG1t7WKYYmwqMvlNAoY/), and to review again and make a new judgment/conclusion of this document? Best Regards Aijun Wang China Telecom 发件人: [email protected] [mailto:[email protected]] 代表 Adrian Farrel 发送时间: 2024年11月13日 5:22 收件人: [email protected] 抄送: [email protected] 主题: [Lsr] Debate on draft-ietf-lsr-multi-tlv Hi, Well, this whole discussion sent me off to read the draft. I have some suggestions to clarify the text that might help with the current discussion, and also some nits that were thrown up by reading the draft. I don't know where this fits into the last call sequence, but I hope my comments are useful to improve the document �C feel free to tell me I have misunderstood. I should add, I support the publication of this document for many of the reasons stated: in particular clarifying what is in the field and ensuring that others can interoperate safely. Cheers, Adrian = Clarification points = Section 3 has: A TLV is a tuple of (Type, Length, Value) and can be advertised in IS-IS packets. TLVs sometimes contain information, called a key, that indicates the applicability of the remaining contents of the TLV. If a router advertises multiple TLV tuples with the same Type code in an IS-IS IIH packet or in the set of LSPs for a level with the same key value, they are considered a multi-part TLV (MP-TLV). This seems to be the crunch paragraph that could set the scene for the rest of the document, and could help new readers understand the scope. This is clear except for: - "key" is a term which I understand, and which I would also have used, but I wondered whether anything is actually called a key in the current IS-IS literature. So I did a search and didn’t find anything �C it’s possible my search wasn’t broad enough. But, if you have a citation for the assertion, then please include it and some of the angst will go away. OTOH, if this is a term being defined here, we might strengthen the definition and scope. - "sometimes contains" means that sometimes a key is not included in which case, presumably, the mechanisms in this document can still be applied. So maybe… A TLV is a tuple of (Type, Length, Value) and can be advertised in IS-IS packets. The applicability of the TLV is indicated by the Type field, and for the case of some specific Type codes, may be further qualified by specific information in the Value that, in this document, are collectively called the key. If a router advertises multiple TLV tuples in an IS-IS IIH packet or in the set of LSPs for a level, and they have the same Type code, they are considered a multi-part TLV (MP-TLV) if any key defined for the specific TLV Type also matches in the TLVs --- In Section 4 Network operators should not enable Multi-part TLVs until ensuring that all implementations that will receive the Multi-part TLVs are capable of interpreting them correctly. - Why is that not “SHOULD NOT”? - But also, why is that not “MUST NOT”? --- The example in 4.1 is ambiguous. You have previously stated (section 4) that the key information MUST be replicated in additional TLV instances This example defines: The key consists of the 7 octets of system ID and pseudonode number plus the set of link identifiers which are present. But then: The following key information MUST be replicated in each of the additional Extended IS Reachability TLVs: * 7 octets of system ID and pseudonode number * The set of link identifiers SHOULD be identical in all TLVs which are part of the MP-TLV set. How is that “SHOULD” consistent with the two cases of “MUST”? I should have thought this should read… The following key information MUST be replicated in each of the additional Extended IS Reachability TLVs: * 7 octets of system ID and pseudonode number * The set of link identifiers Except, this is an example, not normative text so the whole should read… The following key information is replicated in each of the additional Extended IS Reachability TLVs: * 7 octets of system ID and pseudonode number * The set of link identifiers . --- Similarly in 4.2, the use of BCP14 language is not appropriate. So… OLD The key information MUST be replicated identically. NEW The key information is replicated identically. END --- Somethings we learn in section 5 about how a sender constructs MP-TLVs are: - The series of TLVs with the same T and K values may be sent in any order - The contents of a Value that is being split across MP-TLVs cannot be arbitrarily split. Any split must be at a sub-TLV boundary (and no lower, i.e., not at a sub-sub-TLV boundary, and certainly not just “some bytes in one MP-TLV and some bytes in another). This is because the receiver “concatenates” the contents of the received Values in any order and it wouldn’t be clear to which sub-TLV the sub-sub-TLV belonged (and, of course, you could just garble the order of bytes). These points really need to be brought out in section 4 because they instruct the sender. --- Section 5 contains: If the internals of the TLV do NOT include key information, the relevant key can be found in the parent TLV. I am struggling to parse this. I think it is probably simply not adding anything (except confusion). Suppose I have: TLV{key, info, sub-TLV1, sub-TLV2} Here, TLV is the parent and sub-TLV1 and sub-TLV2 are the children But it is too long. So I send: MP-TLV1{key, info, sub-TLV1} MP-TLV2{key, sub-TLV2} Key all sorted. So, I’m trying to find a case where MP-TLV would not include the key and I would have to go to the parent (especially since the text in section 4 says I must put the key in). Probably, all that this says is, if you need to know the key and the key is not in the sub-TLV, look in the parent. That is, of course, true. But it is nothing to do with MP-TLV: just a bit of ISIS know-how. --- I think there is some confusion in Section 6 with: Multi-part procedures may also be applicable to codepoints which do not support sub-TLVs, but which define an unbounded number of attributes which may be advertised in a single codepoint. An example of the latter is GMPLS-SRLG as defined in [RFC5307]. There is some confusion over the wording here because 5307 *does* use sub-TLVs, even if the SRLG sub-TLV has an unbounded number of attributes (controlled by the length field). How about… Multi-part procedures may also be applicable both to TLV codepoints which do not support sub-TLVs and to those which define an unbounded number of attributes which may be advertised within a single sub-TLV. An example of the latter is GMPLS-SRLG as defined in [RFC5307]. However, my concern (expressed in a comment on section 5) is that you can’t make MP-TLVs arbitrarily because of the rule that says you can re-assemble in any order. = Nits = Section 1 has: Today, for example, the Extended IS Reachability TLV (22) [RFC5305] and MT Intermediate Systems TLV (222) [RFC5120] are TLVs where existing standards do not specify sending multiple TLVs for the same object and no other mechanism for expanding the information carrying capacity of the TLV has been specified. Consider that you hope to publish this as an RFC that will persist. You need text that will survive the test of time, and also will not contradict the document itself. How about... For example, [RFC5305] defines the Extended IS Reachability TLV (22) and [RFC5120] defines the MT Intermediate Systems TLV (222). Those documents do not specify sending multiple TLVs for the same object. --- Section 1 has: [RFC7356] has proposed a 16 bit length field for TLVs in flooding scoped Protocol Data Units (PDUs), but this does not address how to expand the information advertised when using the existing 8-bit length TLVs. RFC 7356 is a standards track document, so it didn't "propose", it "defined". How about... [RFC7356] defined a 16 bit length field for TLVs in flooding scoped Protocol Data Units (PDUs), but does not address how to advertise more information when using the existing 8-bit length TLVs. --- Section 1 has: The mechanism described in this document has not been documented for all TLVs previously, so it is likely that some implementations would not interoperate correctly if these mechanisms were used without caution. This is foundational motivation for the document, so maybe it could be made a little clearer? How about... The mechanism described in this document has been implemented, but without formal documentation, there is a risk that these and new implementations would not interoperate correctly. This document provides the necessary protocol definition. --- Finally in Section 1: The mechanism described in this document has been used explicitly by some implementations, so this document is not creating an unprecedented mechanism. It is specifying a means for extending TLVs where no extension mechanism has been previously specified, and defining a default extension mechanism for future TLVs, if they choose not to specify another extension mechanism. The mechanism described in this document is applicable to top level TLVs as well as any level of sub-TLVs which may appear within a top level TLV. This feels like an over-sell to me. Having already had the previous paragraph, you could safely slim this down to: This document specifies a means for extending TLVs where no extension mechanism has been previously specified, and defines a default extension mechanism for future TLVs. The mechanism described in this document is applicable to top level TLVs as well as any level of sub-TLVs that may appear within a top level TLV. --- Section 5 has a couple of instances of “NOT”. It is general good practice to only use upper case in the BCP14 phrases.
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
