Subject: I-D Action: draft-ietf-bfd-yang-05.txt

From: [email-masked]
Date: Fri, 10 Mar 2017 13:26:19 -0800

A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the Bidirectional Forwarding Detection of the IETF.

        Title           : Yang Data Model for Bidirectional Forwarding Detection (BFD)
        Authors         : Reshad Rahman
                          Lianshu Zheng
                          Santosh Pallagatti
                          Mahesh Jethanandani
                          Greg Mirsky
	Filename        : draft-ietf-bfd-yang-05.txt
	Pages           : 53
	Date            : 2017-03-10

Abstract:
   This document defines a YANG data model that can be used to configure
   and manage Bidirectional Forwarding Detection (BFD).

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-bfd-yang/

There's also a htmlized version available at:
https://tools.ietf.org/html/draft-ietf-bfd-yang-05

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-bfd-yang-05

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.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Fri, 10 Mar 2017 21:29:45 +0000

Main changes from rev 04 are:
- Removed augment of network-instance, added reference to schema-mount
- bfd is not top-level anymore, augments control-plane-protocol from
RFC8022
- Addeds section on Interaction with other YANG modules

Regards,
BFD YANG authors.

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 24 Mar 2017 12:14:04 -0400

Authors,

Thanks for continued work on the yang module.  It does appear it's
approaching a good level of maturity!

I have a few comments from my recent read-through.  I'd like to encourage
other members of the Working Group to review the module.  I'd also like to
encourage the authors to reach out to other protocol groups to check the
suitability of the module for interaction with their protocols.

Some stream of thought comments:
- TTL seems to be only a property of multi-hop sessions.  How do you
  configure GTSM for single-hop sessions?
- In single-hop, sessions are operationally keyed on "interface".  However,
  there's also a "out-interface" node.  How are they intended to be
  different?
- state-change-reason in notification is a string type.  While this is
  probably the fastest path to getting some code written, it's also the
  least machine usable mechanism.  Has there been any discussion among the
  authors on how that might otherwise be addressed?
- time-in-previous-state is oddly a string.  Why not a time of some sort?
- Future-proofing observation: An mpls-fec is not required to be an IP
  address, although that is pretty much its only major manifestation at the
  moment.  This may cause weirdness down the road.
- mpls-fec is of type ip-address.  FECs are typicall prefixes.
- bfd-diagnostic is missing code point 9. (See IANA issues.)
- Suggest adding the range restriction 0..31 to the description for
  bfd-diagnostic.
- Naming elements such as nbor, conc, etc. are rather unnecessarily
  truncated.  While I realize that yang is somewhat human readable, it's
  mostly for machine interaction.  Reasonably expanded names make a module
  easier to read, especially to English as a second language parties.  This,
  however, needs to be balanced against things like "admin" which is already
  present in other BFD documents.
- In the bfd-auth-replay-protection description, please include citations to
  the specific section of RFC 5880.
- The bfd-all-session grouping includes source-port and dest-port.  This may
  not be relevant to all transports of BFD.  What discussion have you given
  to restricting this, if any?
- rx-ttl's description is a bit confusing to read.  I think you are trying
  to suggest that the received TTL should be <= this value to be acceptable?
- potential nit on "ttl"; ipv6 calls this hop-count.  Any discussion on the
  naming of this type among the authors?

Features:
Echo and demand mode are not pervasively implemented for BFD.  (I've not
actually heard of an implementation of demand mode.)  I suspect these
elements should be covered by if-feature?  While I realize the elements are
optionally usable in the configuration leaves, the lack of a feature might
lead a netconf implementation with the thought that they can use this
feature.  What has been the author discussion on this point?

IANA management of some module components:
Several elements in this draft are things that will regularly need to be
maintained.  Examples include:
- bfd-path-type
- bfd-auth-replay-protection
- bfd-diagnostic

I suspect we wish to place these components in a module/modules that are
IANA maintained.  I'm a bit behind in current yang practices, but I suspect
we have something like this defined now.  It unfortunately has the headache
that it impacts name-spacing issues for the consuming modules.

Timing hints:
Has any discussion been given to an operational module that might suggest
what timer values are acceptable to an implementation?  Note: I'm not
suggesting this as part of the core module, but it's a common "what timers
does your implementation support" question that typically happens.

---

From: "Reshad Rahman (rrahman)" <[email-masked]>
Date: Sat, 25 Mar 2017 02:05:47 +0000

Thanks Jeff for review + detailed comments, please see inline.

Co-authors, feel free to chime in. We¹ll meet to discuss in Chicago.

>  configure GTSM for single-hop sessions?

<RR> TTL must be 255 as per section 5 of RFC5881?
 
>  there's also a "out-interface" node.  How are they intended to be

<RR> This is for the case where ³interface² is a virtual interface,
³out-interface² would indicate the physical interface. Thinking more about
this, we should consider taking it out of the base model, vendors who want
to support this can do augment.

>  authors on how that might otherwise be addressed?

<RR> You¹re the first one to bring it up :-) So you¹d like us to e.g. look
into defining an identity for this?
 
>- time-in-previous-state is oddly a string.  Why not a time of some sort?

<RR> I agree.

>  moment.  This may cause weirdness down the road.

<RR> So are you suggesting we use a choice too cater for this?

>- mpls-fec is of type ip-address.  FECs are typicall prefixes.

<RR> Ack.

>- bfd-diagnostic is missing code point 9. (See IANA issues.)

<RR> Ack.

>- Suggest adding the range restriction 0..31 to the description for

<RR> Ack.

>  present in other BFD documents.

<RR> Ack.

>  the specific section of RFC 5880.

<RR> Ack.

>  to restricting this, if any?

<RR> Good point. You¹re referring to encaps such as ACH I assume?

>  to suggest that the received TTL should be <= this value to be

<RR> >= from what recall. We¹ll discuss this amongst authors.

>  naming of this type among the authors?

<RR> None but we will.

>feature.  What has been the author discussion on this point?

<RR> We have had some discussions on this. I agree that we should add
if-feature. Authors will discuss.

>that it impacts name-spacing issues for the consuming modules.

<RR> Hadn¹t thought of that. I suspect BFD isn¹t the first module to need
thisŠ.

>does your implementation support" question that typically happens.

<RR> No discussions on this. We¹ll have to discuss. What is tricky with
this is that some implementations support different timer ranges based on
³path-type². E.g. 5 ms mayb be supported for single-hop but for multi-hop
the minimum could be 150 ms.

Regards,
Reshad.

---

From: Mahesh Jethanandani <[email-masked]>
Date: Sun, 26 Mar 2017 23:33:34 -0500

Jeff, 

Will echo Reshad's comment on the review. You should join the YANG doctors :-)

More inline

> On Mar 24, 2017, at 7:05 PM, Reshad Rahman (rrahman) <[email-masked]> wrote:
> 

Agree that this should be an augmentation. 

> if-feature. Authors will discuss.

Agree.

> <RR> Hadn¹t thought of that. I suspect BFD isn¹t the first module to need

While we need to put the module name in IANA list of modules for namespace reasons, I am not sure of individual elements.  Are these values not already maintained by IANA?

> the minimum could be 150 ms.

I seem to remember an effort to define the most commonly used timeout values.  If so, we could define them in the model, leaving individual implementations to add their own preferred values. 

Mahesh Jethanandani
[email-masked]