Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

["Jan Lindblad (jlindbla)" <[e-mail]>]

Kenichi,

Hi Jan,

Thank you for your support.

Following your proposal, we try to put "valid-provider-identifiers" and set "config false" under valid-provider-identifiers/cloud-identifier. However, pyang says "error: the key "cloud-identifier"does not have same "config" as its list".
The list cloud-access has some configurable leaves, then we cannot put "config false" there. How should we model this?

That's right. Config true leafrefs must not refer to config false data, or the validity of the configuration would depend on external (non-configuration) factors. That's something that the WG outlawed early on.

So to model this case, there are a couple of different options:

1. Make the valid-provider-identifiers list config true, and set up NACM access control rules to ensure only the provider operators can modify this part of the config.

2. Move the model to YANG 1.1, and add require-instance false; to the cloud-identifier leafref. This makes it possible to point to use any value (valid values, as well as legacy, future or random values). Even if this is a bit lax (allowing anything), it at least gives the reader and client tools a good hint at what valid values there are. We would have to define in the description for the leaf what it means if this leaf points to a no longer/not yet valid value.

3. It is certainly possible make a YANG work around for the configness constraint (I can show you), but I wouldn't recommend this. I believe the same-configness rule exists for a good reason, and we are better off respecting it.

/jan




   grouping vpn-service-cloud-access {
    container cloud-accesses {
    if-feature cloud-access;
    list cloud-access {

     key cloud-identifier;

     leaf cloud-identifier {
      type leafref {
   path "../../../valid-provider-identifiers/cloud-identifier/id";
      }
      description
       "Identification of cloud service.
       Local administration meaning.";
     }
     choice list-flavor {
      case permit-any {
       leaf permit-any {
        type empty;
        description
         "Allows all sites.";
       }
      }
      case deny-any-except {
       leaf-list permit-site {
        type leafref {
         path "/l3vpn-svc/sites/site/site-id";
        }
        description
         "Site ID to be authorized.";
       }
      }
      case permit-any-except {
       leaf-list deny-site {
        type leafref {
         path "/l3vpn-svc/sites/site/site-id";
        }
        description
         "Site ID to be denied.";
       }
      }
      description
       "Choice for cloud access policy.";
     }
     container authorized-sites {
      list authorized-site {
       key site-id;

       leaf site-id {
        type leafref {
         path "/l3vpn-svc/sites/site/site-id";
        }
        description
         "Site ID for each authorized site.";
       }
       description
        "List of authorized sites.";
      }
      description
       "Configuration of authorized sites.";
     }
     container denied-sites {
      list denied-site {
       key site-id;

       leaf site-id {
        type leafref {
         path "/l3vpn-svc/sites/site/site-id";
        }
        description
         "Site ID for each denied site.";
       }
       description
        "List of denied sites.";
      }
      description
       "Configuration of denied sites.";
     }

     container address-translation {
      container nat44 {
       leaf enabled {
        type boolean;
        default false;
        description
         "Controls whether or not Network address
                 translation from IPv4 to IPv4 (NAT44)
                 [RFC3022]is required.";
       }
       leaf nat44-customer-address {
        type inet:ipv4-address;
        must "../enabled = 'true'" {
         description
          "Applicable only if Network address
                  translation from IPv4 to IPv4 is
                  enabled.";
        }
        description
         "Address to be used for network address
                 translation from IPv4 to IPv4. This is
                 to be used if the customer is providing
                 the IPv4 address.";
       }
       description
        "IPv4-to-IPv4 translation.";
      }
      description
       "Container for NAT.";
     }
     description
      "Cloud access configuration.";
    }
     description
      "Container for cloud access configurations.";
    }
    description
     "Grouping for VPN cloud definition.";
   }
...
   grouping vpn-svc-cfg {
    container valid-provider-identifiers {
     list cloud-identifier {
      if-feature cloud-access;
      key id;
      leaf id {
       type string;
description
       "Identification of cloud service.
       Local administration meaning.";
      }
   //config false;
     }
    }
   ...
   }

Thanks,
Kenichi

-----Original Message-----
From: L3sm [mailto:[e-mail]] On Behalf Of [e-mail]<mailto:[e-mail]>
Sent: Wednesday, May 10, 2017 5:56 PM
To: Jan Lindblad (jlindbla); Ogaki, Kenichi
Cc: [e-mail]<mailto:[e-mail]>
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

