Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 6 Jul 2020 21:55:56 +0000

Hi John, Yingzhen, Co-authors, et al,

With respect to #16, I think we should explicitly state that it is a most specific prefix match within the prefix-set. Unfortunately, that is not enough since the mask range limits are also a part of the prefix-set list key:

            container prefixes {
               description
                 "Container for the list of prefixes in a policy
                  prefix list.";

               list prefix-list {
                 key "ip-prefix mask-length-lower mask-length-upper";
                 description
                   "List of prefixes in the prefix set.";

                 uses prefix;
               }
             }

However, I really don’t see the utility of multiple prefix entries with the same prefix but different mask ranges. In fact, if they overlap there could be ambiguity without the missing rules. We could easily make up some rules involving mask-length-lower and mask-length-upper but my feeling is that we’d just be adding complexity without solving any real problem. My proposal would be to remove mask-length-lower and mask-length-upper from the list key.

Comments??

Thanks,
Acee

From: rtg-dir <[email-masked]> on behalf of John Scudder <jgs=[email-masked]>
Date: Monday, July 6, 2020 at 5:25 PM
To: Yingzhen Qu <[email-masked]>
Cc: Routing ADs <[email-masked]>, Routing Directorate <[email-masked]>, "[email-masked]" <[email-masked]>, Routing WG <[email-masked]>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

Hi Yingzhen,

