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

From: Benoit Claise <[email-masked]>
Date: Thu, 9 Mar 2017 10:09:41 +0100

Dear all,

It seems like Jan discovered a bug.
Can the authors confirm?

Regards, Benoit
> A great achievement that I have long awaited, for sure!
>
> I have some concerns with the resulting model, however, and I now (obviously) regret not looking at this earlier and taking part in the review process.
>
> At ietf-l3vpn-svc.yang:2053 I find:
>
> grouping site-devices {
>    container devices {
>     must "/l3vpn-svc/sites/site/management/type = "+
>      "'provider-managed' or "+
>      "/l3vpn-svc/sites/site/management/type = "+
>      "'co-managed'" {
>       description
>        "Applicable only for provider-managed or co-managed device.";
>      }
>
> 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.
>
> This was something I noticed after spending a few minutes reading the model. Perhaps a YANG doctor review would be appropriate?
>
> Best Regards,
> /jan
>
>
>
>> On 23 Feb 2017, at 11:15, Adrian Farrel <[email-masked]> wrote:
>>
>> Thanks to all of you for helping to complete this work.
>> Special thanks (of course) to the authors and contributors.
>> And massive thanks to Stephane who was relentless in making the minor edits for
>> the many different phases of "final" review.
>>
>> Adrian
>>
>>> -----Original Message-----
>>> From: L3sm [mailto:[email-masked]] On Behalf Of rfc-editor@rfc-
>>> editor.org
>>> Sent: 23 February 2017 00:26
>>> To: [email-masked]; [email-masked]
>>> Cc: [email-masked]; [email-masked]; [email-masked]
>>> Subject: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery
>>>
>>> A new Request for Comments is now available in online RFC libraries.
>>>
>>>
>>>         RFC 8049
>>>
>>>         Title:      YANG Data Model for L3VPN
>>>                     Service Delivery
>>>         Author:     S. Litkowski, L. Tomotaki, K. Ogaki
>>>         Status:     Standards Track
>>>         Stream:     IETF
>>>         Date:       February 2017
>>>         Mailbox:    [email-masked],
>>>                     [email-masked],
>>>                     [email-masked]
>>>         Pages:      157
>>>         Characters: 272974
>>>         Updates/Obsoletes/SeeAlso:   None
>>>
>>>         I-D Tag:    draft-ietf-l3sm-l3vpn-service-model-19.txt
>>>
>>>         URL:        https://www.rfc-editor.org/info/rfc8049
>>>
>>>         DOI:        10.17487/RFC8049
>>>
>>> This document defines a YANG data model that can be used for
>>> communication between customers and network operators and to deliver
>>> a Layer 3 provider-provisioned VPN service.  This document is limited
>>> to BGP PE-based VPNs as described in RFCs 4026, 4110, and 4364.  This
>>> model is intended to be instantiated at the management system to
>>> deliver the overall service.  It is not a configuration model to be
>>> used directly on network elements.  This model provides an abstracted
>>> view of the Layer 3 IP VPN service configuration components.  It will
>>> be up to the management system to take this model as input and use
>>> specific configuration models to configure the different network
>>> elements to deliver the service.  How the configuration of network
>>> elements is done is out of scope for this document.
>>>
>>> This document is a product of the L3VPN Service Model Working Group of the
>>> IETF.
>>>
>>> This is now a Proposed Standard.
>>>
>>> STANDARDS TRACK: This document specifies an Internet Standards Track
>>> protocol for the Internet community, and requests discussion and suggestions
>>> for improvements.  Please refer to the current edition of the Official
>>> Internet Protocol Standards (https://www.rfc-editor.org/standards) for the
>>> standardization state and status of this protocol.  Distribution of this
>>> memo is unlimited.
>>>
>>> This announcement is sent to the IETF-Announce and rfc-dist lists.
>>> To subscribe or unsubscribe, see
>>>   https://www.ietf.org/mailman/listinfo/ietf-announce
>>>   https://mailman.rfc-editor.org/mailman/listinfo/rfc-dist
>>>
>>> For searching the RFC series, see https://www.rfc-editor.org/search
>>> For downloading RFCs, see https://www.rfc-editor.org/retrieve/bulk
>>>
>>> Requests for special distribution should be addressed to either the
>>> author of the RFC in question, or to [email-masked].  Unless
>>> specifically noted otherwise on the RFC itself, all RFCs are for
>>> unlimited distribution.
>>>
>>>
>>> The RFC Editor Team
>>> Association Management Solutions, LLC
>>>
>>>
>>> _______________________________________________
>>> L3sm mailing list
>>> [email-masked]
>>> https://www.ietf.org/mailman/listinfo/l3sm
>> _______________________________________________
>> L3sm mailing list
>> [email-masked]
>> https://www.ietf.org/mailman/listinfo/l3sm
>>
> _______________________________________________
> L3sm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/l3sm
> .
>

---

From: Jan Lindblad <[email-masked]>
Date: Thu, 9 Mar 2017 20:47:57 +1100

Benoît, all,

> On 9 Mar 2017, at 20:09, Benoit Claise <[email-masked]> wrote:
> 
> Dear all,
> 

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'" {

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

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";

#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'" {

#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";
       }

#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";

#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

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 09 Mar 2017 13:55:18 +0100 (CET)

Hi,

> > It seems like Jan discovered a bug.

I'm not an author, but I agree with Jan.

[...]

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

It needs one more modification:

   container devices {
    when "../management/type = 'provider-managed' or "
       + "../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'.

/martin

---

From: "Ogaki, Kenichi" <[email-masked]>
Date: Thu, 16 Mar 2017 18:25:40 +0900

Hi Jan, All,

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

Thanks,
Kenichi

-----Original Message-----
From: L3sm [mailto:[email-masked]] On Behalf Of Jan Lindblad
Sent: Thursday, March 09, 2017 6:48 PM
To: Benoit Claise
Cc: [email-masked]; [email-masked]
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

Benoît, all,

#1)

  container devices {

[KO] Martin's following modification seems correct.

#2)

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

#3)

[KO] Agree.

#4)

[OK] The current path is correct. Not "vpn-svc", but "vpn-service".

#5)

[KO] We need one more "../" and s/vpn-policy-list/vpn-policies/ .
       path "../../../../vpn-policies/vpn-policy/vpn-policy-id";

#6)

629: leaf nat-enabled
653: choice group-format
757: leaf rp-address

Best Rergards,
/jan

---

From: <[email-masked]>
Date: Thu, 16 Mar 2017 10:31:21 +0000

Hi,

Pls find inline comments.

-----Original Message-----
From: L3sm [mailto:[email-masked]] On Behalf Of Ogaki, Kenichi
Sent: Thursday, March 16, 2017 10:26
To: 'Jan Lindblad'; 'Benoit Claise'
Cc: [email-masked]; [email-masked]
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

Hi Jan, All,

Thanks,
Kenichi

-----Original Message-----
From: L3sm [mailto:[email-masked]] On Behalf Of Jan Lindblad
Sent: Thursday, March 09, 2017 6:48 PM
To: Benoit Claise
Cc: [email-masked]; [email-masked]
Subject: Re: [L3sm] RFC 8049 on YANG Data Model for L3VPN Service Delivery

Benoît, all,

#1)

  container devices {

[KO] Martin's following modification seems correct.
[SLI] I also agree with Martin's proposal.

#2)

[SLI] I would tend to limit to the objects created in the current site.

#3)

[KO] Agree.
[SLI] Agree.

#4)

#5)

#6)

629: leaf nat-enabled
653: choice group-format
757: leaf rp-address

Best Rergards,
/jan

_________________________________________________________________________________________________________________________

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.