[yang-doctors] Yangdoctors early review of draft-ietf-rtgwg-policy-model-09

[Mahesh Jethanandani via Datatracker <[e-mail]>]

Reviewer: Mahesh Jethanandani
Review result: On the Right Track

This review is looking at the draft from a YANG perspective. With that said, I
have marked it as on the right track, because of some of the points discussed
below.

Summary:

This document defines a YANG data model for configuring and managing routing
policies in a vendor-neutral way.

The description in the document and in the model is well written and easy to
understand.

Nits

1. Please run the spell checker on the document.
2. Indentations in the YANG model are not consistent. There is at least one
place in the definition of 'grouping neighbor-set' that needs fixing. 3.
Consistency in how the description statements are written. Most of the
descriptions start on a new line, except for a few places where they do not. 4.
The match-set-options-type defines enums as 'all', 'any' and 'invert'. Then why
use ALL, ANY (all caps) when describing them? Could you use single quotes (as
above) around them when referring to the definition? 5. Remove unnecessary
empty lines in the model.

Comments:

Section 1 - Introduction:

The document does not mention whether the model is YANG 1.0 or a 1.1 model. It
includes both RFC 6020 and RFC 7950 even though the actual YANG model clearly
states that the yang-version is 1.1. Please remove reference to RFC 6020.

The document includes a section on Terminology and Notation. However it does
not define what is a "routing policy" or provide a reference to it.

Section 8 - Security Considerations:

The security considerations section needs to follow the guidelines outlined in
Section 3.7 of RFC 8407.

Section 9 - IANA Considerations

The IANA considerations section needs a complete rework. Please see Section 3.8
of RFC 8407 or see any of the many drafts/RFCs that have samples of what needs
to go into that section.

Section 10.1 - Routing policy model

The model uses a lot of groupings, most of them used only once in the model.
That makes the model difficult to understand. Unless these groupings are meant
for use by other models, they should be folded into the main container. Take
this grouping for example:

     grouping policy-definitions {
       description
         "This grouping provides policy definitions";

       leaf name {
         type string;
         description
           "Name of the top-level policy definition -- this name
           is used in references to the current policy";
       }
     }

For one the description of the grouping is not correct. Secondly, it is used
only once in the model and that too to define a key of a list.

The YANG model imports multiple models from other drafts. It provides normative
references to the imported models in Section 2.2. (A nit here is that it
missing the cross-reference in the table for if-l3-vlan), but does not provide
references to them in the model. See Section 3.9 of RFC 8407. Please add
something like this in the model.

import ietf-inet-types {
  prefix "inet";
  reference
    "RFC 6991: Common YANG Data Types.";
}

Please drop <mailto:....> and just keep the e-mail address. That tag works only
when embedded within a HTML document.

The description section splits the description of the model by inserting the
Copyright statement in the middle. Can that statement be moved so that the
actual description of the model is together.

The model defines a lot of identities some of which are not used in the model
or used at all. Would it not be better to have those identities defined in the
individual routing models that augment this model? The BGP YANG model does not
use any of the identities defined in this model.

Can the following comment be made part of the description statement of the
typedef?

       /* This typedef retained for name compatibiity with default
          import and export policy. */

s/compatibiity/compatibility/

I believe masklength is two words. s/masklength-lower/mask-length-lower/ and
the same for masklength-upper. Also update the description.

A pyang compilation of the model with —ietf and —lint option was clean.
However, there seems to be an issue with the example in Section 11. Instead of
actually including an example, it is showing the path to what should have been
the example. I was therefore not able to validate the example against the model.

A idnits run of the draft reveals a few issues. Please address them.

   Checking nits according to https://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------

  == There are 4 instances of lines with private range IPv4 addresses in the
     document.  If these are generic example addresses, they should be changed
     to use any of the ranges defined in RFC 6890 (or successor): 192.0.2.x,
     198.51.100.x or 203.0.113.x.

  == The document doesn't use any RFC 2119 keywords, yet seems to have RFC
     2119 boilerplate text.

  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------

     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)

  == Unused Reference: 'I-D.ietf-netmod-sub-intf-vlan-model' is defined on
     line 1620, but no explicit reference was found in the text

  -- No information found for draft-ietf-netmod-intf-ext-yang - is the name
     correct?

  -- Possible downref: Normative reference to a draft: ref.
     'I-D.ietf-netmod-intf-ext-yang'

  -- No information found for draft-ietf-netmod-sub-intf-vlan-model - is the
     name correct?

  -- Possible downref: Normative reference to a draft: ref.
     'I-D.ietf-netmod-sub-intf-vlan-model'

     Summary: 0 errors (**), 0 flaws (~~), 9 warnings (==), 5 comments (--).