Re: [yang-doctors] [mpls] YANG Doctor Early Review for "YANG Data Model for MPLS mLDP" - draft-ietf-mpls-mldp-yang-03

"Kamran Raza (skraza)" <skraza@cisco.com> Mon, 26 March 2018 03:10 UTC

Return-Path: <skraza@cisco.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5400212D7F2; Sun, 25 Mar 2018 20:10:04 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.511
X-Spam-Level:
X-Spam-Status: No, score=-14.511 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, 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 0bzaeZ4oat1S; Sun, 25 Mar 2018 20:09:57 -0700 (PDT)
Received: from rcdn-iport-5.cisco.com (rcdn-iport-5.cisco.com [173.37.86.76]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6C6581200A0; Sun, 25 Mar 2018 20:09:57 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=338986; q=dns/txt; s=iport; t=1522033797; x=1523243397; h=from:to:cc:subject:date:message-id:mime-version; bh=Ifdis1P3y1FcZ+VJvHhLpl1XOooLfIyenMyIh6fit5k=; b=L4ggeA1mcsFd59NV7RwZixXC0sI6S2GfAj2Lp/nffh7dPks1P6LYMz1l UJqGPx1Akpf8yTH/y8a/WzCLJnth+JfOKsy+Fq91FRlKnV051Y1nrdtBx 29/HruGfq3W9dp3pm53XISRKErpsjsg+0uVhOU1Z+BXDh2Vv1Nubyiaiy c=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0CgBABYY7ha/4gNJK3AfQQCAQICxBmMBg
X-IronPort-AV: E=Sophos;i="5.48,363,1517875200"; d="scan'208,217";a="154476322"
Received: from alln-core-3.cisco.com ([173.36.13.136]) by rcdn-iport-5.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Mar 2018 03:09:56 +0000
Received: from xch-rcd-011.cisco.com (xch-rcd-011.cisco.com [173.37.102.21]) by alln-core-3.cisco.com (8.14.5/8.14.5) with ESMTP id w2Q39uv8026814 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 26 Mar 2018 03:09:56 GMT
Received: from xch-aln-013.cisco.com (173.36.7.23) by XCH-RCD-011.cisco.com (173.37.102.21) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Sun, 25 Mar 2018 22:09:54 -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.1320.000; Sun, 25 Mar 2018 22:09:54 -0500
From: "Kamran Raza (skraza)" <skraza@cisco.com>
To: "Acee Lindem (acee)" <acee@cisco.com>, "draft-ietf-mpls-mldp-yang@ietf.org" <draft-ietf-mpls-mldp-yang@ietf.org>
CC: "mpls@ietf.org" <mpls@ietf.org>, YANG Doctors <yang-doctors@ietf.org>, "xufeng_liu@jabil.com" <xufeng_liu@jabil.com>
Thread-Topic: [mpls] YANG Doctor Early Review for "YANG Data Model for MPLS mLDP" - draft-ietf-mpls-mldp-yang-03
Thread-Index: AQHTxK/pOofvF4P0VE24O1+2wG+1Kg==
Date: Mon, 26 Mar 2018 03:09:54 +0000
Message-ID: <6819A7F2-329C-4AFB-BF6B-882822D0D8F8@cisco.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/10.a.0.180210
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.86.241.159]
Content-Type: multipart/alternative; boundary="_000_6819A7F2329C4AFBBF6B882822D0D8F8ciscocom_"
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/ydrQw4cAXsF-alxznnCkYG0YEGA>
Subject: Re: [yang-doctors] [mpls] YANG Doctor Early Review for "YANG Data Model for MPLS mLDP" - draft-ietf-mpls-mldp-yang-03
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Mar 2018 03:10:10 -0000

Thanks Acee for your review of the model.

Having pushed LDP model near WGLC, the team will now be shifting focus to mLDP model and will address your comments.

Rgds.

From: mpls <mpls-bounces@ietf.org> on behalf of "Acee Lindem (acee)" <acee@cisco.com>
Date: Wednesday, December 13, 2017 at 5:06 PM
To: "draft-ietf-mpls-mldp-yang@ietf.org" <draft-ietf-mpls-mldp-yang@ietf.org>
Cc: "mpls@ietf.org" <mpls@ietf.org>rg>, YANG Doctors <yang-doctors@ietf.org>
Subject: [mpls] YANG Doctor Early Review for "YANG Data Model for MPLS mLDP" - draft-ietf-mpls-mldp-yang-03

I have reviewed this document as part of the YANG doctors directorate's
ongoing effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of the
IETF drafts. Comments that are not addressed in last call may be included in AD reviews
during the IESG review.  Document editors and WG chairs should treat these comments
just like any other early review comments.


Document: draft-ietf-mpls-mldp-yang-03.txt
Reviewer: Acee Lindem
Review Date: December 13, 2017
Review Type: Early Review
Intended Status: Standards Track
Summary: On right track but has issues

Modules: "ietf-mpls-mldp@2017-10-19.yang" and "ietf-mpls-mldp-extended@2017-10-19.yang"

Tech Summary: While I have a good general understanding of MPLS and LDP,
I am not familiar with the details of the all the mLDP features in the
extended model.

Major Issue:

  The "Security Considerations" cannot just reference the ldp YANG considerations. The
  schema nodes that are sensitive will be different and multicast (e.g., P2MP and MP2MP)
  are suspectible to different attacks than unicast.


General Comments:

  1. There are places where it the text alludes to additional features to be covered
     in a future version of the document. If the document is to progress,
     the authors need to agree on the scope of the modules.

  2. There are no configuration and operational state XML or JSON examples as
     recommended by RFC6087BIS.

  3. Some of the YANG descriptions are rather terse and look as though they were
     added solely for pyang validation. Also, why are they all terminated with a
     period. Normally a period is reserved for complete sentences. However, I didn't
     remove these in my editorial comments.

  4. Add RFC 7431 as a normative reference since the extended mLDP model covers
     Multicast-Only FRR.


Comments on ietf-mpsl-mldp.yang:

   1. The module prologue doesn't conform to the RFC6087BIS template. For example,
      the chairs should no longer be listed.

   2. The grouping mldp-binding-label-peer-state-attributes is arbitrary. Cascading
      multiple levels of groupings that are only used in a single place just makes
      the module harder to read. In fact, the module would easier to read even
      with mldp-binding-label-state-attributes inline.

   3. Why is there a choice with only a single case under opaque-type in
      mldp-fec-event?

   3. is-self says "This is the root." I'm assuming it means the YANG server itself
      is the root.

   4. "opaque-type-lspid" is a very poor choice for an identifier. It confuses the
      reading into thinking it is a type. Use "opaque-lspid".

   5. Move the grouping mldp-fec-event inline unless you are using it elsewhere.

Minor Editorial Commments:

*** ietf-mpls-mldp.yang.orig           2017-12-12 19:09:40.000000000 -0500
--- ietf-mpls-mldp.yang                2017-12-12 19:52:08.000000000 -0500
***************
*** 82,91 ****
        description
          "p2mp or mp2mp.";
      }
      /*
       * Groupings
       */
-
      grouping mldp-capabilities {
        description
          "mLDP capabilities.";
--- 82,91 ----
        description
          "p2mp or mp2mp.";
      }
+
      /*
       * Groupings
       */
      grouping mldp-capabilities {
        description
          "mLDP capabilities.";
***************
*** 131,137 ****
      } // mldp-capabilities
      grouping mldp-fec-event {
        description
!          "A mLDP FEC event.";
        choice opaque-type {
          description
            "The type of opaque value element.";
--- 131,137 ----
      } // mldp-capabilities
      grouping mldp-fec-event {
        description
!          "mLDP FEC event.";
        choice opaque-type {
          description
            "The type of opaque value element.";
***************
*** 157,169 ****
              leaf multipoint-type {
                type multipoint-type;
                description
!                  "The type of mutipoint, p2mp or mp2mp.";
              }
            } // container opaque-type-lspid
          }
        }
      } // mldp-fec-event
-
      grouping  mldp-binding-label-peer-state-attributes {
        description
          "mLDP label binding per peer attributes.";
--- 157,168 ----
              leaf multipoint-type {
                type multipoint-type;
                description
!                  "The type of multipoint: p2mp or mp2mp.";
              }
            } // container opaque-type-lspid
          }
        }
      } // mldp-fec-event
      grouping  mldp-binding-label-peer-state-attributes {
        description
          "mLDP label binding per peer attributes.";
***************
*** 199,205 ****
          }
          type enumeration {
            enum none {
!              description "MBB is not enabled.";
            }
            enum active {
              description "This LSP is active.";
--- 198,204 ----
          }
          type enumeration {
            enum none {
!              description "Make-Before-Break (MBB) is not enabled.";
            }
            enum active {
              description "This LSP is active.";
***************
*** 329,335 ****
                  list reachability {
                    key "address interface";
                    description
!                      "A next hop for reachability to root,
                         as a RIB view.";
                    leaf address {
                      type inet:ipv4-address;
--- 328,334 ----
                  list reachability {
                    key "address interface";
                    description
!                      "A next-hop for reachability to root,
                         as a RIB view.";
                    leaf address {
                      type inet:ipv4-address;
***************
*** 348,354 ****
                          + "ldp:peer/ldp:lsr-id";
                      }
                      description
!                        "LDP peer from which this next hop can be
                         reached.";
                    }
                  }
--- 347,353 ----
                          + "ldp:peer/ldp:lsr-id";
                      }
                      description
!                        "LDP peer from which this next-hop can be
                         reached.";
                    }
                  }
***************
*** 359,365 ****
                    container opaque-type-lspid {
                      description
                        "The type of opaque value element is
!                           the generic LSP identifier";
                      reference
                        "RFC6388: Label Distribution Protocol
                         Extensions for Point-to-Multipoint and
--- 358,364 ----
                    container opaque-type-lspid {
                      description
                        "The type of opaque value element is
!                         the generic LSP identifier";
                      reference
                        "RFC6388: Label Distribution Protocol
                         Extensions for Point-to-Multipoint and
***************
*** 377,383 ****
                        leaf multipoint-type {
                          type multipoint-type;
                          description
!                            "The type of mutipoint, p2mp or mp2mp.";
                        }
                        uses mldp-binding-label-state-attributes;
                      } // fec-label
--- 376,382 ----
                        leaf multipoint-type {
                          type multipoint-type;
                          description
!                            "The type of multipoint: p2mp or mp2mp.";
                        }
                        uses mldp-binding-label-state-attributes;
                      } // fec-label
***************
*** 415,421 ****
                    leaf multipoint-type {
                      type multipoint-type;
                      description
!                        "The type of mutipoint, p2mp or mp2mp.";
                    }
                  } // fec-label
                } // opaque-type-lspid
--- 414,420 ----
                    leaf multipoint-type {
                      type multipoint-type;
                      description
!                        "The type of multipoint: p2mp or mp2mp.";
                    }
                  } // fec-label
                } // opaque-type-lspid


Comments on ietf-mpsl-mldp-extended.yang:

   1. The module prologue doesn't conform to the RFC6087BIS template. For example,
      the chairs should no longer be listed.

   2. The module description is a copy of the base mLDP. It should refer that the
      module contains extended functions.

   3. There is a type for route-distinguisher in ietf-routing-types.

   4. The lsa-key-type is very confusing since the root-address is actually the
      key for the grouping.

   5. All these containers that start "opaque-type..." would be less confusing if
      they just read "opaque..". These are NOT YANG type or any other type of
      type.

   6. For FEC-label in opaque-type-transit, do you really need every node to be
      part of the key?


Editorial Nits:

*** ietf-mpls-mldp-extended.yang.orig   2017-12-12 20:33:09.000000000 -0500
--- ietf-mpls-mldp-extended.yang        2017-12-12 21:14:14.000000000 -0500
***************
*** 101,108 ****
       * Typedefs
       */
      typedef route-distinguisher {
!        type string {
!        }
        description
          "Type definition for route distinguisher.";
        reference
--- 101,107 ----
       * Typedefs
       */
      typedef route-distinguisher {
!        type string;
        description
          "Type definition for route distinguisher.";
        reference
***************
*** 175,186 ****
          leaf plr {
            type boolean;
            description
!              "Point of Local Repair capable for MP LSP node
!               protection.";
          }
          container merge-point {
            description
!              "Merge Point capable for MP LSP node protection.";
            leaf enable {
              type boolean;
              description
--- 174,185 ----
          leaf plr {
            type boolean;
            description
!              "Point of Local Repair capable for Multipoint LSP
!               node protection.";
          }
          container merge-point {
            description
!              "Merge Point capable for Multipoint LSP node protection.";
            leaf enable {
              type boolean;
              description
***************
*** 271,277 ****
          leaf prefix-list {
            type ldp-ext:prefix-list-ref;
            description
!              "Enables MoFRR for the specified access list.";
          }
        } // multicast-only-frr
        container recursive-fec {
--- 270,276 ----
          leaf prefix-list {
            type ldp-ext:prefix-list-ref;
            description
!              "Enables Multicast-Only FRR (MoFRR) for the specified
!               prefix list.";
          }
        } // multicast-only-frr
        container recursive-fec {
***************
*** 280,288 ****
          leaf prefix-list {
            type ldp-ext:prefix-list-ref;
            description
!              "Enables recursive FEC for the specified access list.";
          }
!        } // recursive-for
      } // mldp-ext-per-af-config-attibutes

      grouping recursive-fec-attibutes {
--- 279,287 ----
          leaf prefix-list {
            type ldp-ext:prefix-list-ref;
            description
!              "Enables recursive FEC for the specified prefix list.";
          }
!        } // recursive-fec
      } // mldp-ext-per-af-config-attibutes

      grouping recursive-fec-attibutes {
***************
*** 308,314 ****
        leaf multipoint-type {
          type mldp:multipoint-type;
          description
!            "The type of mutipoint, p2mp or mp2mp.";
        }
      } // recursive-fec-attibutes

--- 307,313 ----
        leaf multipoint-type {
          type mldp:multipoint-type;
          description
!            "The type of multipoint: p2mp or mp2mp.";
        }
      } // recursive-fec-attibutes

***************
*** 377,384 ****
             Multipoint-to-Multipoint Label Switched Paths.";
          list fec-label {
            key
!              "root-address source-address group-address "
!            + "rd recur-root-address recur-rd";
            description
              "List of FEC to label bindings.";
            leaf root-address {
--- 376,383 ----
             Multipoint-to-Multipoint Label Switched Paths.";
          list fec-label {
            key
!              "root-address source-address group-address " +
!              "rd recur-root-address recur-rd";
            description
              "List of FEC to label bindings.";
            leaf root-address {
***************
*** 433,439 ****
            leaf rp {
              type inet:ip-address;
              description
!                "RP address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;
--- 432,438 ----
            leaf rp {
              type inet:ip-address;
              description
!                "Rendezvous-Point address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;
***************
*** 472,478 ****
        leaf mldp-disable {
          type boolean;
          description
!            "Disable mLDP forwarding on the interface.";
        }
      }

--- 471,477 ----
        leaf mldp-disable {
          type boolean;
          description
!            "Disable mLDP forwarding on this interface.";
        }
      }

***************
*** 523,535 ****
          leaf plr {
            type boolean;
            description
!              "Point of Local Repair capable for MP LSP node
               protection.";
          }
          leaf merge-point {
            type boolean;
            description
!              "Merge Point capable for MP LSP node protection.";
          } // merge-point
        } // node-protection
      }
--- 522,534 ----
          leaf plr {
            type boolean;
            description
!              "Point-of-Local-Repair capable for Multipoint LSP node
               protection.";
          }
          leaf merge-point {
            type boolean;
            description
!              "Merge Point capable for Multipoint LSP node protection.";
          } // merge-point
        } // node-protection
      }
***************
*** 682,688 ****
            leaf rp {
              type inet:ip-address;
              description
!                "RP address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;
--- 681,687 ----
            leaf rp {
              type inet:ip-address;
              description
!                "Rendezvous Point (RP) address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;
***************
*** 773,784 ****
              list reachability {
                key "address interface";
                description
!                  "A next hop for reachability to root,
                   as a RIB view.";
                leaf address {
                  type inet:ipv6-address;
                  description
!                    "The next hop address to reach root.";
                }
                leaf interface {
                  type if:interface-ref;
--- 772,783 ----
              list reachability {
                key "address interface";
                description
!                  "A next-hop for reachability to root,
                   as a RIB view.";
                leaf address {
                  type inet:ipv6-address;
                  description
!                    "The next-hop address to reach root.";
                }
                leaf interface {
                  type if:interface-ref;
***************
*** 792,798 ****
                      + "ldp:peer/ldp:lsr-id";
                  }
                  description
!                    "LDP peer from which this next hop can be
                     reached.";
                }
              }
--- 791,797 ----
                      + "ldp:peer/ldp:lsr-id";
                  }
                  description
!                    "LDP peer from which this next-hop can be
                     reached.";
                }
              }
***************
*** 821,827 ****
                    leaf multipoint-type {
                      type mldp:multipoint-type;
                      description
!                        "The type of mutipoint, p2mp or mp2mp.";
                    }

                    uses mldp-ext-binding-label-state-attributes;
--- 820,826 ----
                    leaf multipoint-type {
                      type mldp:multipoint-type;
                      description
!                        "The type of multipoint: p2mp or mp2mp.";
                    }

                    uses mldp-ext-binding-label-state-attributes;
***************
*** 861,867 ****
                leaf multipoint-type {
                  type mldp:multipoint-type;
                  description
!                    "The type of mutipoint, p2mp or mp2mp.";
                }
                list recursive-fec {
                  key
--- 860,866 ----
                leaf multipoint-type {
                  type mldp:multipoint-type;
                  description
!                    "The type of multipoint:p2mp or mp2mp.";
                }
                list recursive-fec {
                  key
***************
*** 1040,1046 ****
            leaf rp {
              type inet:ip-address;
              description
!                "RP address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;
--- 1039,1045 ----
            leaf rp {
              type inet:ip-address;
              description
!                "Rendezvous-Point (RP) address.";
            }
            leaf group-address {
              type inet:ip-address-no-zone;


Editorial comments on the draft text are included below. There seems to be an absence of articles...

*** draft-ietf-mpls-mldp-yang-03.txt.orig         2017-12-12 18:17:12.000000000 -0500
--- draft-ietf-mpls-mldp-yang-03.txt              2017-12-13 11:47:06.000000000 -0500
***************
*** 120,132 ****

    This document introduces a YANG data model for MPLS Multipoint Label
    Distribution Protocol (mLDP).  The mLDP model being defined here is
!    dependent on LDP YANG data model [I-D.ietf-mpls-ldp-yang].  This
!    implies that an opertor will need to use base LDP module to configure
!    and manage control plane for mLDP.  For example, an operator would
    enable LDP discovery on MPLS interface to establish LDP/mLDP peering
    on which mLDP bindings could be exchanged.  Similarly, an operator
    could query state information for an LDP peer in order to verify
!    peering attributes etc.

    Moreover, it is important to note here that any assumptions made in
    the LDP model also hold true in this document, unless otherwise
--- 120,132 ----

    This document introduces a YANG data model for MPLS Multipoint Label
    Distribution Protocol (mLDP).  The mLDP model being defined here is
!    dependent on the LDP YANG data model [I-D.ietf-mpls-ldp-yang].  This
!    implies that an opertor will need to use the base LDP module to configure
!    and manage the control plane for mLDP.  For example, an operator would
    enable LDP discovery on MPLS interface to establish LDP/mLDP peering
    on which mLDP bindings could be exchanged.  Similarly, an operator
    could query state information for an LDP peer in order to verify
!    peering attributes, etc.

    Moreover, it is important to note here that any assumptions made in
    the LDP model also hold true in this document, unless otherwise
***************
*** 157,164 ****

 1.1.  Base and Extended

!    Like LDP model, the configuration and state items are divided into
!    following two broad categories:



--- 157,164 ----

 1.1.  Base and Extended

!    Like the LDP model, the configuration and state items are divided into
!    the following two broad categories:



***************
*** 173,197 ****
    o  Extended

    The "base" category contains the basic and fundamental features that
!    are covered in mLDP base specification [RFC6388] alongwith few
    significant extension like targeted mLDP [RFC7060], constituting the
    minumum requirements for an mLDP deployment.  Whereas, the "extended"
    category contains all other non-base features (such as recursive FEC
!    support, protection etc.).  All the items in a base category are
    mandatory and hence no "if-feature" is allowed under the "base"
    category.  While "base" model support will suffice for small
    deployments, large deployments will require not only the "base"
    module support but also "extended" support for some selected and
    required features.

!    The base and extended catogories are defined in their own modules
    ietf-mpls-mldp and ietf-mpls-mldp-extended respectively, each of
!    which augments the LDP base model as defined under ietf-mpls-ldp
    module [I-D.ietf-mpls-ldp-yang].

!    Like LDP, mLDP "base" model configuration and state covers ipv4
    address-family only, with ipv6 address-family related configuration
!    and state be covered in "extended" model.

 2.  Specification of Requirements

--- 173,197 ----
    o  Extended

    The "base" category contains the basic and fundamental features that
!    are covered in the mLDP base specification [RFC6388] along with a few
    significant extension like targeted mLDP [RFC7060], constituting the
    minumum requirements for an mLDP deployment.  Whereas, the "extended"
    category contains all other non-base features (such as recursive FEC
!    support, protection, etc.).  All the items in the base category are
    mandatory and hence no "if-feature" is allowed under the "base"
    category.  While "base" model support will suffice for small
    deployments, large deployments will require not only the "base"
    module support but also "extended" support for some selected and
    required features.

!    The base and extended catogories are defined in their own modules,
    ietf-mpls-mldp and ietf-mpls-mldp-extended respectively, each of
!    which augments the LDP base model as defined within the ietf-mpls-ldp
    module [I-D.ietf-mpls-ldp-yang].

!    Like LDP, the mLDP "base" model configuration and state covers the ipv4
    address-family only, with ipv6 address-family related configuration
!    and state covered in the "extended" model.

 2.  Specification of Requirements

***************
*** 201,213 ****

 3.  Overview

!    This document defines a new module named "ietf-mpls-mldp" for mLDP
    YANG base data model that augments /rt:routing/rt:control-plane-
    protocols/ldp:mpls-ldp defined in [I-D.ietf-mpls-ldp-yang].  The
!    document also defines "ietf-mpls-mldp-extended" module that models
!    the extended mLDP features under YANG.

!    Following diagram depicts high level mLDP yang tree organization and
    hierarchy with respect to LDP:


--- 201,213 ----

 3.  Overview

!    This document defines a YANG module named "ietf-mpls-mldp" for the mLDP
    YANG base data model that augments /rt:routing/rt:control-plane-
    protocols/ldp:mpls-ldp defined in [I-D.ietf-mpls-ldp-yang].  The
!    document also defines the "ietf-mpls-mldp-extended" YANG module that
!    models the extended mLDP features.

!    The following diagram depicts high-level mLDP yang tree organization and
    hierarchy with respect to LDP:


***************
*** 255,262 ****

 3.1.  Scope

!    Following are the main mLDP areas and features that are within the
!    scope of this model:

    o  Base:

--- 255,262 ----

 3.1.  Scope

!    The main mLDP areas and features that are within the scope of this
!    model are as follows:

    o  Base:

***************
*** 282,288 ****

          +  Node Protection [RFC7715]

!          +  Multicast-only

       *  In-band Signaling:

--- 282,288 ----

          +  Node Protection [RFC7715]

!          +  Multicast-only [RFC7431]

       *  In-band Signaling:

***************
*** 300,306 ****
 3.2.  FEC Types

    The FEC for Multipoint LSP is presented as (root-address, opaque-
!    type).  The following is the table for various type of MP opaque
    values with their keys, as covered in the configuration and state
    model:

--- 300,306 ----
 3.2.  FEC Types

    The FEC for Multipoint LSP is presented as (root-address, opaque-
!    type).  The following table lists various types of MP opaque
    values with their keys, as covered in the configuration and state
    model:

***************
*** 322,328 ****

                      Table 1: MP Opaque Types and keys

!    It is to be noted that there are three basic types (LSP Id, Source,
    and Bidir) and then there are variants (VPN, recursive, VPN-
    recursive) on top of these basic types.

--- 322,328 ----

                      Table 1: MP Opaque Types and keys

!    It should be noted that there are three basic types (LSP Id, Source,
    and Bidir) and then there are variants (VPN, recursive, VPN-
    recursive) on top of these basic types.

***************
*** 344,351 ****

 4.1.  Configuration Hierarchy

!    Following is the high-level configuration organization for base and
!    extended mLDP:


         augment /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:
--- 344,351 ----

 4.1.  Configuration Hierarchy

!    The high-level configuration organization for the base and
!    extended mLDP follows:


         augment /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:
***************
*** 399,406 ****

    o  Parameters that leverage/extend LDP containers and parameters

!    Following subsections first describe mLDP specific configuration
!    parameters, followed by those leveraging LDP.  It is to be noted that
    these parameters are defined under their respective base or extended
    module as per their categorization.

--- 399,406 ----

    o  Parameters that leverage/extend LDP containers and parameters

!    The following subsections first describe mLDP specific configuration
!    parameters, followed by those leveraging LDP.  It should be noted that
    these parameters are defined under their respective base or extended
    module as per their categorization.

***************
*** 410,421 ****
    the configuration related to items that are mLDP specific.  The main
    items under this container are:

!    o  mLDP enabling: To enable mLDP under a (VRF) routing instance, mldp
!       container is enabled under LDP.  Given that mLDP requires LDP
!       signalling, it is not sensible to allow disabling LDP control
       plane under a (VRF) network-instance while requiring mLDP to be
!       enabled for the same.  However, if a user wants only to allow
!       signalling for multipoint FECs on an LDP/mLDP enabled VRF
       instance, he/she can use LDP label-policies to disable unicast
       FECs under the VRF.

--- 410,421 ----
    the configuration related to items that are mLDP specific.  The main
    items under this container are:

!    o  mLDP enablement: To enable mLDP under a (VRF) routing instance, mldp
!       is enabled in the mldp container under LDP.  Given that mLDP requires LDP
!       signaling, it is not sensible to allow disabling the LDP control
       plane under a (VRF) network-instance while requiring mLDP to be
!       enabled for the same.  However, if a user wants to only allow
!       signaling for multipoint FECs on an LDP/mLDP enabled VRF
       instance, he/she can use LDP label-policies to disable unicast
       FECs under the VRF.

***************
*** 429,438 ****
          of the feature.

       *  Recursive FEC: The recursive-fec feature [RFC6512] can be
!          enabled per AF with a route-policy.

!       *  Configured Leaf LSPs: To provision multipoint leaf LSP
!          manually, a container is provided per-AF under LDP.  The
          configuration is flexible and allows a user to specify MP LSPs
          of type p2mp or mp2mp with IPv4 or IPv6 root address(es) by
          using either LSP-Id or (S,G).
--- 429,438 ----
          of the feature.

       *  Recursive FEC: The recursive-fec feature [RFC6512] can be
!          enabled per-AF with a route-policy.

!       *  Configured Leaf LSPs: To provision multipoint leaf LSPs
!          manually, a per-AF container is provided under LDP.  The
          configuration is flexible and allows a user to specify MP LSPs
          of type p2mp or mp2mp with IPv4 or IPv6 root address(es) by
          using either LSP-Id or (S,G).
***************
*** 450,478 ****

 4.3.  Leveraging LDP containers

!    mLDP configuration model leverages following configuration areas and
    containers that are already defined for LDP:

    o  Capabilities: A new container "mldp" is defined that augments
       LDP's capabilities container.  This new container specifies any
       mLDP specific capabilities and their parameters.  Moreover, a new
!       "mldp" container is also added by augmenting LDP per-peer
       capability container to override/control mLDP specific
       capabilities on a peer level.  In the scope of this document, the
       most important capabilities related to mLDP are p2mp, mp2mp, make-
       before-break, hub-and-spoke, and node-protection.

!    o  Discovery and Peer: mLDP requires LDP discovery and peer
!       procedures to form mLDP peering.  A peer is treated as mLDP peer
       only when either P2MP or MP2MP capabilities have been successfully
       exchanged with the peer.  If a user wish to selectively enable or
       disable mLDP with a LDP-enabled peer, he/she may use per-peer mLDP
       capabilities configuration.  [Ed Note: The option to control mLDP
!       enabling/disabling on a peer-list is being explored for future ].
       In most common deployments, it is desirable to disable mLDP
       (capabilities announcements) on a targeted-only LDP peering, where
!       targeted-only peer is the one whose discovery sources are targeted
!       type only.  In future revision, a configuration option for this
       support will also be provided.

    o  Forwarding: By default, mLDP is allowed to select any of the LDP
--- 450,478 ----

 4.3.  Leveraging LDP containers

!    The mLDP configuration model leverages following configuration areas and
    containers that are already defined for LDP:

    o  Capabilities: A new container "mldp" is defined that augments
       LDP's capabilities container.  This new container specifies any
       mLDP specific capabilities and their parameters.  Moreover, a new
!       "mldp" container is also added by augmenting the LDP per-peer
       capability container to override/control mLDP specific
       capabilities on a peer level.  In the scope of this document, the
       most important capabilities related to mLDP are p2mp, mp2mp, make-
       before-break, hub-and-spoke, and node-protection.

!    o  Discovery and Peering: mLDP requires LDP discovery and peer
!       procedures to form mLDP peering.  A peer is treated as an mLDP peer
       only when either P2MP or MP2MP capabilities have been successfully
       exchanged with the peer.  If a user wish to selectively enable or
       disable mLDP with a LDP-enabled peer, he/she may use per-peer mLDP
       capabilities configuration.  [Ed Note: The option to control mLDP
!       enabling/disabling on a peer-list is being explored for future].
       In most common deployments, it is desirable to disable mLDP
       (capabilities announcements) on a targeted-only LDP peering, where
!       targeted-only peer is the one whose discovery sources are the targeted
!       type only.  In a future revision, a configuration option for this
       support will also be provided.

    o  Forwarding: By default, mLDP is allowed to select any of the LDP
***************
*** 480,494 ****
       (LDP/mLDP peer) for MP LSP programming.  However, a configuration
       option is provided to allow mLDP to exclude a given interface from
       such a selection.  Note that such a configuration option will be
!       useful only when there are more than one interfaces available for
       the downstream selection.

 4.4.  Configuration Tree

 4.4.1.  Base

!    Following is a simplified graphical representation of the data model
!    for mLDP base configuration



--- 480,494 ----
       (LDP/mLDP peer) for MP LSP programming.  However, a configuration
       option is provided to allow mLDP to exclude a given interface from
       such a selection.  Note that such a configuration option will be
!       useful only when there are more than one interface available for
       the downstream selection.

 4.4.  Configuration Tree

 4.4.1.  Base

!    A simplified graphical representation of the data model
!    for mLDP base configuration follows:



***************
*** 533,540 ****

 4.4.2.  Extended

!    Following is a simplified graphical representation of the data model
!    for mLDP extended configuration



--- 533,540 ----

 4.4.2.  Extended

!    A simplified graphical representation of the data model for mLDP extended
!    configuration follows:



***************
*** 647,659 ****

 5.  Operational State

!    Operational state of mLDP can be queried and obtained from various
    read-only mdlp "state" containers that augment ldp containers.

 5.1.  Base

!    Following is a simplified graphical representation of the data model
!    for mLDP base operational state:



--- 647,659 ----

 5.  Operational State

!    The operational state for mLDP can be queried and obtained from various
    read-only mdlp "state" containers that augment ldp containers.

 5.1.  Base

!    A simplified graphical representation of the data model
!    for mLDP base operational state follows:



***************
*** 713,720 ****

 5.2.  Extended

!    Following is a simplified graphical representation of the data model
!    for mLDP extended operational state:


 module: ietf-mpls-mldp
--- 713,720 ----

 5.2.  Extended

!    A simplified graphical representation of the data model
!    for mLDP extended operational state follows:


 module: ietf-mpls-mldp
***************
*** 871,878 ****

 5.3.  Derived states

!    Following are main areas for which mLDP operational derived state is
!    defined:

    o  Root

--- 871,878 ----

 5.3.  Derived states

!    The main areas for which mLDP operational derived state is
!    defined are:

    o  Root

***************
*** 882,892 ****

 5.3.1.  Root state

!    Root address is a fundamental construct for MP FEC bindings and LSPs.
    The root state provides information on all the known roots in a given
!    address-familty, and their information on the root reachability (as
    learnt from RIB).  In case of multi-path reachability to a root, the
!    selection of upstream path is done on per-LSP basis at the time of
    LSP setup.  Similarly, when protection mechanisms like MBB or MoFRR


--- 882,892 ----

 5.3.1.  Root state

!    The root address is a fundamental construct for MP FEC bindings and LSPs.
    The root state provides information on all the known roots in a given
!    address-family, and their root reachability information (as
    learnt from RIB).  In case of multi-path reachability to a root, the
!    selection of the upstream path is done on per-LSP basis at the time of
    LSP setup.  Similarly, when protection mechanisms like MBB or MoFRR


***************
*** 897,906 ****


    are in place, the path designation as active/standby or primary/
!    backup is also done on per LSP basis.  It is to be noted that a given
    root can be shared amongst multiple P2MP and/or MP2MP LSPs.
!    Moreover, an LSP can be signaled to more than one root for RNR
!    purposes.

    The following diagram illustrates a root database on a branch/transit
    LSR:
--- 897,906 ----


    are in place, the path designation as active/standby or primary/
!    backup is also done on per-LSP basis.  It is to be noted that a given
    root can be shared amongst multiple P2MP and/or MP2MP LSPs.
!    Moreover, an LSP can be signaled to more than one root for Root Node
!    Redundancy (RNR) purposes.

    The following diagram illustrates a root database on a branch/transit
    LSR:
***************
*** 936,942 ****
 5.3.2.  Bindings state

    Binding state provides information on mLDP FEC-label bindings for
!    both P2MP and MP2MP FEC types.  Like LDP, the FEC-label binding
    derived state is presented in a FEC-centric view per address-family,
    and provides information on both inbound (received) and outbound
    (advertised) bindings.  The FEC is presented as (root-address,
--- 936,942 ----
 5.3.2.  Bindings state

    Binding state provides information on mLDP FEC-label bindings for
!    both the P2MP and MP2MP FEC types.  Like LDP, the FEC-label binding
    derived state is presented in a FEC-centric view per address-family,
    and provides information on both inbound (received) and outbound
    (advertised) bindings.  The FEC is presented as (root-address,
***************
*** 955,962 ****
    binding is also provided with respect to MBB (active or standby) or/
    and MoFRR (primary or backup).

!    Following captures a high level tree hierarchy for mLDP bindings
!    state:


 +--rw mpls-ldp!
--- 955,961 ----
    binding is also provided with respect to MBB (active or standby) or/
    and MoFRR (primary or backup).

!    A high-level tree hierarchy for mLDP bindings state follows:


 +--rw mpls-ldp!
***************
*** 990,1003 ****
                                  Figure 9

    mLDP binding state is organized and presented per root address, and
!    hence the bindings container hang off a root node in the model.  The
    bindings state is made available for FECs pertaining to different
!    types of opaque types, with some state avaiable under "base" tree and
!    the rest under "extended".

!    In the above tree, the various opaque types alongwith their type
!    specific key(s) refer to the table Table 1 captured earlier in the
!    document.  For example, if the opaque type is Generic LSP Identifier,



--- 989,1002 ----
                                  Figure 9

    mLDP binding state is organized and presented per root address, and
!    hence the bindings container in under a root node in the model.  The
    bindings state is made available for FECs pertaining to different
!    types of opaque types, with some state avaiable under the "base" tree and
!    the rest under the "extended" tree.

!    In the above tree, the various opaque types along with their type
!    specific key(s) refer to the table Table 1, as captured earlier in the
!    document.  For example, if the opaque type is a Generic LSP Identifier,



***************
*** 1010,1028 ****
    then the type-specific-key will be a uint32 LSP-Id key.  Please see
    the complete model for all other types.

!    It is important to take note of the following:

!    o  The address-family ipv4/ipv4 applies to "root" address in the mLDP
!       binding tree.  The other addresses (source, group, RP etc) do not
       have to be of the same address family type as the root.

!    o  The "recur-root-address" field applies to Recursive opaque type,
!       and (recur-root-address, recur-rd) fields applies to VPN-Recursive
!       opaque types as defined in [RFC6512]

    o  In case of a recursive FEC, the address-family of the recur-root-
       address could be different than the address-family of the root
!       address of original encapsulated MP FEC

    The following diagram illustrates the FEC-label binding information
    structure for a P2MP (Transit IPv4 Source type) LSP on a branch/
--- 1009,1027 ----
    then the type-specific-key will be a uint32 LSP-Id key.  Please see
    the complete model for all other types.

!    It is important to note the following:

!    o  The address-family ipv4/ipv4 applies to "root" addresses in the mLDP
!       binding tree.  The other addresses (source, group, RP, etc.) do not
       have to be of the same address family type as the root.

!    o  The "recur-root-address" field applies to the Recursive opaque type,
!       and the (recur-root-address, recur-rd) fields apply to the VPN-Recursive
!       opaque types as defined in [RFC6512].

    o  In case of a recursive FEC, the address-family of the recur-root-
       address could be different than the address-family of the root
!       address of the original encapsulated MP FEC.

    The following diagram illustrates the FEC-label binding information
    structure for a P2MP (Transit IPv4 Source type) LSP on a branch/
***************
*** 1091,1103 ****

 6.  Notifications

!    mLDP notification module consists of notification related to changes
    in the operational state of an mLDP FEC.

 6.1.  Base

!    Following is a simplified graphical representation of the base data
!    model for mLDP notifications:



--- 1090,1102 ----

 6.  Notifications

!    The mLDP notification module consists of notifications related to changes
    in the operational state of an mLDP FEC.

 6.1.  Base

!    A simplified graphical representation of the base data
!    model for mLDP notifications follows:



***************
*** 1139,1146 ****

 6.2.  Extended

!    Following is a simplified graphical representation of the extended
!    data model for mLDP notifications:



--- 1138,1145 ----

 6.2.  Extended

!    A simplified graphical representation of the extended
!    data model for mLDP notifications follows:



***************
*** 1208,1215 ****

 8.  Open Items

!    Following is a list of open items that are to be discussed and
!    addressed in future revisions of this document:

    o  Specify default values for configuration parameters

--- 1207,1214 ----

 8.  Open Items

!    A list of open items that are to be discussed and
!    addressed in future revisions of this document follows:

    o  Specify default values for configuration parameters

***************
*** 1233,1240 ****

 9.  YANG Specification

!    Following is the actual YANG definition (module) for mLDP constructs
!    defined earlier in the document.

 9.1.  Base

--- 1232,1239 ----

 9.  YANG Specification

!    The YANG definition, i.e., the modules, for mLDP constructs defined earlier
!    in this document are includind the subsections below.

 9.1.  Base


Thanks,
Acee