Hi,



Sorry for the late reply,

Few additional comments inline.



Brgds,





From: L3sm [mailto:[e-mail]] On Behalf Of Jan Lindblad (jlindbla)
Sent: Wednesday, May 03, 2017 14:00
To: Ogaki, Kenichi
Cc: [e-mail]<mailto:[e-mail]>
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery



Kenichi, WG,



Good to hear from you again!

Comments [jan] inline.



Hi Jan,

Thank you for your courtesy review.

See my comments inline [KO], please.
Stephane, Luis, could you correct me if I misunderstand?

Thanks,
Kenichi

-----Original Message-----
From: L3sm [mailto:[e-mail] <mailto:[e-mail]> ] On Behalf Of Jan Lindblad (jlindbla)
Sent: Friday, April 21, 2017 11:49 PM
To: [e-mail]<mailto:[e-mail]> <mailto:[e-mail]>
Subject: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

Since I found some YANG level issues in RFC 8049, the WG chairs asked me to do a full review of the RFC so that the WG should have a complete picture before deciding what to do.

I finally managed to get through this well written RFC. I expect a large part of the audience for this document will be implementors with a strong software, but less strong networking background, much like myself. I find the language here well adapted to this kind of reader, and I enjoyed learning more about this use case, which I am involved with customers on very frequently.

Since I already did a rather thorough examination of the YANG model, and that is where my main expertise lies, I didn't really expect to find much in terms of new issues to comment on by reading the full document. I did find a couple of disturbing disconnects between the text and the model, however, and a a few less concerning possibilities for clarification and improvement.



#1) QoS classification rules

The most disturbing issue I found was in section "6.12.2.1.  QoS Classification". It says:

  The order of rules is very important.  The management system
  responsible for translating those rules in network element
  configuration MUST keep the same processing order in network element
  configuration.  The order of rules is defined by the "id" leaf.  The
  lowest id MUST be processed first.

