Hi Dhruv, Thanks for the quick turnaround. It looks good to me. One nit, I believe a period was added to “However, as noted in [RFC6952].,” by mistake. Thanks, Acee
From: Lsr <lsr-boun...@ietf.org> on behalf of Dhruv Dhody <dhruv.i...@gmail.com> Date: Wednesday, October 5, 2022 at 9:06 AM To: Lars Eggert <l...@eggert.org> Cc: The IESG <i...@ietf.org>, "draft-ietf-lsr-pce-discovery-security-supp...@ietf.org" <draft-ietf-lsr-pce-discovery-security-supp...@ietf.org>, "lsr-cha...@ietf.org" <lsr-cha...@ietf.org>, "l...@ietf.org" <l...@ietf.org>, Acee Lindem <a...@cisco.com>, "pce@ietf.org" <pce@ietf.org>, "Les Ginsberg (ginsberg)" <ginsb...@cisco.com>, Adrian Farrel <adr...@olddog.co.uk>, John Scudder <j...@juniper.net>, Roman Danyliw <r...@cert.org> Subject: Re: [Lsr] Lars Eggert's Discuss on draft-ietf-lsr-pce-discovery-security-support-11: (with DISCUSS and COMMENT) Hi all, I have incorporated changes suggested by Les and Acee in the working copy maintained at - TXT - https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-lsr-pce-discovery-security-support-12.txt DIFF - https://www.ietf.org/rfcdiff?url1=draft-ietf-lsr-pce-discovery-security-support-11&url2=https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-lsr-pce-discovery-security-support-12.txt Also, see inline... On Fri, Sep 30, 2022 at 5:57 PM Lars Eggert via Datatracker <nore...@ietf.org<mailto:nore...@ietf.org>> wrote: Lars Eggert has entered the following ballot position for draft-ietf-lsr-pce-discovery-security-support-11: Discuss 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/about/groups/iesg/statements/handling-ballot-positions/ for more information about how to handle DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-lsr-pce-discovery-security-support/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- # GEN AD review of draft-ietf-lsr-pce-discovery-security-support-11 CC @larseggert ## Discuss ### Section 4, paragraph 3 ``` Section 4 of [RFC5088] states that no new sub-TLVs will be added to the PCED TLV, and no new PCE information will be carried in the Router Information LSA. This document updates [RFC5088] by allowing the two sub-TLVs defined in this document to be carried in the PCED TLV advertised in the Router Information LSA. Section 4 of [RFC5089] states that no new sub-TLVs will be added to the PCED TLV, and no new PCE information will be carried in the Router CAPABLITY TLV. This document updates [RFC5089] by allowing the two sub-TLVs defined in this document to be carried in the PCED TLV advertised in the Router CAPABILITY TLV. This introduction of additional sub-TLVs should be viewed as an exception to the [RFC5088][RFC5089] policy, justified by the requirement to discover the PCEP security support prior to establishing a PCEP session. The restrictions defined in [RFC5089][RFC5089] should still be considered to be in place. ``` (This is mostly for discussion on the telechat, and I expect to clear during the call.) Why were 5088/89 so strict on not allowing new sub-TLVs? This seems quite unusual for IETF specs. I'm not arguing that this document can't update those earlier RFCs to allow these new sub-TLVs, but it seems odd to do so and in the same sentence say "the restrictions should still be considered in place." ### Section 8.2, paragraph 1 ``` The PCED sub-TLVs were defined in [RFC5088] and [RFC5089], but they did not create a registry for it. This document requests IANA to create a new registry called "PCED sub-TLV type indicators" under the "Interior Gateway Protocol (IGP) Parameters" grouping. The registration policy for this registry is "IETF Review" [RFC8126]. Values in this registry come from the range 0-65535. ``` Should the registration policy not be stricter (e.g., Standards Action?) given that 5088/89 didn't even allow any new values? Dhruv: Updated to Standards Action. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- ## Comments ### Inclusive language Found terminology that should be reviewed for inclusivity; see https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance: * Term `master`; alternatives might be `active`, `central`, `initiator`, `leader`, `main`, `orchestrator`, `parent`, `primary`, `server` Dhruv: The term appears in reference to an existing term defined in another RFC. KeyID: The one octet Key ID as per [RFC5925] to uniquely identify the Master Key Tuple (MKT). Not sure how I can avoid using the term. * Term `man`; alternatives might be `individual`, `people`, `person` Dhruv: Updated (as also suggested by Roman) ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### URLs These URLs in the document can probably be converted to HTTPS: * http://www.unicode.org/unicode/reports/tr36/ Dhruv: Updated. ### Grammar/style #### "Abstract", paragraph 1 ``` for OSPF and IS-IS respectively. However these specifications lack a method ^^^^^^^ ``` A comma may be missing after the conjunctive/linking adverb "However". (Also elsewhere.) #### Section 1, paragraph 5 ``` ry" instead of the "IGP registry" where as [RFC8623] and [RFC9168] uses the ^^^^^^^^ ``` Did you mean "whereas"? #### Section 3.2.2, paragraph 3 ``` string to be used to identify the key chain. It MUST be encoded using UTF-8. ^^^^^^^^^ ``` This word is normally spelled as one. (Also elsewhere.) Dhruv: The "key chain" is quite common. 'Greping' in existing RFCs I found many instances! #### Section 5, paragraph 4 ``` enable a man-in-the-middle attack. Thus before advertising the PCEP security ^^^^ ``` A comma may be missing after the conjunctive/linking adverb "Thus". Dhruv: All other nits are incorporated. Thanks! Dhruv ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce