Hi, Mahesh, Thank you for addressing my comments. It appears that the PR link you provided below is not accessible to me. Please feel free to submit a new revision when you believe it is ready and I will review then. Please also see some of my responses below. Thanks.
Best Regards, Qiufang From: Mahesh Jethanandani [mailto:[email protected]] Sent: Sunday, September 22, 2024 7:56 AM To: maqiufang (A) <[email protected]> Cc: YANG Doctors <[email protected]>; [email protected]; rtg-bfd@ietf. org <[email protected]> Subject: Re: Yangdoctors early review of draft-ietf-bfd-optimizing-authentication-18 Hi Qiufang, Thanks for your thorough review. Here are some comments. The changes are being tracked as part of the PR here<https://github.com/bfd-wg/optimized-auth/pull/50>. On Aug 1, 2024, at 11:50 PM, Qiufang Ma via Datatracker <[email protected]<mailto:[email protected]>> wrote: Reviewer: Qiufang Ma Review result: Ready with Nits Hi, this is my YANG Doctor review of draft-ietf-bfd-optimizing-authentication, the requested revision is 16, but it is currently at version 18, so my review is based on the latest. This draft defines a YANG module which augments the base BFD YANG model in RFC 9314, and also has an IANA-maintained module in Appendix which updates the initial one in RFC 9127. Both YANG modules have been parsed by yanglint and pyang, which didn’t generate any warnings and errors. Some nits that need to be fixed: 1. Sec.5.1 states “Finally, it adds a flag to enable optimized authentication, an interval value that specifies how often the BFD session should be re-authenticated once it is in the Up state, and the key chain that should be used in the Up state.” But I think the YANG module only defines the reauth-interval, which is inconsistent with the narrative description. Fixed. 2. The YANG module in sec.5.3 imports a set of modules from RFC 9314, but the reference statement to RFC 9314 should be: OLD: reference "RFC 9314: YANG Data Model for Bidirectional Forwarding Detection."; NEW: reference "RFC 9314: YANG Data Model for Bidirectional Forwarding Detection (BFD)”; Fixed. 3. The YANG module in sec.5.3, reference statement to RFC 8177 should be: OLD: reference "RFC 8177: YANG Key Chain."; NEW: reference " RFC 8177: YANG Data Model for Key Chains"; Fixed. 4. The YANG module in sec.5.3, please update the reference for identity definitions optimized-md5-meticulous-keyed-isaac and optimized-sha1-meticulous-keyed-isaac as follows: OLD: reference "I-D.ietf-bfd-optimizing-authentication: Meticulous Keyed ISAAC for BFD Authentication. I-D.ietf-bfd-secure-sequence-numbers: Meticulous Keyed ISAAC for BFD Authentication."; NEW: reference "RFC XXXX: Optimizing BFD Authentication RFC YYYY: Meticulous Keyed ISAAC for BFD Authentication"; And also add a note to RFC editor that YYYY is the number assigned to I-D.ietf-bfd-secure-sequence-numbers at the time of publication. I agree with the change for draft-ietf-bfd-optimizing authentication, but the other draft is a separate draft (even though it is part of the same cluster of documents). I would therefore keep the reference as is. While personally I prefer the suggested approach for a cluster of documents, I’ll leave this to you to decide. 5. The YANG module in sec.5.3, the description for all the augment substatements are identical, distinction should be made here between the descriptions of different modules being augmented. Fixed. 6. Sec.6.4 requests an update to the IANA-maintained YANG module “iana-bfd-types.yang”, maybe it should also mention the revision of this YANG module is to mirror the update to the registry “BFD Authentication Types” as requested in sec.6.1. I am not sure. The IANA maintained YANG module in Appendix already carries the updates the registry “BFD Authentication Types”. Also, the “Note to RFC Editor” in Section 2.1, already carries instructions on how to update the revision of the YANG module. What I am suggesting is to be consistent with what is defined in sec.5.1 of RFC 9127. Feel free to accept or reject. 7. Appendix A, the description: OLD: This version of this YANG module is part of RFC 9127; see the RFC itself for full legal notices. NEW: The initial version of this YANG module is part of RFC 9127; see the RFC itself for full legal notices. (and I think the reference below should be RFC XXXX instead of RFC 9127) The initial version of the YANG module is the one in RFC 9127, so I would agree to add the word ‘initial’. The module in Appendix A is updating that module. 8. Appendix A, the reference: OLD: reference "I-D.ietf-bfd-optimizing-authentication: Optimizing BFD Authentication, I-D.ietf-bfd-stability: BFD Stability."; NEW: reference "RFC XXXX: Optimizing BFD Authentication RFC ZZZZ: BFD Stability"; And also add a note to RFC editor that ZZZZ is the number assigned to I-D.ietf-bfd-stability at the time of publication. Agree to change the reference for the first item. For the second reference see response above. 9. RFC 8340 should be informative reference rather than normative one. See section 3.4 in https://datatracker.ietf.org/doc/draft-ietf-netmod-rfc8407bis/: “If YANG tree diagrams are used, then an informative reference to the YANG tree diagrams specification MUST be included in the document." Done.
