Hi Les, Thanks for the prompt follow-up. Much appreciated as the context is still fresh in my mind ;-)
Please see inline. Cheers, Med > -----Message d'origine----- > De : Les Ginsberg (ginsberg) <ginsb...@cisco.com> > Envoyé : mercredi 26 mars 2025 00:08 > À : BOUCADAIR Mohamed INNOV/NET <mohamed.boucad...@orange.com>; The > IESG <i...@ietf.org> > Cc : draft-ietf-lsr-multi-...@ietf.org; lsr-cha...@ietf.org; > lsr@ietf.org; yingzhen.i...@gmail.com > Objet : RE: [Lsr] Mohamed Boucadair's Yes on draft-ietf-lsr-multi-tlv- > 11: (with COMMENT) > > > Mohamed - > > Thanx for the very detailed review. > I have published V12 of the draft to address your comments. > > Please see my responses inline. > > > -----Original Message----- > > From: Mohamed Boucadair via Datatracker <nore...@ietf.org> > > Sent: Tuesday, March 25, 2025 3:14 AM > > To: The IESG <i...@ietf.org> > > Cc: draft-ietf-lsr-multi-...@ietf.org; lsr-cha...@ietf.org; > > lsr@ietf.org; yingzhen.i...@gmail.com > > Subject: [Lsr] Mohamed Boucadair's Yes on draft-ietf-lsr-multi-tlv- > 11: > > (with > > COMMENT) > > > > Mohamed Boucadair has entered the following ballot position for > > draft-ietf-lsr-multi-tlv-11: Yes > > > > 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.) > > > > > > > > > > -------------------------------------------------------------------- > -- > > COMMENT: > > -------------------------------------------------------------------- > -- > > > > Hi Parag, Tony L., Tony P., Shraddha, and Les, > > > > Many thanks for the effort put into this specification. > > > > I strongly support the objective of this effort and like the > pragmatic > > approach followed here. So, thanks again. > > > > I have some comments that I’d like to be addressed/discussed, > > especially the ones marked with (*). > > > > # General > > > > ## (nit) Please use consistent wording MP-TLV vs. Multi-Part TLV > > through the document > > > [LES:] Done [Med] Thanks > > > ## (nit) I may be old school, but I think the document should use > > “router”, instead of “node”. > > > [LES:] Done [Med] ACK. > > > # Abstract (nit): not every technologies is adding info into IGP + > > split a long > > sentence: > > > > OLD: > > New technologies are adding new information into IS-IS while > > deployment scales are simultaneously increasing, causing the > contents > > of many critical TLVs to exceed the currently supported limit of > 255 > > octets. > > > > NEW: > > Deploying some new techniques relies upon adding new information > > into IS- IS > > while deployment scales are simultaneously increasing. This > causes the > > contents of many critical TLVs to exceed the currently supported > limit of > > 255 octets. > > > [LES:] Done [Med] ACK. > > > # Introduction > > > > ## (nit) s/Some TLV definitions have addressed this by > explicitly/Some > > TLV definitions have addressed this issue by explicitly > > > > ## Justification for alternate mechanisms (*) > > > > CURRENT: > > > > Any future document which > > proposes a different mechanism for scaling TLV contents for a > given > > codepoint MUST provide justification. > > > > I’m not fun of normative language in an introduction (as the context > > is not yet well established), we may consider replacing “provide > > justification” with a more meaningful guidance, e.g., “identify the > > shortcomings of the multiple part approach and motivate the need for > a > > another mechanism”. > > > > Also, you may consider: s/ Any future document which proposes/Any > > future document that proposes standardizing > > > > ## I would delete ", if they choose not to specify another extension > > mechanism". > > > [LES:] Text has been revised. [Med] The new text in -12 about justification is much more clear. Thanks. > > > # Section 3.2.1 > > > > Please cite where type 22 is defined + remind how the identifiers > are encoded: > > > > OLD: > > As an example, consider the Extended IS Reachability TLV (type > 22). > > … > > * Optionally one or more of the following link identifiers: > > > > NEW: > > As an example, consider the Extended IS Reachability TLV (type > 22) > > [RFC5305]. > [LES:] Done > [Med] ACK. > > … > > * Optionally one or more of the following link identifiers > encoded as > > sub-TLVs (0-244 octets): > > > [LES:] Done - though I chose to omit the length qualifier. [Med] Wfm. Thanks. > > > # Section 4: On Keys > > > > CURRENT: > > The encoding of TLVs is not altered by the introduction of MP-TLV > > support. In particular, the "key" which is used to identify the > set > > of TLVs which form an MP-TLV is the same key used in the absence > of > > MP-TLV support. Also note the definition of the "key" exists in > the > > specification(s) which define(s) the TLV. > > > > NOTE: This document intentionally does not include a definition > of > > the key for each codepoint. To do so would be redundant and risk > > unintentionally deviating from the definition which already > exists in > > the relevant specifications. > > > > ## Maybe it is accurate to also remind in the note that term “key” > is > > not used per se in these specs. > > > [LES:] Done [Med] Thank you. Note that I would not surprised if some readers may see an internal consistent with the sentence right before the note " Also note the definition of the "key" exists in the specification(s)". We may soften that by tweaking that sentence, e.g., as follows: "Also note the definition of what is referred to as "key" exists in the specification(s)". Up to you to decide if a change is needed, but as far as I'm concerned I can live with the current text in -12. > > > ## (nit) BTW, please fix this part: s/specification(s) which > define(s) > > the > > TLV/specification(s) that define(s) a TLV. > > > [LES:] Done [Med] Thanks. > > > # Section 5 (*) > > > > CURRENT: > > When processing MP-TLVs, implementations MUST NOT impose a > minimum > > length check. > > > > Agree... however, should we have a max of MP-TLVs to be used as a > > guard for splitting the information into a large numbers of TLVs? > > [LES:] I see no reason to impose a maximum. Any number chosen would be > arbitrary and risks becoming "too small" in the future. [Med] I'm not asking to pick a random max value, but to have a knob for Operators to control a max based on their local policy. We need some guards here. > > > > > # Section 6 > > > > ## I don’t parse what is meant by "advertised in a codepoint". I > guess > > you meant advertised with or in a TLV with a type/codepoint. I > suggest > > to make the following change: > > > > s/may be advertised in a single codepoint /may be advertised with a > > single codepoint > > > [LES:] Revised to use the term "within". [Med] ACK. > > > ## Simplify > > > > OLD: > > This guarantees that > > any new codepoints defined by future protocol extensions will > > explicitly indicate the applicability of Multi-Part TLV > procedures to > > the new codepoints. > > > > NEW: > > Future allocations will > > explicitly indicate the applicability of Multi-Part TLV > procedures to > > the new codepoints. > > > [LES:] Revised - but chose a different wording. [Med] I like the new wording in -12. > > > # Section 7 > > > > ## Please elaborate more the "extremely disruptive" mention in the > following: > > > > CURRENT: > > Introduction of the use of MP-TLV for codepoints where the > existing > > specifications have not explicitly defined MP-TLV support can be > > extremely disruptive to network operations in cases where not all > > > [LES:] I added an example. [Med] Great. > > > ## I don’t parse the following: > > > > CURRENT: > > It is presumed that if such > > support is provided that it applies to all relevant codepoints. > > > > ## Consider adding examples of deployment scenarios > [LES:] Since the new sub-TLV does not support specifying for which > codepoints the implementation provides MP-TLV support, the only > conclusion that can be drawn from the advertisement is that the > implementation has "some" MP-TLV support. > Absent the realities of use cases and development time/testing, one > can assume this means any codepoint where MP-TLV support is applicable > has MP-TLV support in the implementation. > But the reality is quite different. In practice implementations will > add MP-TLV support for codepoints where the deployment requirements of > the customers require it. > > I really do not want to get into example deployment scenarios - the > possibilities are far too large. [Med] ACK. > > > > > OLD: > > a given implementation might limit MP- > > TLV support to particular codepoints based on the needs of the > > deployment scenarios in which it is used. > > > > NEW: > > a given implementation might limit MP- > > TLV support to particular codepoints based on the needs of the > > deployment scenarios in which it is used (e.g., set by default > > or controlled by a configuration parameter). > > > [LES:] Configuration controls are discussed in Section 8. [Med] I feel like we need to exemplify how the "the needs of deployment .." are made known to an implementation. > > > ## I suggest to delete the following. This message is already stated > > in the > > document: > > > > CURRENT: > > Note that with the introduction of explicit specification of MP- > TLV > > applicability for codepoints (Section 9), implicit MP-TLV support > > will never occur in the future. > > > [LES:] Done [Med] ACK. > > > ## What is the goal of this MUST if the implem is already > conformant?! > > (*) > > > > CURRENT: > > Where MP-TLV support is explicitly > > defined, conformant implementations MUST support MP-TLV. > > > [LES:] Removed. [Med] Thanks. > > > # Section 8: Not sure the normative language makes sense in the > > following > > text: > > > > CURRENT: > > Sending of MP-TLVs in the presence of nodes which do not > correctly > > process such advertisements can result in interoperability > issues, > > including incorrect forwarding of packets. This section > discusses > > best practices which SHOULD be used when a deployment requires > the > > use of MP-TLVs for codepoints for which existing specifications > do > > not explicitly indicate MP-TLV support. > > > [LES:] The use of SHOULD is intentional and was the subject of much > discussion during the WG review. Some members wanted a MUST here - but > authors pushed back because we believe it is not within the purview of > an RFC to mandate how an implementation controls feature enablement. > But we wanted to support the intent of advocating for such controls as > an aid to operators in safely deploying MP-TLV support. > I would appreciate not having to revisit this subject. 😊 [Med] Thanks for sharing the context. I think that SHOULD does not add much given that we already have a set of SHOULD/RECOMMENDED that provide the guidance. The rationale and explanatory text in this section is great: no mandate by spec, but a set of recommendations with clear justifications. That's exemplary! > > > > # Section 8.1 > > > > ## Configuration and defaults (*) > > > > CURRENT: > > It is RECOMMENDED that implementations which support the sending > of > > MP-TLVs provide configuration controls to enable/disable > generation > > of MP-TLVs. > > > > (1) Should we also be able to expose the actual (configurable) > capabilities? > > > [LES:] Not in the protocol itself. The purpose of a routing protocol > is to advertise what is used by the protocol. > What is configured/enabled is not of use to the protocol - it belongs > to management applications. [Med] It is related as this is important for those who will deploy (I care a little bit about those ;-)). If you have readily available pointers where such matters are discussed, please consider citing those. > Note that an outgrowth of this draft was the introduction of Protocol > Implementation Conformance Statements (PICS) using YANG. > Please see: [Med] Thanks for the pointers. > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > tracker.ietf.org%2Fdoc%2Fdraft-ietf-lsr-isis-pics-l2member-attr- > yang%2F&data=05%7C02%7Cmohamed.boucadair%40orange.com%7Ca92a988068f84a > 7f973408dd6bf1f527%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638785 > 409138744439%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwL > jAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C% > 7C&sdata=z%2BssYgnqLOWNfuFYAuld3HrJbmo7%2B8fLQWo%2B0tNP2Pw%3D&reserved > =0 > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > tracker.ietf.org%2Fdoc%2Fdraft-ietf-lsr-isis-pics-srmpls- > yang%2F&data=05%7C02%7Cmohamed.boucadair%40orange.com%7Ca92a988068f84a > 7f973408dd6bf1f527%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638785 > 409138752585%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwL > jAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C% > 7C&sdata=y1uGwDMnN77wqz1S8x5Hebbn3TzDm1OnzPtwD7%2FAiKs%3D&reserved=0 > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdata > tracker.ietf.org%2Fdoc%2Fdraft-ietf-lsr-isis-pics-srmpls- > yang%2F&data=05%7C02%7Cmohamed.boucadair%40orange.com%7Ca92a988068f84a > 7f973408dd6bf1f527%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0%7C0%7C638785 > 409138760434%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwL > jAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C% > 7C&sdata=FBxEe80gZGUpAEnPy3Sr5Jt6HFlWUwmIkRk%2BcjbMdXM%3D&reserved=0 > > > > (2) Given the implications of partial support, can we suggest the > > default be to disable? > > [LES:] The choice of default (enable/disable) is intentionally left as > an implementation choice. [Med] ACK. > > > > > ## Does the report in the following covers logging? Can we be > explicit > > here? (*) > > > [LES:] I am not clear on what is not explicit in the current text? [Med] I'm approaching alarm/log from a logic similar to rfc8632#section-3.1, where alarm/alarm reporting/logging are distinct concepts. > > > CURRENT: > > Implementations which support disablement of MP-TLVs MUST report > > alarms under the following conditions: > > > > # Section 9.2: (nit) s/ This document requests that IANA extend/This > > document requests that IANA extends > > > [LES:] Text modified. [Med] Thanks. > > > # Section 10 (*) > > > > CURRENT: > > This document creates no new security issues for IS-IS. > > > > Isn’t generalizing applicability of MP-TLV may be misused (e.g., > abuse > > the number of occurrences to consume computing resources of a > receiver)? > > > [LES:] No. Even in the absence of MP-TLV, nothing prevents an attacker > from sending multiple TLVs for the same object - potentially with > conflicting content. > It could be argued that with MP-TLV support implementations are > somewhat less vulnerable because at least they know how to deal with > MP-TLV if it is received. [Med] What about if you formulate this as a considerations and include it in that section? > > > Cheers, > > Med > [LES:] Appreciate the detailed review. [Med] Thanks for the meticulous changes made so far. ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ Lsr mailing list -- lsr@ietf.org To unsubscribe send an email to lsr-le...@ietf.org
