Re: [mpls] Yangdoctors early partial review of draft-ietf-mpls-ldp-yang-01

["Kamran Raza (skraza)" <[e-mail]>]

Thanks for your review and comments. 

We further discussed the comments in our bi-weekly meeting amongst the authors.
Please see some clarifications/response inline tagged as [skraza].

On 2017-09-08, 3:25 PM, "Rajiv Asati (rajiva)" <[e-mail]> wrote:

    Hi Dean,
    
    Thank you so much for your review and feedback. Please see inline,
    
    >   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.
     
    Thanks for pointing it out. We co-authors discussed quite a bit on this before settling on it.
    
     
    >   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?
    
    Good point. I somewhat agree though there are 2 points to consider – 
    a) capabilities are covered mostly in Extended model (and LDP IPv6 requires a dual-stack capability)
    b) LDP IPv6 is mostly specified in rfc7552, beyond rfc5036
    
    Nonethless, co-authors could discuss and revert, given the v6-only may become a norm at some point.
    
[skraza]: We had discussed this matter at length in our meetings when doing the base/extended categorization.
In our opinion, “ipv6” LDP falls under extended given our definition of a “base” – see quote below from our draft:
“The "base" category contains the basic and fundamental features that
   are covered in LDP base specification [RFC5036] and constitute the
   minimum requirements for a typical base LDP deployment”
Note that LDP base RFC 5036 did not cover IPv6 fully/properly, and hence LDPv6 was not deployable until we came up with its own RFC 7552.

 
    
    >    4.1.1. Base
    >  
    >   1. should 'discovery' be under 'global'?
    
    It should be. 
    
    >    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'.
     
    Peers are under ‘global’. 
    
    Perhaps, you allude to making ‘discovery’ and ‘peers’ consistent – either under global or global config.  Is that it?
 
[skraza]: Infact, discovery, peer, and global should be on the same level. Looks like discovery got moved under “global” by mistake.
  We will fix it in our next major rev.

      
    >       leaf peer-ldp-id {
    >         type yang:dotted-quad;
    >         description "Peer LDP ID.";
    >       }
    >    Nit: "peer-" prefix needed?  
    
    No. 
    
    However, LDP ID should not be a dotted-quad, since it is a 6-octet value. 
    We will fix it in the next version.

[skraza]: We will define ldp-id type as { lsr-id:label-space-id}  and use it for peer-ldp-id.

Thx.
--
Kamran


    >    Is this value different from peer LSR ID?
        
    Yes. LSR ID is the first 4 octets of LDP ID.
     
        
    
    -- 
    Cheers,
    Rajiv Asati
    Distinguished Engineer, Cisco
    
    -----Original Message-----
    From: Dean Bogdanovic <[e-mail]>
    Date: Friday, September 8, 2017 at 9:10 AM
    To: "[e-mail]" <[e-mail]>
    Cc: "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, IETF Discussion <[e-mail]>
    Subject: Yangdoctors early partial review of draft-ietf-mpls-ldp-yang-01
    Resent-From: <[e-mail]>
    Resent-To: <[e-mail]>, Rajiv Asati <[e-mail]>, <[e-mail]>, <[e-mail]>, <[e-mail]>, Himanshu Shah <[e-mail]>, <[e-mail]>, <[e-mail]>, "[e-mail]" <[e-mail]>, Loa Andersson <[e-mail]>, <[e-mail]>, DEBORAH BRUNGARD <[e-mail]>, <[e-mail]>
    Resent-Date: Friday, September 8, 2017 at 9:10 AM
    
        
        Review is partially done. Another review request has been registered for
        completing it.
        
        Reviewer: Dean Bogdanovic
        Review result: On the Right Track
        
        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