I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.

Document: draft-ietf-pim-msdp-yang
Reviewer: Yingzhen Qu
Review Date: Jan 20th, 2020
Intended Status: Standards Track

Summary:

This document is near ready for publication. It has some issues that should be at least considered prior to publication.

Comments:

Thanks for working on this draft. As an active YANG contributor I appreciate the work here.

Major issues:

The tree in the draft needs to be updated to match the model. 

In the grouping definition of “global-config-attributes”:
      leaf prefix-policy {
        type string;
        description
          "If specified, only those SA entries whose RP is
           permitted in the prefix list are allowed;
           if not specified, all SA messages from the default
           peer are accepted.
           The according policy model is defined in
           'ietf-rtgwg-policy-model'.";
      }
It seems that this leaf is referencing what’s defined in the routing policy model, hence the type should be a leafref instead of string. I'd suggest to use ACL YANG model as defined in RFC8519 instead of routing policy model.

Same issue as above for “sa-filter” leaf “in” and “out”.
    container sa-filter {
      description
        "Specifies an access control list (ACL) to filter source
         active (SA) messages coming in to or going out of the
         peer.";
      leaf in {
        type string;
        description
          "Filters incoming SA messages only.
           The string value is the name to uniquely identify a
           policy that contains one or more policy rules used to
           accept or reject MSDP SA messages.
           If a policy is not specified, all MSDP SA messages are
           accepted, the definition of such a policy is outside
           the scope of this document.
           The according policy model is defined in
           'ietf-rtgwg-policy-model'.";
      }
      leaf out {
        type string;
        description
          "Filters outgoing SA messages only.
           The string value is the name to uniquely identify a
           policy that contains one or more policy rules used to
           accept or reject MSDP SA messages.
           If a policy is not specified, all MSDP SA messages are
           accepted, the definition of such a policy is outside
           the scope of this document.
           The according policy model is defined in
           'ietf-rtgwg-policy-model'.";
      }
    } // sa-filter



Minor Issues:

Section 5 Security Considerations
It should be the “key” field which is sensitive. 

Section 6 IANA Considerations:
   The IANA is requested to assign two new URIs from the IETF XML
   registry [RFC3688].  Authors are suggesting the following URI
It should be one URI requested instead of two.


Nits for your consideration:

In the module:
The copyright should be changed 2020. 


      leaf connect-retry-interval {
        type uint16;
        units seconds;
        default 30;
        description "Peer timer for connect-retry,
                     SHOULD be set to 30 seconds.";
      }
The description needs to be fixed. By default, MSDP peers wait 30 seconds after session is reset.