Re: [Lsr] Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with COMMENT)
Hi Les, Looks good, thanks for making these tweaks. (I was about to write "hopefully final" but I see that half the IESG still has to weigh in ... so I can't really expect this to be the last word quite yet.) -Ben On Tue, Jul 14, 2020 at 03:39:55AM +, Les Ginsberg (ginsberg) wrote: > Ben - > > I have prepared V3 of the draft to address your comments. > As the window for draft submissions is temporarily closed, I have attached > the diffs for your review. > > I will post the updated version once the window for submissions reopens. > > A few more remarks inline. > > > -Original Message- > > From: Benjamin Kaduk > > Sent: Monday, July 13, 2020 4:24 PM > > To: Les Ginsberg (ginsberg) > > Cc: The IESG ; draft-ietf-lsr-isis-invalid-...@ietf.org; lsr- > > cha...@ietf.org; lsr@ietf.org; Christian Hopps ; > > aretana.i...@gmail.com > > Subject: Re: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: > > (with > > COMMENT) > > > > Hi Les, > > > > On Mon, Jul 13, 2020 at 11:05:35PM +, Les Ginsberg (ginsberg) wrote: > > > Ben - > > > > > > > > > > > > Thanx for your review. > > > > My pleasure; this is a nice document. (A shame it's needed, of course, but > > that's neither here nor there.) > > > > > Responses inline. > > > > > > > > > > > > > -Original Message- > > > > > > > From: Benjamin Kaduk via Datatracker > > > > > > > Sent: Monday, July 13, 2020 2:11 PM > > > > > > > To: The IESG > > > > > > > Cc: draft-ietf-lsr-isis-invalid-...@ietf.org; lsr-cha...@ietf.org; > > > > lsr@ietf.org; > > > > > > > Christian Hopps ; aretana.i...@gmail.com; > > > > > > > cho...@chopps.org > > > > > > > Subject: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: > > > > (with > > > > > > > COMMENT) > > > > > > > > > > > > > > Benjamin Kaduk has entered the following ballot position for > > > > > > > draft-ietf-lsr-isis-invalid-tlv-02: 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.) > > > > > > > > > > > > > > > > > > > > > Please refer to https://www.ietf.org/iesg/statement/discuss- > > criteria.html > > > > > > > for more information about IESG DISCUSS and COMMENT positions. > > > > > > > > > > > > > > > > > > > > > The document, along with other ballot positions, can be found here: > > > > > > > https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-invalid-tlv/ > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > COMMENT: > > > > > > > -- > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > [Les:] What prompted the writing of this document was a real world > > interoperability scenario wherein one implementation generated a > > malformed TLV and a receiving implementation rejected the entire PDU > > because of it. This resulted in persistent LSPDB inconsistency in the > > network > > f
Re: [Lsr] Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with COMMENT)
Ben - I have prepared V3 of the draft to address your comments. As the window for draft submissions is temporarily closed, I have attached the diffs for your review. I will post the updated version once the window for submissions reopens. A few more remarks inline. > -Original Message- > From: Benjamin Kaduk > Sent: Monday, July 13, 2020 4:24 PM > To: Les Ginsberg (ginsberg) > Cc: The IESG ; draft-ietf-lsr-isis-invalid-...@ietf.org; lsr- > cha...@ietf.org; lsr@ietf.org; Christian Hopps ; > aretana.i...@gmail.com > Subject: Re: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with > COMMENT) > > Hi Les, > > On Mon, Jul 13, 2020 at 11:05:35PM +, Les Ginsberg (ginsberg) wrote: > > Ben - > > > > > > > > Thanx for your review. > > My pleasure; this is a nice document. (A shame it's needed, of course, but > that's neither here nor there.) > > > Responses inline. > > > > > > > > > -Original Message- > > > > > From: Benjamin Kaduk via Datatracker > > > > > Sent: Monday, July 13, 2020 2:11 PM > > > > > To: The IESG > > > > > Cc: draft-ietf-lsr-isis-invalid-...@ietf.org; lsr-cha...@ietf.org; > > > lsr@ietf.org; > > > > > Christian Hopps ; aretana.i...@gmail.com; > > > > > cho...@chopps.org > > > > > Subject: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with > > > > > COMMENT) > > > > > > > > > > Benjamin Kaduk has entered the following ballot position for > > > > > draft-ietf-lsr-isis-invalid-tlv-02: 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.) > > > > > > > > > > > > > > > Please refer to https://www.ietf.org/iesg/statement/discuss- > criteria.html > > > > > for more information about IESG DISCUSS and COMMENT positions. > > > > > > > > > > > > > > > The document, along with other ballot positions, can be found here: > > > > > https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-invalid-tlv/ > > > > > > > > > > > > > > > > > > > > -- > > > > > COMMENT: > > > > > -- > > > > > > > > > > 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. > > > > > > > > > [Les:] What prompted the writing of this document was a real world > interoperability scenario wherein one implementation generated a > malformed TLV and a receiving implementation rejected the entire PDU > because of it. This resulted in persistent LSPDB inconsistency in the network > for a prolonged period with a significant impact on the proper functioning of > the network. This failure was considered significant enough that Standards > Track seemed appropriate. > > > > > > > > As the document developed, the authors were encouraged to address > some other issues, one of which was clarifying disallowed TLV handling as > well. > > > > > > > > I can understand why Informational track may seem appropriate to you. In > early discussions with Alvaro I had suggested that there was no need to write > the document - that existing specifications were sufficiently clear. But the > fact that - despite existing standards - such an interoperability issue did > occur > was compelling. The WG also embraced this as useful. > > Thanks for the extra explanation. I think PS makes sense, and the only > text change I might (emphasis: *might*) consider is to emphasize that th
Re: [Lsr] Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with COMMENT)
Hi Les, On Mon, Jul 13, 2020 at 11:05:35PM +, Les Ginsberg (ginsberg) wrote: > Ben - > > > > Thanx for your review. My pleasure; this is a nice document. (A shame it's needed, of course, but that's neither here nor there.) > Responses inline. > > > > > -Original Message- > > > From: Benjamin Kaduk via Datatracker > > > Sent: Monday, July 13, 2020 2:11 PM > > > To: The IESG > > > Cc: draft-ietf-lsr-isis-invalid-...@ietf.org; lsr-cha...@ietf.org; > > lsr@ietf.org; > > > Christian Hopps ; aretana.i...@gmail.com; > > > cho...@chopps.org > > > Subject: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with > > > COMMENT) > > > > > > Benjamin Kaduk has entered the following ballot position for > > > draft-ietf-lsr-isis-invalid-tlv-02: 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.) > > > > > > > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > > > for more information about IESG DISCUSS and COMMENT positions. > > > > > > > > > The document, along with other ballot positions, can be found here: > > > https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-invalid-tlv/ > > > > > > > > > > > > -- > > > COMMENT: > > > -- > > > > > > 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. > > > > > [Les:] What prompted the writing of this document was a real world > interoperability scenario wherein one implementation generated a malformed > TLV and a receiving implementation rejected the entire PDU because of it. > This resulted in persistent LSPDB inconsistency in the network for a > prolonged period with a significant impact on the proper functioning of the > network. This failure was considered significant enough that Standards Track > seemed appropriate. > > > > As the document developed, the authors were encouraged to address some other > issues, one of which was clarifying disallowed TLV handling as well. > > > > I can understand why Informational track may seem appropriate to you. In > early discussions with Alvaro I had suggested that there was no need to write > the document - that existing specifications were sufficiently clear. But the > fact that - despite existing standards - such an interoperability issue did > occur was compelling. The WG also embraced this as useful. Thanks for the extra explanation. I think PS makes sense, and the only text change I might (emphasis: *might*) consider is to emphasize that the "occurrence of non-conformant behavior seen in real world deployments" is decidedly not hypothetical. But I could understand if the current text is seen to be saying that already, too. > > > > 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). > > > > > [Les:] Rejecting a PDU because of some issue with a single TLV would mean > that the full set of updates contained in that LSP would not be propagated. > This has a significant impact on the correct operation of the protocol. So I > think this really isn’t an option. I agree that doing anything else would have been a bad idea! I was just suggesting that a different word might be preferred (but am not trying to press the issue). &
Re: [Lsr] Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with COMMENT)
Ben - Thanx for your review. Responses inline. > -Original Message- > From: Benjamin Kaduk via Datatracker > Sent: Monday, July 13, 2020 2:11 PM > To: The IESG > Cc: draft-ietf-lsr-isis-invalid-...@ietf.org; lsr-cha...@ietf.org; > lsr@ietf.org; > Christian Hopps ; aretana.i...@gmail.com; > cho...@chopps.org > Subject: Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with > COMMENT) > > Benjamin Kaduk has entered the following ballot position for > draft-ietf-lsr-isis-invalid-tlv-02: 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.) > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-invalid-tlv/ > > > > -- > COMMENT: > -- > > 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. > [Les:] What prompted the writing of this document was a real world interoperability scenario wherein one implementation generated a malformed TLV and a receiving implementation rejected the entire PDU because of it. This resulted in persistent LSPDB inconsistency in the network for a prolonged period with a significant impact on the proper functioning of the network. This failure was considered significant enough that Standards Track seemed appropriate. As the document developed, the authors were encouraged to address some other issues, one of which was clarifying disallowed TLV handling as well. I can understand why Informational track may seem appropriate to you. In early discussions with Alvaro I had suggested that there was no need to write the document - that existing specifications were sufficiently clear. But the fact that - despite existing standards - such an interoperability issue did occur was compelling. The WG also embraced this as useful. > 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). > [Les:] Rejecting a PDU because of some issue with a single TLV would mean that the full set of updates contained in that LSP would not be propagated. This has a significant impact on the correct operation of the protocol. So I think this really isn’t an option. > 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. > [Les:] Well spotted. I see you have put your newly acquired copy of ISO 10589 to good use. Bravo!! > 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. > [Les:] Fair enough. I will change this to RFC6232 - which is one of documents updated by this draft. >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"? [Les:] This sentence is serving as a guideline for new behaviors that have backwards compatibi
[Lsr] Benjamin Kaduk's Yes on draft-ietf-lsr-isis-invalid-tlv-02: (with COMMENT)
Benjamin Kaduk has entered the following ballot position for draft-ietf-lsr-isis-invalid-tlv-02: 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.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-invalid-tlv/ -- COMMENT: -- 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. ___ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
