Subject: Yangdoctors last call review of draft-ietf-rtgwg-policy-model-29

From: Mahesh Jethanandani via Datatracker <[email-masked]>
Date: Sun, 18 Jul 2021 21:07:28 -0700

Reviewer: Mahesh Jethanandani
Review result: Almost Ready

This review is looking at the draft from a YANG perspective. With that said, I
have marked it as almost ready, 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 model provides a generic routing policy
framework which can be extended for specific routing protocols using the YANG
'augment' mechanism.

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

Nits

- Repeat of parent as a prefix. It is not necessary to repeat the parent name
in child attributes, e.g. routing-policy -> policy-definitions ->
policy-definition. This can be shortened to routing-policy -> definitions >
definition.

s/domian/domain/
s/suspectable/susceptible/

- Consistency in how the reference statements are written. Most of the
reference statements start on a new line, except for a few places where they do
not.

Comments:

Section 1 - Introduction:

The document does not mention whether the model is YANG a 1.1 model. It
includes RFC 7950 which would imply a 1.1 module, and the YANG model has a
yang-version is 1.1., but it would be nice to state it explicitly.

Section 7.2

- Consider moving identity 'metric-type' and 'route-level' and their derived
identities into an IANA maintained module, e.g. 'iana-policy-types', so that
module can be updated separately from the rest of the module (much more easily).

- The leaf 'mode' is defined as an enumeration with enum values of ipv4 and
ipv6. The description however says:

              "Indicates the mode of the prefix set, in terms of
               which address families (IPv4, IPv6, or both) are
               present."

How does a user indicate both?

The model uses a lot of groupings, most of them used only once in the model. It
does identify that prefix sets, neighbor sets and tag sets as reusable
groupings. Is that the case for the rest of the groupings? Unless these
groupings are meant for use by other models, they should be folded into the
main container.

Please drop <mailto:....> and just keep the e-mail address. That tag works only
when embedded within a HTML document. (This is a leftover item from the early
review, and if there was a discussion about it already, just ignore it).

Section 8 - Security Considerations:

The security considerations section lists /routing-policy as one of the nodes
as being sensitive from a write operation perspective. That would imply the
whole module is sensitive. It however, goes onto identifying specific nodes
within the module. Not clear if the whole module was intended to be identified
or specific nodes.

Similarly a sub-tree of the module is identified in
/routing-policy/policy-definitions.

If the idea of having a node without a description is to identify
(sub-)sections of the module where the nodes occur (the path already indicates
so), some words to that effect might help. E.g. In the
/routing-policy/policy-definitions section of the module the following nodes
are considered vulnerable.

The last paragraph is a fairly generic, and seems to repeat what is already
identified above. Moreover, it is not clear what is meant by "related model
carries potential risk". What is "related model"?

General

A pyang compilation of the model with —ietf and —lint option was clean. A
validation of the model and the example in Appendix B also succeeded. Thank you
for providing an example.

An idnits run of the draft was generally clean.

---

From: Yingzhen Qu <[email-masked]>
Date: Thu, 29 Jul 2021 22:18:10 -0700

Hi Mahesh,

Thank you for your review. We just submitted version -30 and addressed your comments. Please see detailed answer below.

Thanks,
Yingzhen

> On Jul 18, 2021, at 9:07 PM, Mahesh Jethanandani via Datatracker <[email-masked]> wrote:
> 
[Yingzhen]: We didn’t change this unless you have a strong opinion about it.
> s/suspectable/susceptible/
[Yingzhen]: thanks for catching these. Fixed.
> reference statements start on a new line, except for a few places where they do
[Yingzhen]: fixed.
> yang-version is 1.1., but it would be nice to state it explicitly.
[Yingzhen]: My understanding is RFC 7950 means YANG 1.1. If this has to be explicitly stated, please let us know.
> module can be updated separately from the rest of the module (much more easily).
[Yingzhen]: I think it’s a bit too late to make this change unless it’s really necessary.
> How does a user indicate both?
[Yingzhen]: I removed “both”.

> groupings are meant for use by other models, they should be folded into the
[Yingzhen]: we removed all the groupings not necessary.

> review, and if there was a discussion about it already, just ignore it).
[Yingzhen]: I’ll leave this to RFC editor.
> /routing-policy/policy-definitions.
[Yingzhen]: removed these two sub-tree.
> carries potential risk". What is "related model”?
[Yingzhen]: modified the text. Hope it’s clear now.

---

From: tom petch <[email-masked]>
Date: Fri, 30 Jul 2021 11:01:50 +0000

From: rtgwg <[email-masked]> on behalf of Yingzhen Qu <[email-masked]>
Sent: 30 July 2021 06:18

Hi Mahesh,

<tp>
I agree with you that it is a bit late to create IANA maintained modules.  It is the sort of thing that the rtgwg or lsr might have done years ago and then everyone could have used them, as NETMOD created RFC6991, but to do it now would just tread on the toes of those who have done it for themselves and confuse those yet to come,

Tom Petch

Thanks,
Yingzhen

[Yingzhen]: fixed.

> An idnits run of the draft was generally clean.

_______________________________________________
rtgwg mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/rtgwg

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Fri, 30 Jul 2021 11:41:10 +0000

The values for metric-types and route-level aren't likely to be changed very often and we have made them identities so that they can easily be augmented. 

Thanks,
Acee

On 7/30/21, 7:02 AM, "tom petch" <[email-masked]> wrote:

    Hi Mahesh,

    <tp>

    Tom Petch

    Thanks,
    Yingzhen

    [Yingzhen]: fixed.

---

From: Mahesh Jethanandani <[email-masked]>
Date: Fri, 30 Jul 2021 10:37:01 -0700

Hi Yingzhen,

Thanks for addressing my comments.

Cheers.

> On Jul 29, 2021, at 10:18 PM, Yingzhen Qu <[email-masked]> wrote:
> 
> Hi Mahesh,
> 

Mahesh Jethanandani
[email-masked]