Other than the question of prefix definition (my #16) everything looks good, just one nit,

                                                        creating

   policies nested beyond a small number of levels (e.g., 2-3) are

   discouraged.

Should be “is discouraged”. Sorry, I know I suggested that wording but I didn’t think about it in context. I was thinking “nested policies… are discouraged” but in fact it’s “creating” that’s being discouraged, so “is”.

Regarding #16, you propose changing from your current, which looks like

198.51.100.0/24 mask-length-lower 24 mask-length-upper 28

to be like this:

198.51.100/0/24 mask-length-max 28

The first form is more expressive and matches what’s in the field more closely, so I’d suggest retaining it or something like it. I just think the definition needs to be tightened. One thing to mention is the constraint that mask-length <= mask-length-lower <= mask-length-upper. You’ve made a start at that in your update to the description of mask-length-lower, but shouldn’t that be a “must” clause and not just a description? I guess maybe that would require you to change your prefix encoding so that you can break the mask length out as an atom, but it seems worth it. I also notice that -lower and -upper aren’t mandatory: should you specify what their defaults are if not specified? I presume -lower defaults to be equal to mask-length, and -upper defaults to be equal to -lower.

And finally, I think you should say a little more about the semantics of what a prefix match is. I suppose you can argue that this is common knowledge by now, but it seems to me it’s worth laying out. Given the way your document is structured, I’m not sure where I’d suggest putting the procedure, but for what it’s worth here’s what we did in RFC 6811 to define a similar matching procedure:

   o  Covered: A Route Prefix is said to be Covered by a VRP when the

      VRP prefix length is less than or equal to the Route prefix

      length, and the VRP prefix address and the Route prefix address

      are identical for all bits specified by the VRP prefix length.

      (That is, the Route prefix is either identical to the VRP prefix

      or more specific than the VRP prefix.)

   o  Matched: A Route Prefix is said to be Matched by a VRP when the

      Route Prefix is Covered by that VRP, the Route prefix length is

      less than or equal to the VRP maximum length, and the Route Origin

      ASN is equal to the VRP ASN.

Obviously this text doesn’t work verbatim for your document, but it probably isn’t that hard to adapt, e.g. something like:

   o  Covered: A Route Prefix is said to be Covered by a prefix filter

      when the mask-length is less than or equal to the Route prefix

      length, and the ip-prefix address and the Route prefix address

      are identical for all bits specified by the mask-length.

      (That is, the Route prefix is either identical to the prefix filter

      or more specific than the prefix filter.)

   o  Matched: A Route Prefix is said to be Matched by a prefix filter when the

      Route Prefix is Covered by that prefix filter, the Route prefix length is

      less than or equal to the max-length-upper, and the Route prefix

      length is greater than or equal to the max-length-lower.

I’m sure that needs some work, or of course you may not choose to adopt it at all, but you get the idea. Now that I think about it, this text doesn’t capture the idea of a best match, because we didn’t need that for RFC 6811. I’m not sure you need it either since a prefix-list either matches a route, or doesn’t, and it doesn’t have the potential for different dispositions depending on which leaf matched. If it did have that potential, you’d need to capture the idea of best-match.

$0.02,

—John

On Jul 5, 2020, at 8:22 PM, Yingzhen Qu <[email-masked]<mailto:[email-masked]>> wrote:

Hi John,

Thank you so much for your thorough review, really appreciate. I’ve uploaded version -17 to address your comments and please see detailed response below starting with [YQ].

Question 16 still remains open, and please let me know your comments.

Thanks,
Yingzhen

From: John Scudder <[email-masked]<mailto:[email-masked]>>
Date: Tuesday, June 30, 2020 at 11:14 AM
To: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, RTGWG <[email-masked]<mailto:[email-masked]>>
Subject: RtgDir review: draft-ietf-rtgwg-policy-model-16
Resent-From: <[email-masked]<mailto:[email-masked]>>
Resent-To: <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>
Resent-Date: Tuesday, June 30, 2020 at 11:14 AM

Hello,
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<https://urldefense.com/v3/__https:/nam11.safelinks.protection.outlook.com/?url=http*3A*2F*2Ftrac.tools.ietf.org*2Farea*2Frtg*2Ftrac*2Fwiki*2FRtgDir&data=02*7C01*7Cyingzhen.qu*40futurewei.com*7Cc87e948839554e7786e308d81d216ffc*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C1*7C637291376752227134&sdata=zixUU1xImjqWnN8jg7F2Qg4DaQkGNyn89M3TGl25Rg8*3D&reserved=0__;JSUlJSUlJSUlJSUlJSUlJSU!!NEt6yMaO-gk!XZuh9ykx2jpIDjT5R-BWU-N9PMpRCh_gjjLWQ-JGr5mSxXCIoz7BCHcCRqXLpQ$>
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-rtgwg-policy-model-16
Reviewer: John Scudder
Review Date: June 30, 2020
IETF LC End Date: ?
Intended Status: Standards Track

Summary: I have one significant concern about this document and recommend that the Routing ADs discuss this issue further with the authors.

Comments:
The draft is clear and readable, thank you for putting in the effort to produce a high-quality document.

Major Issues:
1. On page 17 you say:

        Policy 'subroutines' (or nested policies) are supported by
        allowing policy statement conditions to reference another
        policy definition which applies conditions and actions from
        the referenced policy before returning to the calling policy
        statement and resuming evaluation.  If the called policy
        results in an accept-route (either explicit or by default),
        then the subroutine returns an effective true value to the
        calling policy.  Similarly, a reject-route action returns
        false.  If the subroutine returns true, the calling policy
        continues to evaluate the remaining conditions (using a
        modified route if the subroutine performed any changes to the
        route).

I read this as saying further evaluations should consider the modified route, not the original route. But on page 10 you say:

   Note that the route's pre-policy attributes are always used for
   testing policy statement conditions.  In other words, if actions
   modify the policy application specific attributes, those
   modifications are not used for policy statement conditions.

Which is it? I think this has to be resolved one way or the other before progressing the document.

[YQ] Thanks for catching this. For nested policies, original data should always be used. I’ve changed the description in the model (the page 17 one) to match page 10.

Minor Issues:

2. For an outsider reading the document, the precise meanings of “import” and “export”, as well as “routing context” (and similar) are not clear. The first place I noticed this was in section 6:

   Policy chains are sequences of policy
   definitions (described in Section 4) that have an associated
   direction (import or export) with respect to the routing context in
   which they are defined.

Possibly all this would be obvious to someone with the necessary YANG expertise, I guess. I can pretty much intuit it because I’m familiar with routing policy, however I think it’s worth putting in the effort to make it clearer to the reader and not require them to use their intuition. Would it be accurate to rewrite the quoted sentence something like this?

“Policy chains are sequences of policy definitions (described in Section 4). They can be referenced from different contexts. For example, a policy chain could be associated with a routing protocol and used to control its interaction with its protocol peers. Or, it could be used to control the interaction between a routing protocol and the local routing information base. A policy chain has an associated direction (import or export), with respect to the context in which it is referenced."

[YQ]: thanks for suggesting the text. It’s modified as what you suggested.

3. This sentence rubbed me wrong:

   Nested policies are a
   convenience in many routing policy constructions but creating
   policies nested beyond a small number of levels (e.g., 2-3) should be
   discouraged.

By whom should they be discouraged? Maybe you mean “are discouraged”? By the way, *why* are they discouraged?

[YQ]: changed to “are discouraged”. The reason is also explained in the same paragraph, please let us know if you think this is not clear or enough.

4. A question about the next sentence:

   Also, implementations should have validation to assure
   that there is no recursion amongst nested routing policies.

I guess the concern about recursion is that given the route data is treated as immutable by policy and there’s no way to pass results or parameters, there’s no way to have a termination condition? This makes sense, or doesn’t, depend on how you resolve my question #1. (Also I think you mean “ensure”.)

[YQ]: A recursion might happen if you have policy A calling policy B, then policy B is calling policy A without proper termination condition. Bottom line is when a policy gets more complicated, it’s easier to make mistakes, and the operators need to be more careful.

5. Mostly in this document it’s clear that you’re using “operator” in the sense of “network operator” and not in its algebraic sense, but I found it a little ambiguous in this sentence:

   Match conditions may be further modified using the match-set-options
   configuration which allows operators to change the behavior of a
   match.  Three options are supported:

It would remove ambiguity if you changed this to say “network operators”.

[YQ]: Done.

6. Some of the types you describe are suffixed with “-type”, for example “ospf-external-type”. Others aren’t suffixed, for example “ospf-external-t1”. Is there any reason for this lack of consistency? It hurts my eyes but is otherwise harmless I suppose.
[YQ]: added “-type” to a few identities to make them consistent.

7. It’s unclear to me why the draft defines “bgp-local” and “bgp-external”, since it otherwise doesn’t deal with BGP in any way, leaving that for a separate document. Furthermore, I don’t even understand what the semantics of these are. I suppose “bgp-external” is likely to be a route received from an EBGP peer. As for “bgp-local”, I don’t care to guess.

[YQ]: I changed “bgp-local” to “bgp-internal”, also changed descriptions.

8. The description of ip-prefix says “The IP prefix represented as an IPv6 or IPv4 network number followed prefix length with an intervening slash character a delimiter”. I think maybe you are missing a “by a” and a “as”? As in, "The IP prefix represented as an IPv6 or IPv4 network number followed by a prefix length with an intervening slash character as a delimiter.”
[YQ]: fixed.

9. Speaking of IP addresses and prefixes, you use net 10 in one of your examples. It’s my understanding that this is poor form, that you should be using the “documentation” addresses given in RFC 6890. (Thanks for doing that with the rest of the document, BTW.)
[YQ]: Changed net 10 to 198.51.100.0/24, one of the documentation addresses.

10. Also speaking of prefixes, all your examples are nicely-formed. Here’s a less-nicely formed one: 192.0.2.0/8 mask-length-lower=24 mask-length-upper=24. Does this match 192.0.2/24? Does it match 192.0.0/24? How about 192.255.1/24? Does it match something else, if so what? Is it a syntax error? I think this relates to my later question #16.
[YQ]: Please see answer to question #16.

11. You describe export-policy as:

           "List of policy names in sequence to be applied on
            sending a routing update in the current context, e.g.,
            for the current peer group, neighbor, address family,
            etc.";

Is export-policy really restricted only to the formation of routing updates? This makes perfect sense for BGP, but I don’t think the link-state IGPs really work this way, for example.

[YQ]: you’re right, the descriptions regarding peer group etc. are more BGP-centric. I’ve changed the description for both import-policy and export-policy. Please let me know if you have more comments. Cop&paste below for your convenience.
Import-policy:
      description
        "List of policy names in sequence to be applied on
         receiving redistributed routes from another routing protocol
         or receiving a routing update in the current context, e.g.,
         for the current peer group, neighbor, address family,
         etc.";
Export-policy:
      description
        "List of policy names in sequence to be applied on
         redistributing routes from one routing protocol to another
         or sending a routing update in the current context, e.g.,
         for the current peer group, neighbor, address family,
         etc.";

12. You describe apply-policy as:

           "Anchor point for routing policies in the model.
            Import and export policies are with respect to the local
            routing table, i.e., export (send) and import (receive),
            depending on the context.";

This is not clear to me (sorry). If I expand “i.e.” as “in other words” it doesn’t help I’m afraid. Since I don’t understand it I can’t suggest a fix. :-(

[YQ]: this grouping is meant to be used by other models. There is also the following description at the grouping level:
    description
      "Top level container for routing policy applications. This
       grouping is intended to be used in routing models where
       needed.";
This model supports both import and export mode, so it’s up to an implementation to choose the right one.

13. Where you say “ambiguous and implementation dependent” I suggest dropping “and implementation dependent”, it seems redundant.
[YQ]: done.

14. I feel sad that all the examples are IPv4, poor IPv6 remains the red-headed stepchild. ¯\_(ツ)_/¯
[YQ]: ¯\_(ツ)_/¯

15. In the final example on p. 37, I scratched my head a little that the operation being done is set-import-level. I guess the policy is going to be applied in the IS-IS context, and therefore it’s not OSPF exporting the route, but rather IS-IS importing the route into its LSPDB? This is what led to my comment #2.
[YQ]: yes, it’s IS-IS to import OSPF routes. Vendors have different implementations when redistributing routes, and the model support both import and export.

16. I don’t think you ever define the semantics of a prefix set. Of course “everyone knows” that this should be a best match, and not (say) sequential match or something else — but please say so. See also my comment #10, you also seem to be underspecified with respect with what to do if mask-length-lower is greater than the given mask length.

[YQ]: Thanks for bringing up the prefix length issue. How about we change it to only ip-prefix and “mask-length-max”? similar to “ipAddrBlocks” defined in RFC 6482. The “mask-length-max” should not be less than the prefix length.
Open to suggestions on this.
Nits:
[YQ]: all fixed.
17. I’ve attached an edited version of the draft with a few small typo and grammar corrections. Here’s a diff as well, vs. the base -16:

jgs-mbp:Downloads jgs$ diff draft-ietf-rtgwg-policy-model-16.txt draft-ietf-rtgwg-policy-model-16-jgs-edits.txt
155c155
<    exported, modified, and advertised between routing protocols
---
>    exported, modified, and advertised between routing protocol
302c302
<    The models provides a set of generic sets that can be used for
---
>    The models provide a set of generic sets that can be used for
777c777
<    elsewhere in the draft.
---
>    elsewhere in this document.
876c876
<         or comparison operations, and similarly actions may be
---
>         or comparison operations, and similarly actions may be a
996c996
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1014c1014
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1059c1059
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1077c1077
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1088c1088
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1103c1103
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1113c1113
<          "RFC 5302 - Domain-Wide Prefix Distributino with
---
>          "RFC 5302 - Domain-Wide Prefix Distribution with
1483c1483
<              "Condition to check the protocol specific type
---
>              "Condition to check the protocol-specific type
2208c2208
<    The routing policy module defined in this draft is based on the
---
>    The routing policy module defined in this document is based on the

---

From: Yingzhen Qu <[email-masked]>
Date: Tue, 7 Jul 2020 05:14:58 +0000

Hi John and all,

Thanks for the comments and suggestions.

Regarding question #16, the leaf “ip-prefix” type includes both the address and mask length as a string, so it’s hard to add a “must” statement using mask length. If we separate the prefix definition into address and mask length, we can have something like:

  grouping prefix {
    description
      "Configuration data for a prefix definition.";

    leaf ip-address {
      type inet:ip-address;
      mandatory true;
      description
        "An IPv6 or IPv4 address. A prefix is defined using
         both ip-address and mask-length. All members of the
         prefix set should be of the same address family as
         the prefix-set mode.";
    }

    leaf mask-length {
      type uint8 {
        range "1..128";
      }
      mandatory true;
      description
        "The mask-length, is in range from 0 to 32 for IPv4,
         0 to 128 for IPv6.";
    }

    leaf mask-length-lower {
      type uint8;
      must "../mask-length-lower >= ../mask-length" {
        error-message "The lower bound should not be less"
                    + "than the mask-length.";
      }
      description
        "Mask length range lower bound.";
    }
    leaf mask-length-upper {
      type uint8 {
        range "1..128";
      }
      must "../mask-length-upper >= ../mask-length-lower" {
        error-message "The upper bound should not be less"
                    + "than lower bound.";
      }
      description
        "Mask length range upper bound.

         The combination of mask-length-lower and mask-length-upper
         define a range for the mask length, or single 'exact'
         length if mask-length-lower and mask-length-upper are
         equal.

         Example: 192.0.2.0/24 through 192.0.2.0/26 would be
         expressed as prefix: 192.0.2.0/24,
                      mask-length-lower=24,
                      mask-length-upper=26

         Example: 192.0.2.0/24 (an exact match) would be
         expressed as prefix: 192.0.2.0/24,
                      mask-length-lower=24,
                      mask-length-upper=24

         Example: any prefix of length 28 in the range from
         192.0.2.0/28 to 192.0.2.240/28.
         expressed as prefix: 192.0.2.0/24,
                      mask-length-lower=28,
                      mask-length-upper=28

         A route prefix is said to be matched by a prefix list
         when the route prefix length is less than or equal to
         the mask-length-upper, and is greater than or equal to
         the mask-length-lower.";
    }
  }

A “must” is added to the mask-length-lower to constrain it’s not less than the mask length.  I also added the “match” explanation at the end of the description and one more example.

Please let me know your comments.

Thanks,
Yingzhen

From: "Acee Lindem (acee)" <[email-masked]>
Date: Monday, July 6, 2020 at 2:56 PM
To: Yingzhen Qu <[email-masked]>, John Scudder <[email-masked]>
Cc: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, RTGWG <[email-masked]>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

            container prefixes {
               description

               list prefix-list {

                 uses prefix;
               }
             }

Comments??

Thanks,
Acee

Hi Yingzhen,

                                                        creating

   discouraged.

to be like this:

$0.02,

—John

Hi John,

Thanks,
Yingzhen

Hello,
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<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldefense.com%2Fv3%2F__https%3A%2Fnam11.safelinks.protection.outlook.com%2F%3Furl%3Dhttp*3A*2F*2Ftrac.tools.ietf.org*2Farea*2Frtg*2Ftrac*2Fwiki*2FRtgDir%26data%3D02*7C01*7Cyingzhen.qu*40futurewei.com*7Cc87e948839554e7786e308d81d216ffc*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C1*7C637291376752227134%26sdata%3DzixUU1xImjqWnN8jg7F2Qg4DaQkGNyn89M3TGl25Rg8*3D%26reserved%3D0__%3BJSUlJSUlJSUlJSUlJSUlJSU!!NEt6yMaO-gk!XZuh9ykx2jpIDjT5R-BWU-N9PMpRCh_gjjLWQ-JGr5mSxXCIoz7BCHcCRqXLpQ%24&data=02%7C01%7Cyingzhen.qu%40futurewei.com%7Cf7d05d58ffac4064cc4508d821f7601e%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637296693645977131&sdata=dB9UN0wZ5Ip1hwRGGBAHWdNqZwiMqd%2F2abeH1YO3p6o%3D&reserved=0>

Comments:

Major Issues:
1. On page 17 you say:

Minor Issues:

   Nested policies are a

[YQ]: Done.

302c302
777c777
876c876
996c996
1014c1014
1059c1059
1077c1077
1088c1088
1103c1103
1113c1113
1483c1483
2208c2208

---

From: tom petch <[email-masked]>
Date: Tue, 7 Jul 2020 10:29:39 +0000

From: rtgwg <[email-masked]> on behalf of Yingzhen Qu <[email-masked]>
Sent: 07 July 2020 06:14

Hi John and all,

<tp>
Inserting my comment before all the YANG, just because it is technically possible to specify something formally in a DDL does not always make it a good idea. Sometimes the DDL becomes so complex that it gets misunderstood.  I did comment earlier about the YANG requiring lower and upper to be present and to have upper >= lower but I do not suggest going beyond that.  For example in the YANG below there is a contradiction between length zero and length one;-)  Sometimes a line of text is the better engineering IMHO.

Tom Petch

  grouping prefix {
    description

    leaf ip-address {
      type inet:ip-address;
      mandatory true;
      description

    leaf mask-length {
      type uint8 {
        range "1..128";
      }
      mandatory true;
      description

    leaf mask-length-lower {
      type uint8;

Thanks,
Yingzhen

            container prefixes {
               description

               list prefix-list {

                 uses prefix;
               }
             }

Comments??

Thanks,
Acee

Hi Yingzhen,

                                                        creating

   discouraged.

to be like this:

$0.02,

—John

Hi John,

Thanks,
Yingzhen

Hello,

Comments:

Major Issues:
1. On page 17 you say:

Minor Issues:

   Nested policies are a

[YQ]: Done.

302c302
777c777
876c876
996c996
1014c1014
1059c1059
1077c1077
1088c1088
1103c1103
1113c1113
1483c1483
2208c2208

---

From: John Scudder <[email-masked]>
Date: Tue, 7 Jul 2020 14:33:38 +0000

Hi Yingzhen,

Shouldn’t all the ranges be “0..X” and not “1..X”?

I’ll think about the rest and reply later.

Thanks,

—John

On Jul 7, 2020, at 1:14 AM, Yingzhen Qu <[email-masked]<mailto:[email-masked]>> wrote:

Hi John and all,

  grouping prefix {
    description

    leaf ip-address {
      type inet:ip-address;
      mandatory true;
      description

    leaf mask-length {
      type uint8 {
        range "1..128";
      }
      mandatory true;
      description

    leaf mask-length-lower {
      type uint8;

Thanks,
Yingzhen

From: "Acee Lindem (acee)" <[email-masked]<mailto:[email-masked]>>
Date: Monday, July 6, 2020 at 2:56 PM
To: Yingzhen Qu <[email-masked]<mailto:[email-masked]>>, John Scudder <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, RTGWG <[email-masked]<mailto:[email-masked]>>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

            container prefixes {
               description

               list prefix-list {

                 uses prefix;
               }
             }

Comments??

Thanks,
Acee

From: rtg-dir <[email-masked]<mailto:[email-masked]>> on behalf of John Scudder <jgs=[email-masked]<mailto:jgs=[email-masked]>>
Date: Monday, July 6, 2020 at 5:25 PM
To: Yingzhen Qu <[email-masked]<mailto:[email-masked]>>
Cc: Routing ADs <[email-masked]<mailto:[email-masked]>>, Routing Directorate <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Routing WG <[email-masked]<mailto:[email-masked]>>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

Hi Yingzhen,

                                                        creating

   discouraged.

to be like this:

$0.02,

—John

Hi John,

Thanks,
Yingzhen

Hello,
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<https://urldefense.com/v3/__https://nam11.safelinks.protection.outlook.com/?url=https*3A*2F*2Furldefense.com*2Fv3*2F__https*3A*2Fnam11.safelinks.protection.outlook.com*2F*3Furl*3Dhttp*3A*2F*2Ftrac.tools.ietf.org*2Farea*2Frtg*2Ftrac*2Fwiki*2FRtgDir*26data*3D02*7C01*7Cyingzhen.qu*40futurewei.com*7Cc87e948839554e7786e308d81d216ffc*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C1*7C637291376752227134*26sdata*3DzixUU1xImjqWnN8jg7F2Qg4DaQkGNyn89M3TGl25Rg8*3D*26reserved*3D0__*3BJSUlJSUlJSUlJSUlJSUlJSU!!NEt6yMaO-gk!XZuh9ykx2jpIDjT5R-BWU-N9PMpRCh_gjjLWQ-JGr5mSxXCIoz7BCHcCRqXLpQ*24&data=02*7C01*7Cyingzhen.qu*40futurewei.com*7Cf7d05d58ffac4064cc4508d821f7601e*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C0*7C637296693645977131&sdata=dB9UN0wZ5Ip1hwRGGBAHWdNqZwiMqd*2F2abeH1YO3p6o*3D&reserved=0__;JSUlJSUlJSUlJSoqKioqKioqJSUqKioqKioqKiUlKiUlJSUlJSUlJSUlJSUl!!NEt6yMaO-gk!UJWlfyEqmryZ-Cq3BTJOypz8nX9nQVhp3FznEXF1Qw0qaaXHrRuYDIePEb3rZw$>

Comments:

Major Issues:
1. On page 17 you say:

Minor Issues:

   Nested policies are a

[YQ]: Done.

302c302
777c777
876c876
996c996
1014c1014
1059c1059
1077c1077
1088c1088
1103c1103
1113c1113
1483c1483
2208c2208

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Tue, 7 Jul 2020 15:16:24 +0000

Hi John, Yingzhen,

From: John Scudder <[email-masked]>
Date: Tuesday, July 7, 2020 at 10:33 AM
To: Yingzhen Qu <[email-masked]>
Cc: Acee Lindem <[email-masked]>, Routing ADs <[email-masked]>, Routing Directorate <[email-masked]>, "[email-masked]" <[email-masked]>, Routing WG <[email-masked]>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

Hi Yingzhen,

Yes. I’d say we should just use the ip-prefix type from RFC 6021. This type has the right semantics.
However, I’m wondering how we do the mask-length-lower checking with the union. I imagine it should be possible.
Thanks,
Acee

Thanks,

—John

Hi John and all,

  grouping prefix {
    description

    leaf ip-address {
      type inet:ip-address;
      mandatory true;
      description

    leaf mask-length {
      type uint8 {
        range "1..128";
      }
      mandatory true;
      description

    leaf mask-length-lower {
      type uint8;

Thanks,
Yingzhen

            container prefixes {
               description

               list prefix-list {

                 uses prefix;
               }
             }

Comments??

Thanks,
Acee

Hi Yingzhen,

                                                        creating

   discouraged.

to be like this:

$0.02,

—John

Hi John,

Thanks,
Yingzhen

Hello,
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<https://urldefense.com/v3/__https:/nam11.safelinks.protection.outlook.com/?url=https*3A*2F*2Furldefense.com*2Fv3*2F__https*3A*2Fnam11.safelinks.protection.outlook.com*2F*3Furl*3Dhttp*3A*2F*2Ftrac.tools.ietf.org*2Farea*2Frtg*2Ftrac*2Fwiki*2FRtgDir*26data*3D02*7C01*7Cyingzhen.qu*40futurewei.com*7Cc87e948839554e7786e308d81d216ffc*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C1*7C637291376752227134*26sdata*3DzixUU1xImjqWnN8jg7F2Qg4DaQkGNyn89M3TGl25Rg8*3D*26reserved*3D0__*3BJSUlJSUlJSUlJSUlJSUlJSU!!NEt6yMaO-gk!XZuh9ykx2jpIDjT5R-BWU-N9PMpRCh_gjjLWQ-JGr5mSxXCIoz7BCHcCRqXLpQ*24&data=02*7C01*7Cyingzhen.qu*40futurewei.com*7Cf7d05d58ffac4064cc4508d821f7601e*7C0fee8ff2a3b240189c753a1d5591fedc*7C1*7C0*7C637296693645977131&sdata=dB9UN0wZ5Ip1hwRGGBAHWdNqZwiMqd*2F2abeH1YO3p6o*3D&reserved=0__;JSUlJSUlJSUlJSoqKioqKioqJSUqKioqKioqKiUlKiUlJSUlJSUlJSUlJSUl!!NEt6yMaO-gk!UJWlfyEqmryZ-Cq3BTJOypz8nX9nQVhp3FznEXF1Qw0qaaXHrRuYDIePEb3rZw$>

Comments:

Major Issues:
1. On page 17 you say:

Minor Issues:

   Nested policies are a

[YQ]: Done.

302c302
777c777
876c876
996c996
1014c1014
1059c1059
1077c1077
1088c1088
1103c1103
1113c1113
1483c1483
2208c2208

---

From: "John G. Scudder" <[email-masked]>
Date: Tue, 7 Jul 2020 11:35:01 -0400

Hi Acee,

> On Jul 7, 2020, at 11:16 AM, Acee Lindem (acee) <acee=[email-masked]> wrote:
> 

How would you test the constraint on mask-length-upper if you use ip-prefix?

—John

---

From: John Scudder <[email-masked]>
Date: Tue, 7 Jul 2020 16:05:58 +0000

On Jul 7, 2020, at 11:35 AM, John G. Scudder <[email-masked]> wrote:

Hi Acee,

Duh, that’s what you said, s/upper/lower/. I don’t see a way to do it, but if you can work it out I agree it otherwise seems like the right approach.

—John

---

From: Yingzhen Qu <[email-masked]>
Date: Tue, 7 Jul 2020 18:45:26 +0000

Hi all,

Thank you all for the discussions and comments.

If we’re to do error-checking in the YANG module, here’re the two issues:

Ip-prefix is a union of “ipv4-prefix” and “ipv6-prefix”. The mask-length-upper is 32 for IPv4 and 128 for IPv6, so we either have to separate ipv4 and ipv6 in order to add a constraint or it will be up to the server to reject the config.

“ipv4-prefix” and “ipv6-prefix” are defined as a string, so the mask-length is part of the string. I don’t know an easy way to add a constraint if mask-length-lower needs to be verified against mask-length.

Any comments and suggestions are welcome.

Thanks,
Yingzhen

From: John Scudder <[email-masked]>
Date: Tuesday, July 7, 2020 at 9:06 AM
To: "John G. Scudder" <[email-masked]>
Cc: "Acee Lindem (acee)" <acee=[email-masked]>, Yingzhen Qu <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, RTGWG <[email-masked]>
Subject: Re: [RTG-DIR] RtgDir review: draft-ietf-rtgwg-policy-model-16

Hi Acee,

—John

---

From: John Scudder <[email-masked]>
Date: Tue, 7 Jul 2020 19:13:11 +0000

I defer to people with more YANG chops than I have, to decide whether the module should embed the error checking or if the constraints should just be written out in prose and left for the implementor. On the face of it, it’s attractive to have the YANG do it, because it minimizes the scope for implementations to get it wrong, but I understand that it comes with tradeoffs.

The main point in my review was that the semantics of what a prefix filter does need to be clearly expressed in the document. I’m OK with this being in code, or in prose, as long as it’s clear and unambiguous.

Thanks,

—John

On Jul 7, 2020, at 2:45 PM, Yingzhen Qu <[email-masked]> wrote:

Hi all,

Thanks,
Yingzhen

Hi Acee,

—John

---

From: tom petch <[email-masked]>
Date: Wed, 8 Jul 2020 11:52:53 +0000

Sent: 07 July 2020 19:45

Hi all,

<tp> 
As I said before, it is possible to make more checks with YANG but as you point out, they are complex.  For me, it is not worth the added complexity and I would just make sure that the description is clear to such as RTG-DIR.  'must' be present, and max>= min I think fine, but that is as far as I would go.

Where ipv4 goes one way and ipv6 goes another then YANG choice case case comes to mind but probably not worth it here.

Tom Petch

Thanks,
Yingzhen

Hi Acee,

—John

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 13 Jul 2020 17:47:29 +0000

Hi Tom, et al, 
In the latest version, -17, we have used the standard IETF ip-prefix type and specified the constrain in the text. 
Thanks,
Acee

On 7/8/20, 7:53 AM, "tom petch" <[email-masked]> wrote:

    Hi all,

    <tp> 

    Tom Petch

    Thanks,
    Yingzhen

    Hi Acee,

    —John