This is highly problematic for two reasons. First, it contradicts the YANG model. The YANG model looks like this:

 container qos {
  if-feature qos;
  container qos-classification-policy {
   list rule {
    key id;
    ordered-by user;


    leaf id {
     type uint16;
     description
      "ID of the rule.";
    }

Since this list is "ordered-by user", it is expressly *not ordered in numeric order*, but in some other order determined by the user. Removing the "ordered-by user" statement would be one way of making the text and model consistent. Which leads us to the second problem.

Secondly, if numeric order was indeed to be used in this model, we are cementing this inefficient (I will even say stupid) network management bottleneck for another decade or two. The current implementation in many devices are based on 16-bit integers. However, they are often not stable across reboots and operators may renumber at any time. From experience, I can tell that most implementors would not realize that a 1:1 mapping between these service model sequencing numbers directly to the device sequencing numbers is incorrect, and even if properly done is slow and highly error prone in the event of manual device level changes. In fact, this inefficiency/source of error in devices is the number one complaint I get from my customers. To standardize as described in the text would be a horrible mistake, IMO.

I realize, however, that this is an area of debate and opinion. IETF has mostly moved away from this inefficiency in other models. E.g. the qos-model and acl-models referenced below both use ordered-by user, as I find sound.
https://tools.ietf.org/html/draft-asechoud-netmod-qos-model-00
https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-model

In the OpenConfig models, on the other hand, the legacy 16-bit numerical order is still in their YANG models. The only way to reliably handle this case, as far as I can imagine, is for the management system to overwrite all QoS (or ACL) rules for every change. Since that is often a rather heavy operation with no option to include manually introduced changes, my customers are not happy.

In my opinion, this RFC should side with the other IETF documents and keep the "ordered-by user" in the model and update the RFC text to mirror this. I would also recommend that the leaf id changes type from 16-bit integer to string, as keeping 16-bit integers as keys would undoubtedly be an endless source of misunderstanding since "ordered-by user" means they must be processed out of numerical order. The example should also be updated to reflect this.


[KO] I agree.
We can just remove the text "The order of rules is defined by the "id" leaf.  The lowest id MUST be processed first." And, change type from 16-bit integer to string for the leaf id.



[jan] Excellent.



[SLI] I would also propose to change the description of the leaf “id” to tell that this is a description of the rule.





The current example can cover the change without any modification.



[jan] I think the numeric keys in the example should be changed into some strings that are not sorted in alphabetical order, to make the point clear that it is the order they appear in the configuration, not the numeric or alphabetic order, that decides how they should be processed. E.g. rule id:s like "Toyota", "Volvo", "Ford".



[SLI] Make sense





#2) Use of deviations

In sections 6.12.2.1 and  6.12.2.2, there are wordings like this:

  an
  implementation might not support this criterion and should advertise
  a deviation in this case.

I find it very hard to accept that a standards document would say "if this or that, it's ok that you break the standard". IMO, normative text cannot mandate the use of deviations. There are other mechanisms that should be used in situations like this. The most common one would be if-feature, but other approaches are also possible, e.g. modeling a leafref to a config list with the options supported by this implementation, and use access rules to prevent any operator from making changes to that config list.


[KO] We can remove the text, "an implementation might not support this criterion and should advertise a deviation in this case." Then, put "if-feature target-sites;" under the leaf-list target-sites.



[jan] Good.



#3) Hard to understand mix of MUST NOT and SHOULD

In section 6.6, the following is said:

  The management system SHOULD honor any customer constraints.  If a
  constraint is too strict and cannot be fulfilled, the management
  system MUST NOT provision the site and SHOULD provide relevant
  information to the user.

I find the above sentences very hard to understand. Is this a SHOULD or MUST NOT requirement?


[KO] How about this?
  The management system SHOULD honor any customer constraints.  If a
  constraint is too strict and cannot be fulfilled, the management
  system might not provision the site and MUST provide relevant
  information to the user.


Also, further down in section 6.6.5 it says:

  When the management system cannot
  determine the placement of a site-network-access, it SHOULD return an
  error message indicating that placement was not possible.


[KO] In this case, MUST is appropriate.


And in section 7:

  As described in Section 6.6 <https://tools.ietf.org/html/rfc8049#section-6.6 <https://tools.ietf.org/html/rfc8049#section-6.6> > , the management system SHOULD
  use the location information and SHOULD use the access-diversity
  constraint to find the appropriate PE.


[KO] Second SHOULD should be replaced with MUST.


Which might suggest that it's all SHOULD requirements. Perhaps best to remove the MUST NOT in that case?



[jan] I think the proposed text is better than the original, but how about something like with fewer moving parts, like this:



 The management system MUST honor all customer constraints, or if a
 constraint is too strict and cannot be fulfilled, the management
 system MUST NOT provision the site and MUST provide
 information to the user about which constraints that could not be fulfilled.



Or if that is too hard on implementors, how about this easier one



 The management system SHOULD honor all customer constraints.

 Regardless if a service is provisioned or not, the management system

 MUST provide information to the user about which constraints

 that could not be fulfilled.



[SLI] It looks dangerous to provision something  which is not compliant with the customer requirement. I would be in favor of your first text proposal.





#4) Incomplete examples

In section 6.5.2.1 and 6.5.2.2 there are examples which are informative, but omit certain mandatory fields. Omitting aspects that are not in focus at the moment may be a good thing in the interest of the reader, but when such artistic freedom is used, I think it may be in place to point out   that the example is incomplete. I did not check most of the examples in the document, but it would be polite to do so, and either make them real world usable examples or note in the text that they are not.


[KO]I can agree, but we may need a guideline how to write this kind of example.



[jan] There are two ways to fix this. Either add a note that the example is incomplete. It could be as simple as adding "Note that the following example omits some mandatory leafs." near the example. The other alternative would be to add sample values for the missing leafs. All examples should of course be validated against the YANG module anyway, but if you don't have any easy way to do that, I can help.





[SLI] I think the best way would be to add the missing leaves (so the examples can directly be used by anyone). I personally do not have a way to check that the examples are 100% right, if you have any proposal , feel free to help on this.







#5) Only single constraint of each type

Around line 894 in the YANG model, is is possible to model various constraints on the access. The way this is currently modeled, there can be only one constraint of each type. Does this cover all reasonable use cases? For example, currently it's possible for an access link to specify that it needs to be access-diverse with another link. Would it ever make sense to specify two such diversity constraints, i.e. that link A is diverse with respect to both links B and C? This is not possible with the current modeling.

   container constraints {
    list constraint {
     key constraint-type;


     leaf constraint-type {
      type identityref {
       base placement-diversity;
      }
      description
       "Diversity constraint type.";
     }


[KO] As described in 6.6.4 and YANG model, each constraint type can list multiple target groups.



[jan] Right, I see. If this models gives you all the flexibility you need, then it's good.



  The group-id used to target some site-network-accesses may also be
  different than the one used by the current site-network-access.  This
  can be used to express that a group of sites has some constraints
  against another group of sites, but there is no constraint within the
  group.  For example, we consider a set of six sites and two groups;
  we want to ensure that a site in the first group must be pop-diverse
  from a site in the second group:

Then, the above scenario can model like this:

  <site>
   <site-id>SITE1</site-id>
   <site-network-accesses>
    <site-network-access>
     <site-network-access-id>1</site-network-access-id>
     <access-diversity>
      <groups>
       <group>
        <group-id>A</group-id>
       </group>
      </groups>
      <constraints>
       <constraint>
        <constraint-type>pe-diverse</constraint-type>
        <target>
         <group>
          <group-id>B</group-id>
          <group-id>C</group-id>
         </group>
        </target>
       </constraint>
      </constraints>
     </access-diversity>
    </site-network-access>
   </site-network-accesses>
  </site>
  <site>
   <site-id>SITE2</site-id>
   <site-network-accesses>
    <site-network-access>
     <site-network-access-id>1</site-network-access-id>
     <access-diversity>
      <groups>
       <group>
        <group-id>B</group-id>
       </group>
      </groups>
      <constraints>
       <constraint>
        <constraint-type>pe-diverse</constraint-type>
        <target>
         <group>
          <group-id>A</group-id>
         </group>
        </target>
       </constraint>
      </constraints>
     </access-diversity>
    </site-network-access>
   </site-network-accesses>
  </site>
  <site>
   <site-id>SITE3</site-id>
   <site-network-accesses>
    <site-network-access>
     <site-network-access-id>1</site-network-access-id>
     <access-diversity>
      <groups>
       <group>
        <group-id>C</group-id>
       </group>
      </groups>
      <constraints>
       <constraint>
        <constraint-type>pe-diverse</constraint-type>
        <target>
         <group>
          <group-id>A</group-id>
         </group>
        </target>
       </constraint>
      </constraints>
     </access-diversity>
    </site-network-access>
   </site-network-accesses>
  </site>


Also, section 6.6.4 states that:

  These constraint-types can be extended through augmentation.

Since at most one constraint of each type is possible for a given access, that would limit the freedom of expressibility and ingenuity here also for future use cases, I would think. Reading a bit further in that section, I see:

  Each constraint is expressed as "The site-network-access S must be
  <constraint-type> (e.g., pe-diverse, pop-diverse) from these <target>
  site-network-accesses."

To me, the use of plurals make it sound to me as if the author is suggesting to allow more than one diversity constraint, possibly of the same type, for a given access. Which is not possible today.



[KO] As described above, the model can multiple constraint types with multiple targets per each constraint type.



[jan] Good. I also thought about some future use cases where multiple constraints of the same type would be desirable, but they too can be modeled such that multiple groups and additional leafs are augmented under the single constraint type instance. So this looks good.



#6) Maximum bandwidth limit

Around line 1100 in the YANG model, the service's bandwitdh limit is specified by uint32 leaves. This gives a maximum service speed possible to express just over 4Gbps. Is this model constraint future proof enough, or would it make sense to switch types to e.g. uint64?

grouping site-service-basic {
 leaf svc-input-bandwidth {
     type uint32;
     units bps;
     description
      "From the PE's perspective, the service input
      bandwidth of the connection.";
 }
 leaf svc-output-bandwidth {
    type uint32;
    units bps;
    description


[KO] Agree.



[jan] Fine.



[SLI] Oh  yes, what a big mistake here !





#7) Incomplete description statements

In numerous cases, the document text contains details left out from the description statements in the YANG. For overview and background type of information, this may be acceptable and appropriate. For details around the meaning and constraints of YANG model leaves, this information ought to be in the YANG model, IMO. Some examples:

Around line 1596:

  container rip {


   when "../type = 'rip'" {
    description
     "Only applies when protocol is RIP.";
   }
   if-feature rtg-rip;
   leaf-list address-family {
    type address-family;


    description
     "Address family to be activated.";
   }

The text points out that this is about RIPv2. The description statement could+should too.

Section 6.11.7 says:

 BGP activation requires the SP to know the address of the customer
 peer.  The "static-address" allocation type for the IP connection
 MUST be used.

Usage information like this should be mentioned in the YANG model description statements (or even better in must/when statements), not only in RFC text.

Section 6.13.1 says:

 If CsC is enabled, the requested "svc-mtu" leaf will refer to the
 MPLS MTU and not to the IP MTU.

Should be mentioned in the description statement.

I believe there are many more instances of this, but these are the ones I took note of.



[KO] I can agree, but we may need a guideline how to write this kind of description.



[jan] This is a general comment that applies to a lot of text in the document, but I'll try to explain what I mean by a couple of examples:



description

     "RIP routing specific

     configuration.";



could be changed to



description

     "RIPv2 routing specific

     configuration.";



And



  description

   "MTU at service level.

   If the service is IP,

   it refers to the IP MTU.";



could become



  description

   "MTU at service level.

   If the service is IP,

   it refers to the IP MTU.

   If CsC is enabled, it will refer

   to the MPLS MTU";



And



    description

     "BGP specific configuration.";



could become



    description

     "BGP specific configuration.

      BGP activation requires the SP to know the address of the customer

      peer.  The "static-address" allocation type for the IP connection
      MUST be used.";



Or even a must statement with this effect.



#8) When statement appropriate

In section 6.3.2.2.1 it is mentioned:

  o  slaac: This parameter enables stateless address autoconfiguration
     [RFC4862 <https://tools.ietf.org/html/rfc4862 <https://tools.ietf.org/html/rfc4862> > ].  This is applicable to IPv6 only.

The YANG model has no check that slaac parameters are used only for IPv6. Maybe that could be added?


[KO] Agree. How do we express this condition?



[jan] For example, it could be done like this



  container ipv4 {

   if-feature ipv4;

   leaf address-allocation-type {

    must "current() != 'slaac'" {

      error-message "SLAAC is only applicable to IPv6";

    }

    type identityref {

     base address-allocation-type;

    }



Another way would be to express it by changing the identity tree.



identity address-allocation-type {

 description

  "Base identity for address-allocation-type

  for PE-CE link.";

}

identity ipv4-address-allocation-type {

 base address-allocation-type;

 description

  "Base identity for ipv4-address-allocation-type

  for PE-CE link.";

}

identity provider-dhcp {

 base ipv4-address-allocation-type;

 description

  "Provider network provides DHCP service to customer.";

}

identity slaac {

 base address-allocation-type;

 description

  "Use IPv6 SLAAC.";

}



...



  container ipv4 {

   if-feature ipv4;

   leaf address-allocation-type {

    type identityref {

     base ipv4-address-allocation-type;

    }



...



  container ipv6 {

   if-feature ipv6;

   leaf address-allocation-type {

    type identityref {

     base address-allocation-type;



[SLI] I’m more in favor of your first option





#9)  Strange wording

In section 6.11.7 it says:

 It will
 be up to the SP and management system to determine how to decline the
 configuration

Is decline really the right word here? If so, I don't understand.


[KO] Maybe typo. s/decline/describe/



[jan] Makes sense.





#10) SP identifiers available to customers

Section 6.14 notes:

 How an SP provides the meanings of those identifiers to the customer
 is out of scope for this document.

It would be easy to add YANG modeling to convey this information. Doing so would enable proper use of leafrefs rather than plain strings that are communicated in an ad-hoc fashion.


[KO] Could you show us an example how to leafref an id defined outside of this model?



[jan] All I'm saying is that it's easy to add a list with the identifiers in use. The merit would be fewer mistakes, and might make it easier for customers and providers alike to notice (by YANG validation) if they refer to an identifier that is no longer valid. Perhaps like this:





 list cloud-access {

  if-feature cloud-access;

  key cloud-identifier;



  leaf cloud-identifier {

   type leafref "/valid-provider-identifiers/cloud-identifier/id"; // was type string;

   description

    "Identification of cloud service. Local

    admin meaning.";

  }



...



// This part of the model is configured by the SP, read-only for customers.

container valid-provider-identifiers {

list cloud-identifier {

 key id;

 leaf id {

  type string;

 }

}

}



This sort of table could of course be placed in a separate module if desired.



[SLI] I agree that this could make sense, and helps correctness of the information used by the customer. If we go this way, we will need to replicate this for multiple datas like Qos profiles…

Moreover these parameters may not be linked only to this model that was the first reason on the current modeling proposal.





These findings add to the YANG-level comments I made earlier. I hope this was the kind of review you were looking for.

Best Regards,
/jan



[jan] I know the YANG level comments I made earlier have already been discussed (see below), let's not forget to include them. To my knowledge, the note I made about many elements being optional with no explanation was not addressed, however. It would probably make sense to go through them, make some mandatory, add a default or description for others.



Best Regards,

/jan



> Benoît, all,
>
>> On 9 Mar 2017, at 20:09, Benoit Claise <[e-mail]<mailto:[e-mail]> <mailto:[e-mail]> > <mailto:[e-mail] <mailto:[e-mail]> >  wrote:
>>
>> Dear all,
>>
>> It seems like Jan discovered a bug.
>> Can the authors confirm?
> Since the issue I found was in a quick scan, I decided to go through the entire document in a little more ordered fashion. My findings are below. Issue #1 is what I reported earlier. I am truly sorry I did not go through the YANG before the release, but there isn't much to do about that now.
>
>
> I was interested in the final result, so I took a quick look at the model. Within a couple of minutes I stumbled upon this, which is almost certainly not what the author had intended.
>
> #1)
>
> 1995: grouping site-devices /l3vpn-svc/sites/site
>    container devices {
>     must "/l3vpn-svc/sites/site/management/type = "+
>      "'provider-managed' or "+
>      "/l3vpn-svc/sites/site/management/type ="+
>      "'co-managed'" {
>
> To me, it appears that the author intended to say that the "container device" configuration is only applicable when the current site management type is 'provider-managed' or 'co-managed'. That would make sense.
>
> The use of "must" and the use of the absolute paths above changes the meaning in YANG to "as soon as any l3vpn is configured, there must be at least one l3vpn (not necessarily the current one) that is 'provider-managed' or 'co-managed'." One implication would be that it would not be legal to have a collection of only 'customer-managed' vpns. What would be legal, though, is to have one 'provider-managed' l3vpn with no device configuration and another 'customer-managed' with devices configured. I expect this was not the intention.
>
> I think an expression that reflects the intent would be
>
>    container devices {
>     when "../management/type = "+
>      "'provider-managed' or "+
>      "/l3vpn-svc/sites/site/management/type ="+
>      "'co-managed'" {
>
> This would make the container only exist when the management type of this site specifically is 'provider-managed' or 'co-managed'.
>
> [KO] Martin's following modification seems correct.
> [SLI] I also agree with Martin's proposal.
>
>>    container devices {
>>     when "../management/type = 'provider-managed' or "
>>        + "../management/type = 'co-managed'" {
>
> Now I went through the YANG and discovered a couple more similar things, as well as a slew of optional leafs where the meaning of a non-existent leaf is undefined.
>
> #2)
>
> 2011: grouping site-devices /l3vpn-svc/sites/site/devices/device/
>      leaf location {
>       type leafref {
>        path "/l3vpn-svc/sites/site/locations/"+
>         "location/location-id";
>
> The location and name of the grouping in which 'leaf location' is
> defined makes me believe the intent is to only allow pointers to
> locations within the current site. Currently this leafref allows
> pointing to any location in any site (as well as no reference at all).
> If my assumption about the modeler's intent is right, the path should
> be
>
>        path "../../../locations/location/location-id";
>
> [KO] Basically agree. However, a service provider may allow his/her customer to use two or more l3vpn-svc instances and refer their parameters from each other.
>
> [SLI] I would tend to limit to the objects created in the current site.
>
>
> #3)
>
> On line 2019 there is another must statement which wants to be a when, and the path is a little too liberal, I'd think.
>
> 2019: grouping site-devices /l3vpn-svc/sites/site/devices/device
>      container management {
>       must "/l3vpn-svc/sites/site/management/type"+
>        "= 'co-managed'" {
>
> I think the modeler's intent should be
>
>       must "../../../management/type = 'co-managed'" {
>
> [KO] Agree.
> [SLI] Agree.
>
>
> #4)
>
> There's probably nothing wrong with this one, but since my confidence
> in guessing the modeler's intent is low here, and there are many
> previous leafref mistakes in this module, maybe someone should assert
> that this is what's intended
>
> 2139: grouping site-vpn-policy /l3vpn-svc/sites/site
>        leaf vpn-id {
>         type leafref {
>          path "/l3vpn-svc/vpn-services/vpn-svc/vpn-id";
>         }
>
> [OK] The current path is correct. Not "vpn-svc", but "vpn-service".
>
> #5)
>
> Again, my guess is that the modeler would like to allow references to site-network-access objects from within the current site rather than in any site.
>
> 2395:/l3vpn-svc/sites/site/site-network-accesses/site-network-access grouping access-vpn-policy
>       leaf vpn-policy-id {
>        type leafref {
>         path "/l3vpn-svc/sites/site/"+
>         "vpn-policy-list/vpn-policy/"+
>         "vpn-policy-id";
>        }
>
> If so, the leafref path should be
>
>         path "../../../vpn-policy-list/vpn-policy/vpn-policy-id";
>
> [KO] We need one more "../" and s/vpn-policy-list/vpn-policies/ .
>         path "../../../../vpn-policies/vpn-policy/vpn-policy-id";
>
>
> #6)
>
> Assuming I'm right in my suspicion that the current module has errors as described above, I'd say they are grave enough to require fixing. On a far lower severity, there are lot's of leafs and choices which are optional, but declare nothing about any meaning if absent. Probably many of them should be made mandatory or have a default. Most of the rest should probably have a sentence or two added to the description. Below I have collected the line numbers and optional element names where I honestly don't know what the system would do if left without a value. It may be that l3vpn experts would know in some cases, but it wouldn't hurt to spell out.
>
> Not fixing these undefined cases limits the interoperability, IMO.
>
> 629: leaf nat-enabled
> 653: choice group-format
> 757: leaf rp-address
> 966: choice target-flavor
> 1149:leaf svc-input-bandwidth
> 1156:leaf svc-output-bandwidth
> 1163:leaf svc-mtu
> 1177:leaf enabled
> 1194:leaf signalling-type
> 1237:choice match-type
> 1272:choice qos-profile
> 1362:leaf enabled
> 1367:leaf layer
> 1382:choice profile
> 1504:leaf area-address
> 1509:leaf metric
> 1526:leaf metric
> 1550:leaf autonomous-system
> 1555:leaf-list address-family
> 1639:leaf-list address-family
> 1662:leaf-list address-family
> 1742:leaf provider-address
> 1747:leaf customer-address
> 1752:leaf mask
> 1803:leaf-list server-ip-address
> 1821:leaf provider-address
> 1826:leaf customer-address
> 1831:leaf mask
> 1850:leaf bfd-enabled
> 1856:choice holdtime
> 1979:leaf type
> 2052:leaf site-vpn-flavor
> 2087:choice lan
> 2280:leaf site1
> 2285:leaf site2
> 2311:leaf src-site
> 2316:leaf dst-site
> 2349:leaf local-sites-role
> 2406:leaf vpn-id
>
>
> Best Rergards,
> /jan
>
> _______________________________________________
> L3sm mailing list
> [e-mail]<mailto:[e-mail]> <mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/l3sm <https://www.ietf.org/mailman/listinfo/l3sm>
>
> _______________________________________________
> L3sm mailing list
> [e-mail]<mailto:[e-mail]> <mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/l3sm <https://www.ietf.org/mailman/listinfo/l3sm>
>
> ______________________________________________________________________
> ___________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
> This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>
> _______________________________________________
> L3sm mailing list
> [e-mail]<mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/l3sm
>
> ______________________________________________________________________
> ___________________________________________________
>
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc pas etre diffuses,
> exploites ou copies sans autorisation. Si vous avez recu ce message
> par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
>
> This message and its attachments may contain confidential or
> privileged information that may be protected by law; they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.



_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.