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

"Ogaki, Kenichi" <ke-oogaki@kddi.com> Tue, 02 May 2017 06:30 UTC

Return-Path: <ke-oogaki@kddi.com>
X-Original-To: l3sm@ietfa.amsl.com
Delivered-To: l3sm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8B9A2129AE5 for <l3sm@ietfa.amsl.com>; Mon, 1 May 2017 23:30:37 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.202
X-Spam-Level:
X-Spam-Status: No, score=-4.202 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v3zOyx2c8yYh for <l3sm@ietfa.amsl.com>; Mon, 1 May 2017 23:30:33 -0700 (PDT)
Received: from post-send.kddi.com (athena2.kddi.com [27.90.165.195]) by ietfa.amsl.com (Postfix) with ESMTP id 738BF12EB0A for <l3sm@ietf.org>; Mon, 1 May 2017 23:27:33 -0700 (PDT)
Received: from LTMC2121.kddi.com (LTMC2121.kddi.com [10.206.0.61]) by post-send.kddi.com (KDDI Mail) with ESMTP id 650DFE0055; Tue, 2 May 2017 15:27:32 +0900 (JST)
Received: from LTMC2146.kddi.com ([10.206.0.236] [10.206.0.236]) by LTMC2121.kddi.com with ESMTP; Tue, 2 May 2017 15:27:32 +0900
Received: from LTMC2146.kddi.com (localhost [127.0.0.1]) by localhost.kddi.com (Postfix) with ESMTP id 4029E30005D; Tue, 2 May 2017 15:27:32 +0900 (JST)
Received: from LTMC2151.kddi.com (post-incheck [10.206.0.239]) by LTMC2146.kddi.com (Postfix) with ESMTP id 352E5300050; Tue, 2 May 2017 15:27:32 +0900 (JST)
Received: from LTMC2151.kddi.com (localhost.localdomain [127.0.0.1]) by LTMC2151.kddi.com with ESMTP id v426RV30023263; Tue, 2 May 2017 15:27:32 +0900
Received: from LTMC2151.kddi.com.mid_19140134 (localhost.localdomain [127.0.0.1]) by LTMC2151.kddi.com with ESMTP id v426HSEA015241; Tue, 2 May 2017 15:17:28 +0900
X-SA-MID: 19140134
Received: from KDDI1508PC0003 ([10.200.122.178] [10.200.122.178]) by post-smtp2.kddi.com with ESMTPA; Tue, 2 May 2017 15:17:28 +0900
From: "Ogaki, Kenichi" <ke-oogaki@kddi.com>
To: "'Jan Lindblad (jlindbla)'" <jlindbla@cisco.com>, l3sm@ietf.org
References: <30844BBA-3D8A-4581-97D3-4CFA7B305B93@cisco.com> <53E535EA-9A19-4D17-A81F-C0AB45515108@cisco.com>
In-Reply-To: <53E535EA-9A19-4D17-A81F-C0AB45515108@cisco.com>
Date: Tue, 02 May 2017 15:17:28 +0900
Message-ID: <027201d2c30b$c603cc80$520b6580$@kddi.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AQD5EjI3C9st5tsIP29axDx172SKOAG16Ujyo4Y/VIA=
Content-Language: ja
X-TM-AS-MML: disable
Archived-At: <https://mailarchive.ietf.org/arch/msg/l3sm/WzdKYngvksBWp8CpxCdGgURWBZc>
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery
X-BeenThere: l3sm@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: L3VPN Service YANG Model discussion group <l3sm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/l3sm>, <mailto:l3sm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/l3sm/>
List-Post: <mailto:l3sm@ietf.org>
List-Help: <mailto:l3sm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/l3sm>, <mailto:l3sm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 02 May 2017 06:30:38 -0000

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:l3sm-bounces@ietf.org] On Behalf Of Jan Lindblad (jlindbla)
Sent: Friday, April 21, 2017 11:49 PM
To: l3sm@ietf.org
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.
The current example can cover the change without any modification.


#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.


#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> , 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?



#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.


#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.

   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.


#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.


#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.


#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> ].  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?


#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/


