Last Call Review of draft-ietf-l3sm-l3vpn-service-model-16
review-ietf-l3sm-l3vpn-service-model-16-genart-lc-carpenter-2016-10-04-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at
<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-l3sm-l3vpn-service-model-16.txt
Reviewer: Brian Carpenter
Review Date: 2016-10-04
IETF LC End Date: 2016-10-11
IESG Telechat date:

Summary: Ready with issues
--------

Comments:
---------

I assume that the minor fixes mentioned in the shepherd's writeup will
be done. I have not checked the details of the yang.

Maror Issues:
-------------

> 5.3.2.2.1.  IP addressing
...
>    o  slaac : enables stateless address autoconfiguration ([RFC4862]).
>      This is applicable only for IPv6.

You can't stop there. Within SLAAC, privacy addresses (RFC4941) may or
may not be allowed by an operator, and opaque addresses (RFC7217)
may be required. So two more Boolean properties are needed.

Also, DHCPv6, SLAAC and static addresses may coexist; they are not
mutually exclusive. I'm not sure if your model allows that.

> 5.12.2.1.  QoS classification

This is too simple. At least, it needs to be able to
handle a port range, not just a single port number.

> 5.12.2.2.  QoS profile

rate-limit, priority-level, and guaranteed-bw-percent may be OK for
MPLS, but they do not capture the needed parameters for differentiated
services. I could write an essay here, but I think the best starting
point is draft-ietf-tsvwg-diffserv-intercon.

Also, I don't understand how you can separate this issue from
Section 5.13.2. Transport constraints, where you do discuss parameters
relevant to diffserv. The whole point about diffserv-intercon is
to quantify and standardise the constraints at interconnections.

I recommend having TSVWG review sections 5.12 and 5.13.

Minor Issues:
-------------

> 5.2.2.  Cloud access
...
>   If NAT is required to access to the cloud, the nat-enabled leaf MUST
>   be set to true.
...
Although NAT is mentioned, I saw no support for NPTv6 (RFC6296). I also
saw no mention of private or shared address space (RFC1918, RFC4193 or RFC6598).

...
> How the restrictions will be configured on network elements is out of
> scope of this document and will be specific to each deployment.

"Each deployment"? I would have thought that this might be uniform for
a given suite of software+hardware implementing the model, or even that
standard practice might emerge with experience. So I suggest to truncate
this sentence:
 How the restrictions will be configured on network elements is out of
 scope of this document.

>   <vpn-svc>
>       <vpn-id>ZKITYHJ054687</vpn-id>
...

Suddenly we have two chunks of XML with no explanation. Why are we using XML?
Should these be captioned as figures? Some text explaining the usage of XML is
missing. Since there are XML configuration fragments in many places later in
the document, this could be in Section 1.1. Terminology.

> 5.3.2.2.1.  IP addressing
>
>   IP subnet can be configured for either transport protocols.  For a
>   dual stack connection, two subnets will be provided, one for each
>   transport layer.

Surely you don't mean 'transport layer'? And I think you mean 'prefix'
rather than 'subnet'.
...
>    o  provider-dhcp : the provider will provide DHCP service for
>      customer equipments, this is applicable to both IPv4 and IPv6
>      addressing.

I find it confusing that you use provider-dhcp for both DHCP and
DHCPv6. They are different protocols. I understand that the usage
is inside container ipv6 {} or container ipv4 {} but it's still
confusing.

> 5.3.2.3.  Inheritance of parameters between site and site-network-access

This section raises more questions than it answers - especially
questions about how the orchestrator works. I suggest adding
a comment along the lines of "out of scope... requires further study."

> 5.6.6.1.  Multihoming
>
>   The customer wants to create a multihomed site.

How do you express, for IPv6, that the customer has multiple IPv6
prefixes, one per ISP? (RFC7157 situation) This is not clear
in section 5.3.2. Site network accesses.

> 5.9.  Security

Don't you want a placeholder for firewall policy elements?

> 5.11.  Routing protocols
>
>   Routing-protocol defines which routing protocol must be activated
>   between the provider and the customer router.  The current model
>   support : bgp, rip, ospf, static, direct, vrrp.

As with DHCP, I find it confusing. There are two BGPs, two RIPs,
and two OSPFs, and using the same name for IPv4 and IPv6 seems wrong.
(VRRPv3 seems to be the same for both IP versions, but do you need
to distinguish it from VRRPv2?).

> 9.  Security Considerations

It would be useful to refer here to Section 5.9. Security.

Nits:
-----

Watch for undefined jargon (VRF is an example).

There are a lot of minor grammatical or typographical errors that make
the text more difficult to read. Quite a lot of work for the RFC
editor. Two examples:

> 5.2.1.2.  Any to any
...
> In the any to any VPN service topology, all VPN sites can discuss
> between each other without any restriction.

The word 'discuss' is badly chosen; I suggest 'communicate' everywhere:

 In the any-to-any VPN service topology, all VPN sites can communicate
 with each other without any restriction.

> It is expected that the
> management system that receives a any to any IPVPN service request
> through this model, needs to assign and then configure the VRF and
> route-targets on the appropriate PEs.

should be

 It is expected that the
 management system that receives an any-to-any IPVPN service request
 through this model needs to assign and then configure the VRF and
 route-targets on the appropriate PEs.