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

"Rajiv Asati (rajiva)" <rajiva@cisco.com> Fri, 08 September 2017 19:25 UTC

Return-Path: <rajiva@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 51882132D40; Fri, 8 Sep 2017 12:25:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.521
X-Spam-Level:
X-Spam-Status: No, score=-14.521 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, 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 irkDeGhe0Wxg; Fri, 8 Sep 2017 12:25:39 -0700 (PDT)
Received: from alln-iport-2.cisco.com (alln-iport-2.cisco.com [173.37.142.89]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 80FC81320C9; Fri, 8 Sep 2017 12:25:36 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=8080; q=dns/txt; s=iport; t=1504898736; x=1506108336; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=RpyWVwvIaHoWH0D8KQsQUkzL3eUQ4Jb2DbBKrEccm3M=; b=eBXphGQZjJshljOASE/DebNi2LEjlkvfO4zdqIWkYEkFxAbDIU6UiTpJ YFa2OtgaIpaOnfrh1PlgOZpik8VHAJFa3dBvYjAqgvU3K4tL49PYvpFK6 Lg6/EYZWcymDmrO2Kz0ClaF/k8khf7OhqDjuP4Y1SgizK44cQpk5dTyL5 w=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0BcAwDI7bJZ/51dJa1cGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBg1qBUicHg3CaRIFPIog5kAEKhT4CGoNxVwECAQEBAQECayiFGAE?= =?us-ascii?q?BAQMBIxFFBQcEAgEIDgMDAQIDAiYCAgIfERUICAIEAQ0FihkDDQirZIInhzYNg?= =?us-ascii?q?3sBAQEBAQEBAQEBAQEBAQEBAQEBAQEdgQ2CHYICgU6CDguCcoJXhTEwghIfBaA?= =?us-ascii?q?4PAKPWYR2ghOFZ4p3jFOIKwIRGQGBOAFXgQ13FVsBhwh2iQ+BDwEBAQ?=
X-IronPort-AV: E=Sophos;i="5.42,363,1500940800"; d="scan'208";a="869390"
Received: from rcdn-core-6.cisco.com ([173.37.93.157]) by alln-iport-2.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Sep 2017 19:25:35 +0000
Received: from XCH-ALN-005.cisco.com (xch-aln-005.cisco.com [173.36.7.15]) by rcdn-core-6.cisco.com (8.14.5/8.14.5) with ESMTP id v88JPZm4027843 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 8 Sep 2017 19:25:35 GMT
Received: from xch-aln-005.cisco.com (173.36.7.15) by XCH-ALN-005.cisco.com (173.36.7.15) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Fri, 8 Sep 2017 14:25:35 -0500
Received: from xch-aln-005.cisco.com ([173.36.7.15]) by XCH-ALN-005.cisco.com ([173.36.7.15]) with mapi id 15.00.1263.000; Fri, 8 Sep 2017 14:25:35 -0500
From: "Rajiv Asati (rajiva)" <rajiva@cisco.com>
To: 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: AQHTKKPpbVk+Ny30A0KzoEGGf9xliKKrb/cA
Date: Fri, 8 Sep 2017 19:25:35 +0000
Message-ID: <429D0F6E-8D87-4333-A383-585679BB0FD2@cisco.com>
References: <150487625250.17301.18089417716327090477@ietfa.amsl.com>
In-Reply-To: <150487625250.17301.18089417716327090477@ietfa.amsl.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.150.190.167]
Content-Type: text/plain; charset="utf-8"
Content-ID: <C0C31DC14445E246845A8B9D6AE2BADA@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/1FJicS6fe-kG2Wh8BPaEOBHzkX4>
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, 08 Sep 2017 19:25:41 -0000

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.


>    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?
   
>       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.

>    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