Hi Juergen,
Thanks again for the excellent review. We've just published rev12 to address
your latest comments.
Please see inline.
Regards,
Reshad.
On 2018-03-13, 10:58 AM, "Juergen Schoenwaelder"
<j.schoenwael...@jacobs-university.de> wrote:
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.
<RR> Done.
- 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.
<RR> Done
- The text at the beginning of Section 2.13 should also mention RFC
8177 since you are importing it.
<RR> Done
- 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.
<RR> Updated 5.1
- 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.
<RR> Added reference sections to the feature definitions and identities.
- 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).
<RR> 2.13 already mentions RFC 6991 but it was missing from 2.15 and 2.17 (it's
been added). 2.13 already has mention of 8022bis (routing model). 8022bis is
now rfc8349.
- The text at the beginning of Section 2.16 should also mention
that you import from RFC XXXX (the routing model).
<RR> We now mention rfc8349 (8022bis).
- 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).
<RR> Added mention of RFC6991.
- The text at the beginning of Section 2.18 should also mention that
you import from RFC XXXX (the routing model).
<RR> We now mention rfc8349 (8022bis).
- The text at the beginning of Section 2.19 should also mention that
you import from RFC XXXX (the routing model).
<RR> We now mention rfc8349 (8022bis).
- I have not validated the examples - I hope the authors have done so.
They look more plausible than in the previous version I reviewed.
<RR> Yes we have validated them using yanglint.
/js
--
Juergen Schoenwaelder Jacobs University Bremen gGmbH
Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany
Fax: +49 421 200 3103 <https://www.jacobs-university.de/>
--- Begin Message ---
A new version of I-D, draft-ietf-bfd-yang-12.txt
has been successfully submitted by Reshad Rahman and posted to the
IETF repository.
Name: draft-ietf-bfd-yang
Revision: 12
Title: YANG Data Model for Bidirectional Forwarding Detection (BFD)
Document date: 2018-03-20
Group: bfd
Pages: 74
URL: https://www.ietf.org/internet-drafts/draft-ietf-bfd-yang-12.txt
Status: https://datatracker.ietf.org/doc/draft-ietf-bfd-yang/
Htmlized: https://tools.ietf.org/html/draft-ietf-bfd-yang-12
Htmlized: https://datatracker.ietf.org/doc/html/draft-ietf-bfd-yang
Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-bfd-yang-12
Abstract:
This document defines a YANG data model that can be used to configure
and manage Bidirectional Forwarding Detection (BFD).
The YANG modules in this document conform to the Network Management
Datastore Architecture (NMDA).
Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.
The IETF Secretariat
--- End Message ---