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

"Kamran Raza (skraza)" <skraza@cisco.com> Fri, 15 September 2017 03:21 UTC

Return-Path: <skraza@cisco.com>
X-Original-To: mpls@ietfa.amsl.com
Delivered-To: mpls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6A2AC132D6D; Thu, 14 Sep 2017 20:21:14 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.52
X-Spam-Level:
X-Spam-Status: No, score=-14.52 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fydtT1ueEBid; Thu, 14 Sep 2017 20:21:11 -0700 (PDT)
Received: from alln-iport-8.cisco.com (alln-iport-8.cisco.com [173.37.142.95]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9F2CF13293F; Thu, 14 Sep 2017 20:21:11 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=10490; q=dns/txt; s=iport; t=1505445671; x=1506655271; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=jsAsBlR6ZwPrIm1nxaZvmY6jFw564w36AG+VM9uSUPo=; b=K2434yMRUAMk1/cFDyEwHzL28Fairy9XCKqAN/rp8X6RtRMcuXwJjD1D xs3XE/LJASliEUoh35sLt1dZmo3gEPvYeb2SO32jw18Dal6rA52AWcp18 rTNbudqGc5Q8sm7G/OFJQeSM0C6GKcmXLWnaqI7eq2GuhWxtAkx4ZXQal w=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0CVAQD8RbtZ/5pdJa1dGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBg1qBUicHg3CaEYF0iDuPfwqFPAIahApXAQIBAQEBAQJrKIUYAQE?= =?us-ascii?q?BAwEjEUUMBAIBBgIRAwECAwImAgICHxEVCAgCBAENBYobAw0IjWCdZoInhzcNg?= =?us-ascii?q?24BAQEBAQEBAQEBAQEBAQEBAQEBAQEdgQ6CHYICg16CfYJYgiAXgnwvghIfBaB?= =?us-ascii?q?GPAKPWYR3ghOFaIp7jFiILAIRGQGBOAFXgQ13FVwBhQYcgWd2iASBDwEBAQ?=
X-IronPort-AV: E=Sophos;i="5.42,396,1500940800"; d="scan'208";a="3218570"
Received: from rcdn-core-3.cisco.com ([173.37.93.154]) by alln-iport-8.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Sep 2017 03:21:10 +0000
Received: from XCH-RCD-001.cisco.com (xch-rcd-001.cisco.com [173.37.102.11]) by rcdn-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id v8F3LAdX019964 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 15 Sep 2017 03:21:10 GMT
Received: from xch-aln-013.cisco.com (173.36.7.23) by XCH-RCD-001.cisco.com (173.37.102.11) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Thu, 14 Sep 2017 22:21:09 -0500
Received: from xch-aln-013.cisco.com ([173.36.7.23]) by XCH-ALN-013.cisco.com ([173.36.7.23]) with mapi id 15.00.1263.000; Thu, 14 Sep 2017 22:21:09 -0500
From: "Kamran Raza (skraza)" <skraza@cisco.com>
To: "Rajiv Asati (rajiva)" <rajiva@cisco.com>, Dean Bogdanovic <ivandean@gmail.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "mpls@ietf.org" <mpls@ietf.org>, "draft-ietf-mpls-ldp-yang.all@ietf.org" <draft-ietf-mpls-ldp-yang.all@ietf.org>, "ietf@ietf.org" <ietf@ietf.org>
Thread-Topic: Yangdoctors early partial review of draft-ietf-mpls-ldp-yang-01
Thread-Index: AQHTKKPo+FqDv+b2UUyu+mL0tScMm6KrswaAgAmvaQA=
Date: Fri, 15 Sep 2017 03:21:09 +0000
Message-ID: <585B9E00-39CC-46D3-9A5F-66B95966A342@cisco.com>
References: <150487625250.17301.18089417716327090477@ietfa.amsl.com> <429D0F6E-8D87-4333-A383-585679BB0FD2@cisco.com>
In-Reply-To: <429D0F6E-8D87-4333-A383-585679BB0FD2@cisco.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/f.25.0.170815
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.86.252.33]
Content-Type: text/plain; charset="utf-8"
Content-ID: <FE24610051876A408C6A1BC914626740@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/J5Ad0F2bIyaxvY5dqnFQIYGTcLM>
Subject: Re: [mpls] Yangdoctors early partial review of draft-ietf-mpls-ldp-yang-01
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mpls>, <mailto:mpls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls/>
List-Post: <mailto:mpls@ietf.org>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mpls>, <mailto:mpls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 15 Sep 2017 03:21:14 -0000

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)" <rajiva@cisco.com> 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 <ivandean@gmail.com>
    Date: Friday, September 8, 2017 at 9:10 AM
    To: "yang-doctors@ietf.org" <yang-doctors@ietf.org>
    Cc: "mpls@ietf.org" <mpls@ietf.org>rg>, "draft-ietf-mpls-ldp-yang.all@ietf.org" <draft-ietf-mpls-ldp-yang.all@ietf.org>rg>, IETF Discussion <ietf@ietf.org>
    Subject: Yangdoctors early partial review of draft-ietf-mpls-ldp-yang-01
    Resent-From: <alias-bounces@ietf.org>
    Resent-To: <skraza@cisco.com>om>, Rajiv Asati <rajiva@cisco.com>om>, <xufeng_liu@jabil.com>om>, <sesale@juniper.net>et>, <jescia.chenxia@huawei.com>om>, Himanshu Shah <hshah@ciena.com>om>, <swallow.ietf@gmail.com>om>, <tsaad@cisco.com>om>, "N.Leymann@telekom.de" <n.leymann@telekom.de>de>, Loa Andersson <loa@pi.nu>nu>, <aretana@cisco.com>om>, DEBORAH BRUNGARD <db3546@att.com>om>, <akatlas@gmail.com>
    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