Subject: [Rtg-yang-coord] comment concerning draft-shaikh-rtgwg-policy-model-00

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 27 May 2015 14:24:56 +0200

Hi,

I have read the document and I have a small comment.

- You define a list 'prefix' in the list 'prefix-set' which is keyed
  by three leafs. As a result, all three leafs are mandatory. Your XML
  instance snipped in section 10 does not validate because it is
  missing mandatory key elements. I wonder (a) why did you not use
  inet:ip-prefix instead of the pair inet:ip-address and a unit8
  masklength. And given the desire to represent a prefix range, would
  a special syntax not make sense, e.g. an extension of ip-prefix,
  lets call it ip-prefix-range, that allows to express a range of
  prefixes:

  10.3.192.0/21-24 -> 10.3.192.0/21
  		      10.3.192.0/22
		      10.3.192.0/23
		      10.3.192.0/24

  If the '-24' part is made optional, your prefix list will be
  collapsed to a simple

  list prefix {
      key prefix-range;

      leaf prefix-range {
          type ip-prefix-range;
      }
  }

  Such an ip-prefix-range type may even be a useful addition to
  ietf-inet-types.

- The XML snippets are nice but it would be cool if they were using
  proper namespaces and validate against the data model. Perhaps
  something to consider for -01. The pyang tutorial provides examples
  how to validate XML snippets against YANG definitions.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Anees Shaikh <[email-masked]>
Date: Wed, 27 May 2015 09:09:37 -0700

Juergen, you're right that prefix-lists in the model turned out to be a bit
problematic as currently defined.  We've updated the model in the most
recent revision after going through some translations of our configs to the
OpenConfig model and running into some issues.  The updated version looks
more like the following:

list prefix {
          key "address masklength masklength-range";

          leaf address {
            type inet:ip-address;
          }

          leaf masklength {
            type uint8;
          }

          leaf masklength-range {
            type string {
              pattern '^([0-9]+\.\.[0-9]+)|exact$';
            }
         }
}

The model used address and a separate masklength so that range checking
could be performed on the mask.   We've since removed the range-checking in
favor of simplifying to keep a single prefix list for both address types,
ipv4 and ipv6.

The original model had the problem you mention, which is that all keys must
be mandatory, including masklength-range, which meant there was no simple
way to express an exact prefix other than, e.g., 21..21 which we didn't
think was desirable.  Changing the masklength-range to a string allows a
more flexible pattern (including an 'exact' setting).  The masklength-range
is included in the keys to make the list have unique members.

I think we could also consider your suggestion to use a inet:ip-prefix
instead of address since we're not doing range-checking on the masklength
in the current approach -- will discuss with the co-authors.

Agree with your suggestion to make the XML examples more conformant (they
should certainly validate), though we've only been using ad-hoc namespaces
for models that are not IETF WG models so far.  My understanding is that
this is the recommended practice.

thanks.
-- Anees

On Wed, May 27, 2015 at 5:24 AM, Juergen Schoenwaelder <
[email-masked]> wrote:

> _______________________________________________
> Rtg-yang-coord mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtg-yang-coord
>

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 27 May 2015 16:47:01 +0000

Hi Anees,
I had noted problems with the prefix-lists in the draft as well but hadn’t got around to raising them. One thing about the masklength and masklength-range is that there is no validation in the model snippet below. In other words, there is nothing to express that the masklength must between 0..32 for IPv4 addresses and 0..128 for IPv6 addresses  (this is enforced by the ip-prefix type from RFC 6991). Similarly, as a string there in no inherent validation of the masklength-range (e.g., both values must be >= masklength, second value must be >= first-value, and both values must be >= AF specific limit). Also, there is nothing to prevent one from mixing IPv4 and IPv6 prefixes in the same length which is definitely not allowed for the prefix-lists we all know and love.
Thanks,
Acee

From: Anees Shaikh <[email-masked]<mailto:[email-masked]>>
Date: Wednesday, May 27, 2015 at 12:09 PM
To: Juergen Schoenwaelder <[email-masked]<mailto:[email-masked]>>, Routing YANG <[email-masked]<mailto:[email-masked]>>
Subject: Re: [Rtg-yang-coord] comment concerning draft-shaikh-rtgwg-policy-model-00

Juergen, you're right that prefix-lists in the model turned out to be a bit problematic as currently defined.  We've updated the model in the most recent revision after going through some translations of our configs to the OpenConfig model and running into some issues.  The updated version looks more like the following:

