[pim] Rtgdir last call review of draft-ietf-pim-msdp-yang-08

[Yingzhen Qu via Datatracker <[e-mail]>]

Reviewer: Yingzhen Qu
Review result: Has Issues

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.