Skip to main content

Early Review of draft-ietf-mpls-ldp-yang-01
review-ietf-mpls-ldp-yang-01-yangdoctors-early-bogdanovic-2017-09-08-00

Request Review of draft-ietf-mpls-ldp-yang-01
Requested revision 01 (document currently at 09)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2017-04-07
Requested 2017-03-13
Requested by Loa Andersson
Authors Syed Raza , Rajiv Asati , Xufeng Liu , Santosh Esale , Xia Chen , Himanshu C. Shah
Draft last updated 2017-09-08
Completed reviews Yangdoctors Early review of -01 by Dean Bogdanović (diff)
Yangdoctors Early review of -02 by Jan Lindblad (diff)
Genart Last Call review of -06 by Reese Enghardt (diff)
Rtgdir Last Call review of -06 by Yingzhen Qu (diff)
Yangdoctors Last Call review of -06 by Jan Lindblad (diff)
Secdir Telechat review of -07 by Shawn M Emery (diff)
Assignment Reviewer Dean Bogdanović
State Partially Completed
Review review-ietf-mpls-ldp-yang-01-yangdoctors-early-bogdanovic-2017-09-08
Reviewed revision 01 (document currently at 09)
Result On the Right Track
Completed 2017-09-08
review-ietf-mpls-ldp-yang-01-yangdoctors-early-bogdanovic-2017-09-08-00
In general, I like you design with base and extended model. Think it is a very
good approach in modeling and maybe I as author should have taken a similar
approach when modeling ACLs.

But there is a bit of confusion for me

Why are IPv4 and v6 separated? Why is IPv6 missing from the base model? The RFC
5036 is specifying both AF is the document. And you are mentioning that RFC
5036 is used for base model. Wy having same hierarchy but different namespaces?

4.1.1. Base

1. should 'discovery' be under 'global'?

Especially the 'interfaces' section. 'Interfaces' are directly attached to the
device. Compared to peers which is the remote end (either that be directly
connected peers, or peers that are more than 1 hop away). If peers are not
under global, it seems that 'discovery' and especially 'discovery/interfaces'
should not be under 'global'.

This applies to 'discovery/targeted' as well.

module: ietf-mpls-ldp
augment /rt:routing/rt:control-plane-protocols:
   +--rw mpls-ldp!
      +--rw global
      |  +--rw config
      |  |  +--rw capability
      |  |  +--rw graceful-restart
      |  |  |  +--rw enable?                boolean
      |  |  |  +--rw reconnect-time?        uint16
      |  |  |  +--rw recovery-time?         uint16
      |  |  |  +--rw forwarding-holdtime?   uint16
      |  |  +--rw lsr-id?             yang:dotted-quad
      |  +--rw address-families
      |  |  +--rw ipv4
      |  |     +--rw config
      |  |        +--rw enable?         boolean
      |  |        +--rw label-policy
      |  |           +--rw advertise
      |  |              +--rw egress-explicit-null
      |  |                 +--rw enable?   boolean
      |  +--rw discovery
      |     +--rw interfaces
      |     |  +--rw config
      |     |  |  +--rw hello-holdtime?   uint16
      |     |  |  +--rw hello-interval?   uint16
      |     |  +--rw interface* [interface]
      |     |     +--rw interface           mpls-interface-ref
      |     |     +--rw address-families
      |     |        +--rw ipv4
      |     |           +--rw config
      |     |              +--rw enable?   boolean

9.1 Base

   leaf peer-ldp-id {
     type yang:dotted-quad;
     description "Peer LDP ID.";
   }

Nit: "peer-" prefix needed?

Is this value different from peer LSR ID?

In general, I believe you are on the right path, but I don’t have deep LDP
knowledge, so can’t understand some of the nuances that you might have wanted
to address