list prefix {

          leaf address {
            type inet:ip-address;
          }

          leaf masklength {
            type uint8;
          }

          leaf masklength-range {
            type string {

The model used address and a separate masklength so that range checking could be performed on the mask.   We've since removed the range-checking in favor of simplifying to keep a single prefix list for both address types, ipv4 and ipv6.

The original model had the problem you mention, which is that all keys must be mandatory, including masklength-range, which meant there was no simple way to express an exact prefix other than, e.g., 21..21 which we didn't think was desirable.  Changing the masklength-range to a string allows a more flexible pattern (including an 'exact' setting).  The masklength-range is included in the keys to make the list have unique members.

I think we could also consider your suggestion to use a inet:ip-prefix instead of address since we're not doing range-checking on the masklength in the current approach -- will discuss with the co-authors.

Agree with your suggestion to make the XML examples more conformant (they should certainly validate), though we've only been using ad-hoc namespaces for models that are not IETF WG models so far.  My understanding is that this is the recommended practice.

thanks.
-- Anees

On Wed, May 27, 2015 at 5:24 AM, Juergen Schoenwaelder <[email-masked]<mailto:[email-masked]>> wrote:
Hi,

  10.3.192.0/21-24<http://10.3.192.0/21-24> -> 10.3.192.0/21<http://10.3.192.0/21>
                      10.3.192.0/22<http://10.3.192.0/22>
                      10.3.192.0/23<http://10.3.192.0/23>
                      10.3.192.0/24<http://10.3.192.0/24>

  list prefix {
      key prefix-range;

      leaf prefix-range {
          type ip-prefix-range;
      }
  }

/js

--
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587<tel:%2B49%20421%20200%203587>         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103<tel:%2B49%20421%20200%203103>         <http://www.jacobs-university.de/>

_______________________________________________
Rtg-yang-coord mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/rtg-yang-coord

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 27 May 2015 16:57:34 +0000

On May 27, 2015, at 12:47 PM, Acee Lindem (acee) <[email-masked]<mailto:[email-masked]>> wrote:

Hi Anees,
I had noted problems with the prefix-lists in the draft as well but hadn’t got around to raising them. One thing about the masklength and masklength-range is that there is no validation in the model snippet below. In other words, there is nothing to express that the masklength must between 0..32 for IPv4 addresses and 0..128 for IPv6 addresses  (this is enforced by the ip-prefix type from RFC 6991). Similarly, as a string there in no inherent validation of the masklength-range (e.g., both values must be >= masklength, second value must be >= first-value, and both values must be >= AF specific limit).

Of course I meant "<= AF specific limit”.

Thanks,
Acee

Also, there is nothing to prevent one from mixing IPv4 and IPv6 prefixes in the same length which is definitely not allowed for the prefix-lists we all know and love.
Thanks,
Acee

list prefix {

          leaf address {
            type inet:ip-address;
          }

          leaf masklength {
            type uint8;
          }

          leaf masklength-range {
            type string {

thanks.
-- Anees

  list prefix {
      key prefix-range;

      leaf prefix-range {
          type ip-prefix-range;
      }
  }

/js

---

From: Anees Shaikh <[email-masked]>
Date: Wed, 27 May 2015 22:53:31 -0700

hi Acee, thanks for the comments -- we debated these same questions :-)

This approach was based on the wish to avoid operators having to worry
about specifying an ipv4 prefix list separately from an ipv6 -- no major
implementation has this separation today (though it could be considered).

If using inet:ip-prefix type instead of inet:ip-address + masklength buys
us some range checking in the pattern , maybe that's another reason to
switch to using that union type instead.  We will discuss making that
update.

You're right that with a string we don't do checking on the
masklength-range field.  As I mentioned earlier in the thread, this was to
accommodate a simple way to express an exact prefix (several
implementations do something similar).

Among OpenConfig operators, after some debate, we generally try to balance
the amount of validation in the model itself, with maintaining flexibility
and simplicity.  In other words, we don't believe the model needs to
prevent every conceivable error -- the implementation also has to
participate by rejecting invalid / unsupported configuration.  That said,
where it's easy to add validation, we have tried to include it, though
ranges, defaults, min/max values, etc. get harder when trying to build
something vendor-neutral.

Same applies to your correct comment that the current construct
theoretically allows one to mix address types in the prefix list.
Interestingly, some implementations allow you to create such a list, but
fail later when you try to use it in a policy.   We've chosen for now to
allow it (again for simplicity), but explicitly say in the description that
most implementations will reject a prefix-list with mixed address types.

thanks.
-- Anees

On Wed, May 27, 2015 at 9:57 AM, Acee Lindem (acee) <[email-masked]> wrote:

>
>  On May 27, 2015, at 12:47 PM, Acee Lindem (acee) <[email-masked]> wrote:
>
>  Hi Anees,
> I had noted problems with the prefix-lists in the draft as well but hadn’t
> got around to raising them. One thing about the masklength and
> masklength-range is that there is no validation in the model snippet below.
> In other words, there is nothing to express that the masklength must
> between 0..32 for IPv4 addresses and 0..128 for IPv6 addresses  (this is
> enforced by the ip-prefix type from RFC 6991). Similarly, as a string there
> in no inherent validation of the masklength-range (e.g., both values must
> be >= masklength, second value must be >= first-value, and both values must
> be >= AF specific limit).
>
>
>   Also, there is nothing to prevent one from mixing IPv4 and IPv6
> prefixes in the same length which is definitely not allowed for the
> prefix-lists we all know and love.
> Thanks,
> Acee
>
>   From: Anees Shaikh <[email-masked]>
> To: Juergen Schoenwaelder <[email-masked]>, Routing
> YANG <[email-masked]>
>   Juergen, you're right that prefix-lists in the model turned out to be a
> bit problematic as currently defined.  We've updated the model in the most
>  The original model had the problem you mention, which is that all keys
> must be mandatory, including masklength-range, which meant there was no
> simple way to express an exact prefix other than, e.g., 21..21 which we
> didn't think was desirable.  Changing the masklength-range to a string
> allows a more flexible pattern (including an 'exact' setting).  The
> masklength-range is included in the keys to make the list have unique
> members.
>
>  Agree with your suggestion to make the XML examples more conformant
> (they should certainly validate), though we've only been using ad-hoc
> namespaces for models that are not IETF WG models so far.  My understanding
> is that this is the recommended practice.
>
>  thanks.
> -- Anees
>
>

---

From: Jeff Tantsura <[email-masked]>
Date: Thu, 28 May 2015 06:50:58 +0000

Hi Anees,

Using ip:prefix type would for sure benefit from better validation enforcement.
Saying "for now" - do you expect it to be changed later?

Thanks!

Regards,
Jeff

On May 27, 2015, at 10:53 PM, "Anees Shaikh" <[email-masked]<mailto:[email-masked]>> wrote:

This approach was based on the wish to avoid operators having to worry about specifying an ipv4 prefix list separately from an ipv6 -- no major implementation has this separation today (though it could be considered).

If using inet:ip-prefix type instead of inet:ip-address + masklength buys us some range checking in the pattern , maybe that's another reason to switch to using that union type instead.  We will discuss making that update.

You're right that with a string we don't do checking on the masklength-range field.  As I mentioned earlier in the thread, this was to accommodate a simple way to express an exact prefix (several implementations do something similar).

Among OpenConfig operators, after some debate, we generally try to balance the amount of validation in the model itself, with maintaining flexibility and simplicity.  In other words, we don't believe the model needs to prevent every conceivable error -- the implementation also has to participate by rejecting invalid / unsupported configuration.  That said, where it's easy to add validation, we have tried to include it, though ranges, defaults, min/max values, etc. get harder when trying to build something vendor-neutral.

Same applies to your correct comment that the current construct theoretically allows one to mix address types in the prefix list.  Interestingly, some implementations allow you to create such a list, but fail later when you try to use it in a policy.   We've chosen for now to allow it (again for simplicity), but explicitly say in the description that most implementations will reject a prefix-list with mixed address types.

thanks.
-- Anees

On Wed, May 27, 2015 at 9:57 AM, Acee Lindem (acee) <[email-masked]<mailto:[email-masked]>> wrote:

Hi Anees,

Thanks,
Acee

list prefix {

          leaf address {
            type inet:ip-address;
          }

          leaf masklength {
            type uint8;
          }

          leaf masklength-range {
            type string {

thanks.
-- Anees

  list prefix {
      key prefix-range;

      leaf prefix-range {
          type ip-prefix-range;
      }
  }

/js

---

From: Anees Shaikh <[email-masked]>
Date: Thu, 28 May 2015 00:27:30 -0700

hi Jeff, agree --  we'll try to get this fix into the next published
version of the models -- which should hopefully be available very soon
(said with fingers crossed).

thanks.
-- Anees

On Wed, May 27, 2015 at 11:50 PM, Jeff Tantsura <[email-masked]>
wrote:

> On May 27, 2015, at 10:53 PM, "Anees Shaikh" <[email-masked]> wrote:
>
>  If using inet:ip-prefix type instead of inet:ip-address + masklength
> buys us some range checking in the pattern , maybe that's another reason to
>  Among OpenConfig operators, after some debate, we generally try to
> balance the amount of validation in the model itself, with maintaining
> flexibility and simplicity.  In other words, we don't believe the model
> needs to prevent every conceivable error -- the implementation also has to
>> I had noted problems with the prefix-lists in the draft as well but
>> hadn’t got around to raising them. One thing about the masklength and
>> To: Juergen Schoenwaelder <[email-masked]>,
>> Routing YANG <[email-masked]>
>>   Juergen, you're right that prefix-lists in the model turned out to be
>> a bit problematic as currently defined.  We've updated the model in the
>> most recent revision after going through some translations of our configs
>> to the OpenConfig model and running into some issues.  The updated version
>> looks more like the following:
>>
>>  list prefix {

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Thu, 28 May 2015 09:53:17 +0000

Thanks Anees - One reason I brought these validation questions up is that we’re struggling with what validation needs to be in the model and what is left to the devices with other models.

Acee

From: Anees Shaikh <[email-masked]<mailto:[email-masked]>>
Date: Thursday, May 28, 2015 at 1:53 AM
To: Acee Lindem <[email-masked]<mailto:[email-masked]>>
Cc: Juergen Schoenwaelder <[email-masked]<mailto:[email-masked]>>, Routing YANG <[email-masked]<mailto:[email-masked]>>
Subject: Re: [Rtg-yang-coord] comment concerning draft-shaikh-rtgwg-policy-model-00

thanks.
-- Anees

Hi Anees,

Thanks,
Acee

list prefix {

          leaf address {
            type inet:ip-address;
          }

          leaf masklength {
            type uint8;
          }

          leaf masklength-range {
            type string {

thanks.
-- Anees

  list prefix {
      key prefix-range;

      leaf prefix-range {
          type ip-prefix-range;
      }
  }

/js

---

From: Mahesh Jethanandani <[email-masked]>
Date: Thu, 28 May 2015 13:18:22 -0700

Aneesh,

When I look at a model to see where the ranges, defaults, min/max values can be defined, I rely on protocol definition and what the expectation is of the packet on the wire. 

Two examples come to mind. VLAN, as we know has a fixed range, and can be easily defined. Same for MPLS labels. The other example is of duplex on a Ethernet port. You can probably set the default to full duplex, and few folks will complain. If the protocol defines it and you can cite a section in the standard where the range/default is defined then you are covered. 

Where there is no clear standards defining ranges, defaults, min/max, I agree with you, it is best to keep it out of a standard model.

Cheers.

> On May 27, 2015, at 10:53 PM, Anees Shaikh <[email-masked]> wrote:
> 
> That said, where it's easy to add validation, we have tried to include it, though ranges, defaults, min/max values, etc. get harder when trying to build something vendor-neutral.

Mahesh Jethanandani
[email-masked]

---

From: Anees Shaikh <[email-masked]>
Date: Thu, 28 May 2015 20:55:11 +0000

Right, those examples are relatively easy because they are defined by a
standard or widely used common settings. We try to do this in the
OpenConfig models but there are plenty of examples where every
implementation has chosen a different default, range, etc.

Thanks.
-- Anees
On Thu, May 28, 2015 at 1:18 PM Mahesh Jethanandani <[email-masked]>
wrote:

> Aneesh,
>
> When I look at a model to see where the ranges, defaults, min/max values
> can be defined, I rely on protocol definition and what the expectation is
> of the packet on the wire.
>
> Two examples come to mind. VLAN, as we know has a fixed range, and can be
> easily defined. Same for MPLS labels. The other example is of duplex on a
> Ethernet port. You can probably set the default to full duplex, and few
> folks will complain. If the protocol defines it and you can cite a section
> in the standard where the range/default is defined then you are covered.
>
> Where there is no clear standards defining ranges, defaults, min/max, I
> agree with you, it is best to keep it out of a standard model.
>
> Cheers.
>
> That said, where it's easy to add validation, we have tried to include it,
> though ranges, defaults, min/max values, etc. get harder when trying to
> build something vendor-neutral.
>
>
> Mahesh Jethanandani
> [email-masked]
>
>
>
>