Early Review of draft-ietf-l3sm-l3vpn-service-model-06
review-ietf-l3sm-l3vpn-service-model-06-yangdoctors-early-heron-2017-03-06-00

Some comments from a YANG-doctor perspective

a couple of meta-issues first - these are perhaps things the YANG Doctors need
to discuss amongst themselves.

1) I tend to the view that service models should be augments of the I2RS
network and topology model rather than being standalone models.

2) I wonder if the IETF should really be focussing on the interface between the
“service component” and the “config component” in your draft.  at that layer we
can look at e.g. route targets and route distinguishers so we can focus on IETF
technologies, but still take a network and service-centric view rather than a
device-centric one.    The true technology-neutral “service” layer is being
worked on in e.g. MEF - certainly for L2 and I think for L3 also.   Sure, in
the L3 case we could probably argue it’s more IETF’s expertise than MEF’s, but
I think there’s still value in having both layers, and I think we do need to
make sure IETF and MEF don’t tread on each others’ toes too much.

In terms of more specific comments on the model:

3) the address-families could possibly use an enum rather than identities (as
hopefully v4 and v6 is all we’ll ever have!)

4) again if you think the site-vpn-flavors, transport constraints, management
types, address allocation types, vpn-topologies, multicast tree types,
multicast rp discovery types etc. are definitive lists you could use enums for
them.

5) re the site-roles we only really need hub and spoke if we have any-to-any as
a default.

6) for the cloud-access perhaps the if-feature should be at the cloud-accesses
container level.  given the indentation I suspect you added the container later?

7) for the authorized/denied sites there might be a better way to do that
(using a choice - so you have either one list or the other).

8) nat-enabled could maybe be a presence container with customer-nat-address as
an optional leaf inside it (since the customer nat address only applies if nat
is enabled)

9) again the multicast container inside the vpn-service-multicast could be a
presence container indicating that multicast is enabled and then have other
various containers as sub-containers within that.

10) tree-flavor could be a leaf-list (it only has one leaf in it).

11) again provider-managed could be a presence container instead of having an
enabled leaf.  that also avoids the when statements for the other leaves.

12) bsr-candidate could be a leaf-list (only one leaf in the list).

13) you could use an empty presence container rather than a leaf for
carrrierscarrier.

14) the customer location info feels like it may be at too high a level for a
service model.

15) do you need two levels of container in the site-diversity grouping?

16) the groups container is used in both site-diversity and access-diversity. 
So you could use a grouping for it.   In fact you could probably just have the
group list and constraint list - there’s less need for an enclosing container
for a list when the list is already embedded in a container.

17) it might be possible to pull in the flow matching definitions from another
model - they seem fairly generic.

18) the fast-reroute stuff could again use a presence container.

19) site-security-authentication looks kind of empty.

20) site-security-encryption could again use a presence container instead of an
enabled leaf.

21) I’m guessing the layer should be a mandatory leaf for encryption and have a
default?

22) I’m guessing the mask for the static address case ought to be in the range
0..31 for IPv4 and 0..127 for IPv6 (as you can’t have a /32 with 2 addresses in
it).

23) again for BFD you could use a presence container

24) the container "vpn-policy-list" should perhaps be called “vpn-policies”.

25) the list “entries” should probably be called “entry”.

26) ipv4-lan-prefixes and ipv6-lan-prefixes should probably be leaf-lists and
should probably have singular names.

27) the site-role for a VPN site could be made optional if the default was
any-to-any (so you’d only need a role for hubs and spokes).

28) shouldn’t the multicast traffic constraints have a leaf-list for the
dst-site rather than just a leaf? (you’d most likely want the same constraint
for multiple destinations).

29) vpn-svc should probably be called vpn-service for consistency with the
enclosing container.

Giles