Subject: Re: [L3sm] draft-zaalouk-supa-vpn-service-management-model

From: "Liushucheng (Will)" <[email-masked]>
Date: Tue, 17 Mar 2015 12:52:37 +0000

Hi authors of supa-vpn-service-mgmt-model,

Just subscribed to l3sm and read the mailing list, I found there is a kindly email from Jan Lindblad (copied) who gave a set of detailed comments. I guess maybe you didn't follow l3sm list so I copy them here below.

Regards,
Will

Re: [L3sm] draft-zaalouk-supa-vpn-service-management-model
________________________________

  *   From: Jan Lindblad <janl at tail-f.com<mailto:[email-masked]>>
  *   To: Benoit Claise <bclaise at cisco.com<mailto:[email-masked]>>
  *   Cc: l3sm at ietf.org<mailto:[email-masked]>
  *   Date: Wed, 11 Mar 2015 14:23:31 +0100
  *   In-reply-to: <[email-masked]<http://www.ietf.org/mail-archive/web/l3sm/current/msg00003.html>>
  *   References: <[email-masked]<http://www.ietf.org/mail-archive/web/l3sm/current/msg00003.html>>
  *   List-id: L3VPN Service YANG Model discussion group <l3sm.ietf.org>

________________________________
FYI. http://tools.ietf.org/id/draft-zaalouk-supa-vpn-service-management-model-02.txt

I took a little while to review this model from a YANG Doctor point of view. This model seems pretty good from a YANG perspective, I think.

ietf-supa-l3vpn.yang:
- list l3vpn-Instance; suggest name change to "l3vpn-instance" to conform to IETF naming conventions
- it is customary to surround the top level list with a container, e.g. container l3vpn-instances
- max-elements 65535; // to be discussed - what is the point with this limit?
- leaf instance-name pattern; why is this pattern here?
- mandatory true; on a key leaf is superfluous, suggest removing it
- leaf servic-type; should presumably be "service-type"?

- list access-interface-id; somewhat odd name for a list, wouldn't "access-interface" be more appropriate?
- max-elements 65535 - what is the point with this limit?
- leaf access-interface-id; should this be a reference to the interface table in ietf-interfaces.yang?
- mandatory true; on a key leaf is superfluous, suggest removing it
- leaf access-interface-address, ip-address-mask-length, maybe replace with "ip-prefix" of type "inet:ip-prefix"? This type allows both IPv4 and IPv6 prefixes (IP/mask)
- leaf role; since this leaf seems relevant only for hub-spoke service types, maybe add a when-_expression_?   when "../../service-type = 'hub-spoke'";

- leaf user-name, leaf user-password; I seriously doubt that we want to have device management information in the l3vpn model. That kind of information should be stored in a separate model, or left out of scope. It's not the customer defining the VPN that should fill in these credentials.
- leaf physical-node-id; I think this should be a reference to another module where the managed physical nodes are listed. An arbitrary string value will not do.
- leaf physical-access-interface; I think this should be a reference to the list of interfaces on that device. An arbitrary string value will not do. Also, is the the interface referenced always going to be a "physical" interface? How does this leaf differ from the leaf access-interface-id above? Maybe a clarifying description would help.

- container protocol; the description is a little short; expand or remove.
- leaf protocol-type; are the only relevant protocols ever going to be "bgp", "ospf", "isis"? If this set could grow over time, potentially, then an identity-ref might be a better idea. This way it's easy to define a module with additional alternatives in the future. Even proprietary ones.
- container igp-attribute and container bgp-attribute would probably be better integrated with the protocol-type, perhaps making the whole construct a choice between igp and egp protocols, with an identityref inside (if it is conceivable that the set of protocols could grow). That would also remove the current issue with strange default "0" statements.
- leaf remote-peer-address is currently defined as mandatory in such a way that it has to be specified even if the selected protocol is an igp protocol. Reworking the protocol-type into a choice would fix that.

ietf-supa-ddc.yang:
- container ddc-service; suggest name "ddc-services" to conform to conventions
- leaf name; mandatory true; superfluous for a key, suggest to remove this
- leaf tenant-name; mandatory true; if an operator wants to leave this blank, is that a problem?
- leaf-list dc-name; should this be a reference to a list of data centers? keeping this as a string is weak
- leaf-list interface-name; modeling this as a leaf-list of strings seems very weak to me. Could we turn this into references of some kind?

- leaf connection-type; suggest "L2VPN" is named "l2vpn", "L3VPN" is named "l3vpn", why is native-ipv4 described as "L4VPN"?
- leaf connection-name; this should be a reference to the l3vpn instance, type leafref { path "/l3vpn:l3vpn-instance/instance-name"; }
- leaf vlanId; suggest name "vlan-id"; description could tell what happens if vlan-id is not set
- leaf bandWidth; suggest name "bandwidth" or "speed"; would be good to use a larger type, e.g. uint64; would be good to specify unit, e.g. units "bps";
- leaf latency; is this something that is configured? would be good to specify unit, e.g. units "ns";

In other l3vpn models I have seen, there have usually been some qos configurations as well. Would we want to add that?

/Jan Lindblad

---

From: JF Tremblay <[email-masked]>
Date: Tue, 17 Mar 2015 10:47:43 -0400

> On Mar 17, 2015, at 9:37 AM, Dan Martin <[email-masked]> wrote:
> 
> the Route Distinguisher /Route Target field, the way different VRF instances find out they are members of the same VPN,  is 64bits, so I can’t have more than 65535 MPLS VPNs.

I’m sorry, but I’m not sure I followed your thought process here, Dan. How do you go from 64 bits to 65535 VPNs?

JF

> Does it make sense to add a string that would contain a customer id/customer site id?  One of the problems I have when a VRF instance goes down is connecting it to a particular customer’s service.  Today I have to take some unique to that VRF string and drag it through a provisioning store to find the customer id, then I have to drag that id through a customer data store to find out stuff about the customer and the customer site.  This doesn’t need to be faster, I’m going to open a trouble ticket for the customer and send it to them or update some portal or both, but it would save me the work of plumbing and maintaining a couple connections to a couple data stores if I had that field populated.
>

---

From: Dan Martin <[email-masked]>
Date: Tue, 17 Mar 2015 13:37:14 +0000

A question answered, a question asked

the Route Distinguisher /Route Target field, the way different VRF instances find out they are members of the same VPN,  is 64bits, so I can't have more than 65535 MPLS VPNs.

Does it make sense to add a string that would contain a customer id/customer site id?  One of the problems I have when a VRF instance goes down is connecting it to a particular customer's service.  Today I have to take some unique to that VRF string and drag it through a provisioning store to find the customer id, then I have to drag that id through a customer data store to find out stuff about the customer and the customer site.  This doesn't need to be faster, I'm going to open a trouble ticket for the customer and send it to them or update some portal or both, but it would save me the work of plumbing and maintaining a couple connections to a couple data stores if I had that field populated.

From: Supa [mailto:[email-masked]] On Behalf Of Liushucheng (Will)
Sent: Tuesday, March 17, 2015 8:53 AM
To: [email-masked]
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: Re: [Supa] [L3sm] draft-zaalouk-supa-vpn-service-management-model

Regards,
Will

ietf-supa-l3vpn.yang:
- max-elements 65535; // to be discussed - what is the point with this limit?  - leaf instance-name pattern; why is this pattern here?

ietf-supa-ddc.yang:

/Jan Lindblad