Re: [yang-doctors] Yangdoctors last call review of draft-ietf-bfd-yang-09

[Juergen Schoenwaelder <[e-mail]>]

On Sun, Mar 04, 2018 at 02:12:30PM +0000, Reshad Rahman (rrahman) wrote:
> 
> We have made the changes in revs 10 and 11 to address your comments . The exception is module ietf-bfd-types which did not get renamed per reason below.
>

Hi,

here is my re-review of draft-ietf-bfd-yang. I think the document has
significantly improved since the -09 version, the authors have done an
excellent job to improve the document quality.

I have mostly a few minor mostly editorial issues left, except the
first one, which concerns the schema mount use case.

- Thanks for clarifying that the modules can be used on standalone
  devices. The new text is helpful.

  For the LNE and NI use cases, does it make sense to detail the mount
  points that are used? My understanding is that schema mount requires
  that mount points are identified with a "mount-point" extension
  statement, i.e., you can't mount at arbitrary places in the
  hierarchy but only at places that have been designated as mount
  points.

  That all said, since your YANG modules are basically augmenting
  other YANG modules that may be mounted, you do not seem to need a
  separate schema mount. If my understanding is correct, then here is
  a starting point for making this clearer:

  OLD

    When used at the network device level, the BFD YANG model is used
    "as-is".  When the BFD model is to be used in a Logical Network
    Element or in a Network Instance, the approach taken is to do a
    schema-mount (see Schema Mount [I-D.ietf-netmod-schema-mount]) of the
    BFD model in the appropriate location.  For example, if an
    implementation supports BFD IP multihop in network instances, the
    implementation would do schema-mount of the BFD IP multihop model in
    a mount-point which resides in a network instance.

  NEW

    When used at the network device level, the BFD YANG model are used
    "as-is".  When the BFD YANG model is used in a Logical Network
    Element or in a Network Instance, then the BFD YANG model augments
    the mounted routing model for the Logical Network Element or the
    Network Instance.

  Note that with this change, you also do not need a reference to
  schema mount.
  
- Since the different use cases (device, LNE, NI) are discussed right
  at the beginning of Section 2, it seems the following statements in
  Sections 2.5, 2.6, 2.7, 2.8, 2.9 are not really needed:

                                   The "bfd" node under control-plane-
   protocol can be used in a network device (top-level), or mounted in
   an LNE or in a network instance.

                                            The "ip-sh" node can be used
   in a network device (top-level), or mounted in an LNE or in a network
   instance.

                                                            The "ip-mh"
   node can be used in a network device (top-level), or mounted in an
   LNE or in a network instance.

                              The "lag" node can be used in a network
   device (top-level), or mounted in an LNE or in a network instance.

                                                             The "mpls"
   node can be used in a network device (top-level), or mounted in an
   LNE or in a network instance.

- The text at the beginning of Section 2.13 should also mention RFC
  8177 since you are importing it.

- It might be useful to give more explicit instructions to IANA. I
  assume you want IANA to update the iana-bfd-types module whenever
  changes are made to the "BFD Diagnostic Codes" registry and "BFD
  Authentication Types" registries. Giving clear instructions what
  IANA is expected to do and when is better than a soft statement such
  as "intended to reflect". But IANA is going to ask questions about
  this anyway during their review I assume.

- The feature definitions in ietf-bfd-types have text of the form "as
  defined in RFC 5880" and perhaps it makes sense to add reference
  statements to these feature definitions. There are also a number of
  identities that say "as per RFC 588X" where perhaps reference
  statements should be added.

- The text at the beginning of Section 2.13 should also	mention	RFC
  6991 since you are importing it. And you are also importing from
  RFC XXXX (the routing model).

- The text at the beginning of Section 2.16 should also mention
  that you import from RFC XXXX (the routing model).

- The text at the beginning of Section 2.17 should also mention that
  you import from RFC 6991 and from RFC XXXX (the routing model).

- The text at the beginning of Section 2.18 should also mention that
  you import from RFC XXXX (the routing model).

- The text at the beginning of Section 2.19 should also mention that
  you import from RFC XXXX (the routing model).

- I have not validated the examples - I hope the authors have done so.
  They look more plausible than in the previous version I reviewed.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <https://www.jacobs-university.de/>