[yang-doctors] Re: Yangdoctors early review of draft-ietf-bfd-optimizing-authentication-18

["maqiufang (A)" <[e-mail]>]

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:[e-mail]]
Sent: Sunday, September 22, 2024 7:56 AM
To: maqiufang (A) <[e-mail]>
Cc: YANG Doctors <[e-mail]>; [e-mail]; rtg-bfd@ietf. org <[e-mail]>
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 <[e-mail]<mailto:[e-mail]>> 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.