#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?


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



	 
	From: Benoit Claise [mailto:bclaise@cisco.com] 
	Sent: 24 March 2017 11:48
	To: Jan Lindblad
	Cc: l3sm-chairs@ietf.org; Benoit Claise
	Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery
	 
	On 3/24/2017 10:25 AM, Jan Lindblad wrote:

		Benoît,  
		 

			See below 
			> If we decide to go to publish an errata, it would be good to have a second YANG doctor full review on the model to ensure we are not missing something else. 
			
			So this is a more official request :-)
			I know you spent some time on this module already...
			Fine?

		 
		Sure. I have already demonstrated interest in the topic, I guess, so hard to get away now ;-) 
		So would you like me to review (even deeper) the current version or an upcoming version with fixes in places?

	Yes please.
	
	Regards, Benoit
	
	
	 
	/jan
	--
	Jan Lindblad, janl@tail-f.com, +46 702855728
	Solutions Architect, Business Development, Tail-f
	Tail-f is now a part of Cisco
	 

		
		Regards, Benoit
		-------- Forwarded Message -------- 
Subject: 
RE: FW: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery
Date: 
Tue, 21 Mar 2017 08:27:06 +0000
From: 
stephane.litkowski@orange.com
To: 
Benoit Claise <bclaise@cisco.com> <mailto:bclaise@cisco.com> 
CC: 
Ogaki, Kenichi (ke-oogaki@kddi.com) <ke-oogaki@kddi.com> <mailto:ke-oogaki@kddi.com> 

		 

		Taking into account the YANG errors that have been found, I think it could make sense.
		 
		-----Original Message-----
		From: Benoit Claise [mailto:bclaise@cisco.com] 
		Sent: Monday, March 20, 2017 18:03
		To: LITKOWSKI Stephane OBS/OINIS
		Cc: Ogaki, Kenichi (ke-oogaki@kddi.com)
		Subject: Re: FW: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery
		 
		On 3/20/2017 4:55 PM, stephane.litkowski@orange.com wrote:
		> Hi Benoit,
		> 
		> If we decide to go to publish an errata, it would be good to have a second YANG doctor full review on the model to ensure we are not missing something else.
		> 
		> What do you think ?
		That makes sense.
		Note that Jan is a YANG doctor.
		Do we want to ask officially for a YANG doctor review?
		 
		Regards, Benoit
		> 
		> Best Regards,
		> 
		> 
		> -----Original Message-----
		> From: L3sm [mailto:l3sm-bounces@ietf.org] On Behalf Of 
		> stephane.litkowski@orange.com
		> Sent: Thursday, March 16, 2017 11:31
		> To: Ogaki, Kenichi; 'Jan Lindblad'; 'Benoit Claise'
		> Cc: l3sm@ietf.org; adrian@olddog.co.uk
		> Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service 
		> Delivery
		> 
		> Hi,
		> 
		> Pls find inline comments.
		> 
		> 
		> -----Original Message-----
		> From: L3sm [mailto:l3sm-bounces@ietf.org] On Behalf Of Ogaki, Kenichi
		> Sent: Thursday, March 16, 2017 10:26
		> To: 'Jan Lindblad'; 'Benoit Claise'
		> Cc: l3sm@ietf.org; adrian@olddog.co.uk
		> Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service 
		> Delivery
		> 
		> Hi Jan, All,
		> 
		> See comments inline [KO], please.
		> Stephane, Luis, could you correct me if I misunderstand?
		> 
		> Thanks,
		> Kenichi
		> 
		> -----Original Message-----
		> From: L3sm [mailto:l3sm-bounces@ietf.org] On Behalf Of Jan Lindblad
		> Sent: Thursday, March 09, 2017 6:48 PM
		> To: Benoit Claise
		> Cc: l3sm@ietf.org; adrian@olddog.co.uk
		> Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service 
		> Delivery
		> 
		> Benoît, all,
		> 
		>> On 9 Mar 2017, at 20:09, Benoit Claise <bclaise@cisco.com> <mailto:bclaise@cisco.com>  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
		> L3sm@ietf.org
		> https://www.ietf.org/mailman/listinfo/l3sm
		> 
		> _______________________________________________
		> L3sm mailing list
		> L3sm@ietf.org
		> 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
		> L3sm@ietf.org
		> 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.