Thanks Joel, There's a lot to digest here. The chairs will work with the authors on a response. Adrian > -----Original Message----- > From: Joel Halpern [mailto:jmh@xxxxxxxxxxxxxxx] > Sent: 25 February 2018 01:00 > To: gen-art@xxxxxxxx > Cc: l2sm@xxxxxxxx; ietf@xxxxxxxx; draft-ietf-l2sm-l2vpn-service-model.all@xxxxxxxx > Subject: Genart telechat review of draft-ietf-l2sm-l2vpn-service-model-08 > > Reviewer: Joel Halpern > Review result: On the Right Track > > 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 wait for direction from your > document shepherd or AD before posting a new version of the draft. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-l2sm-l2vpn-service-model-08 > Reviewer: Joel Halpern > Review Date: 2018-02-24 > IETF LC End Date: 2018-03-26 > IESG Telechat date: 2018-04-05 > > Summary: Given the number of Major and minor issues, this document is not yet > ready for publication as a Proposed Standard RFC. > > Major: > Introduction: The phrasing of "an abstract model", "this model is not a > configuration model..." creates some confusion in the reader as to whether > this model represent the current state of service deliveyr, the desired > state of service delivery (which would drive configuration) or both. > Please clarify. > > The "valid-provider-identifiers' distinguish between cloud-identifier and > remote-carrier-identifier. It i unclear why the VPN service provider > should know or care whether the remote provider he is connecting with is a > cloud provider, and another L2 service provider, or both. And if it is > both, which identifier should be used. > > Also, it is very unclear how these identifiers will be used. They > presumably are names of something. But of what? As known to whom? > Derived from where? I do not see how a provider / customer pair using this > model will know what values to use for this. Even if the intention is that > these be names made available by the provider by external means, the YANG > model needs to say that if it is to be usable. I did eventually find some > explanation in section 5.15. At the very least a forward reference is > needed. I think more explanation of what these things names would also > help. > > The use of different sets of what read like service types (is cloud access > a service type? Is remote-access a service type?) and the use of similar > but not the same terminology between provider descriptions, service types, > and service topologies, leaves the reader VERY confused. Please, do not > use the same term for kinds of providers, kinds of services, and kinds of > topologies unless the names are fully congruent (which they currently are > not.) > > It is unclear why "Cloud-Access" is listed in the VPN Service Overview > (section 5.2), or even why Cloud Access is any different from any other > access. Presumably, the customer can configure authorization for the > sites to meet his needs. Any topological effect would be capture in > 5.2.2 on VPN Service Topology, not as a different kind of VPN Service. > > Regarding VPN Service Type (svc-type) the text in section 5.2 says that > this is explicitly for the local administrator to use to flexibly define > the CPN service type. Section 5.2.1 then says that it has one of six > values, implying that if other values are needed they will need to be > defined in an extension to the model. If they are for model use, and for > model extension, then they should be using a two-level identity (where the > second level provides the possible values.) > > Given taht this is a model for providers and customers to use to > collaborate on the configuration of VPNs, I would expect to see some > discussion of how this is used on the provider end so as to collaborate > with multiple customers, working with each only about their VPNs. I missed > any such description. > > Minor: > I would have expected some reference to the MEF Ethernet service > definitions and MEF defined parameters of interest, as industry usage seems > to reflect those as the common basis for L2 services. I udnerstand that > this model is not mandated to conform to the MEF Forum work. I would > expect some discussion of the relationship. This may be a deliberate > working group choice, as I see in teh change log that there were references > to EVC and OVC. It still seems that it would help readers to have > something. > > The structure of the vpn-profile-cfg grouping seems very strange. It is a > series of 4 lists, each of which only contains an id leaf. First, and less > important, that makes them leaf-lists, doesn't it? Or is it structured > this way with no explanation to allow for unexplained type specific > augmentation? If no Augmentation is needed, it would seem more general to > use a two level identity (identity based enumeration) for the type of VPNs, > use a single list containing an id and a type field, where both are keys > and the type field uses the enumeration. This would still easily allow for > adding new types, and would avoid using the same leaf name in different > lists (which while legal often leads to errors.) If we really need four > distinct lists, then I would recommend changing the names of the id field > so each one has a unique leaf name (cloud-id, qos-id, bfd-id, ...) It > appears that the purpose of this list is to be used as targets for > leafrefs. As such, it does not seem that distinct lists are needed. > > The placement of section 5.2.2.1 (and the resulting YANG objects) seems > odd. "Route Target Allocation" is a mechanism, not a topology. It is not > even listed in the options mentioned in 5.2.2. > > Section 5.2.3 on Cloud Access uses a variant on the unfortunate "MUST ... > except ... MAY" construction. As far as I can tell, that is a very nice > SHOULD, with an explanation of when the SHOULD does not apply. Even if > this is not fixed, the inconsistency between having an exception here, and > the strict requirement (upper case MUST with no exception) in section 5.2 > needs to be fixed. > > Section 5.3 on a Site Overview has an item for "Management" which "Defines > the model of management for the site". It is completely unclear from this > text what it is intended to mean, and the example does not help. (5.11 is > better, but still vague.) > > When I reached the note in section 5.3.1 that a site may have multiple > locations, I realized that I did not see anything explicit as to whether a > site is assumed to have full internal connectivity (so that from the point > of view of the VPN any of the access links to the site are interchangeable, > or if it is fully meshed but there may be preferences for entrance for > different distinations, or whether sites may actually be partitioned, where > one part of a site is only reachable from another part of a site fia the > VPN (the usual assumption when told that there are multiple locations in a > site). I think this should be clarified. > > In section 5.5.1.2 on MultiVPN attachment, the text says "Reaching VPN A or > VPN from the New York office will be done via destination-based routing." > Routing usually refers to the handling of IP packets. Is the intention > that this distinction is based on IP destination even though we are > providing an L2 service? Is the intention that MAC addresses are unique > across the two VPNs, and the bridging tables will know which VPN contains > which destinations? If the later is the intention, how does that interact > with B/U/M frames? > > In section 5.5.2.2 on site policy, the text appears to be attempting to > answer the question of which destinations in a site should be reachable > over (possibly should have reachability to) which VPNs. It does this via > a "lan" tag. The meaning of this tag is unclear. Reading between the > lines, this appears to be intended to say that the segregation is on the > basis vlan tag (although the string is "lan" not "vlan" much less "vlan > tag".) if the intention is that policy is on the basis of vlan, it is > unclear how this relates to the assert in 5.5.1.2 that selection is on the > basis of destination address. > > Section 5.6 seems to indicate that parameters and constraints are different > things. Several of the subsections of 5.6 such as access-type seem to > indicate that information may be either a parameter or a constraint. Given > that the difference seems to be between a customer hint and a customer > requirement, how can something be both? > > Section 5.17 has a short paragraph in the middle that uses the term OVC > that is not otherwise used in this document. > > Why do the examples in section 7 include qos-profile-identifiers when the > description does not include any reference to multiple QoS behaviors, and > nothing in the example makes use of the defined identifiers? > > Editorial: > The wording at the front of section 5.2.5 could use tuning. It currently > says "If Frame Delivery Service support is required..." It seems to me > that by definition all L2VPNs require support for delivery of L2 frames. > This seems instead to be about parameters for handling BUM (Broadcast / > Unkown / Multicast) delivery. If so, this should be named suitably. It > would also be helpful if this were explicitly related to the support > parameter in 5.10.3. > > Section 5.3.2 refers to the "bearer" parameters as "below layer 2". > Section 5.3.2.1 on Bearer refers to it as "below layer 3". I presume that > should be "below layer 2"? > > In Section 5.5.1 the text states that "There are three possible types of .. > Therefore the model supports three flavors:" Which is then followed by a > list of four bullets. > > The indenting of the XML in section 5.5.2.1 should be repaired. All of the > XML examples should have their indenting checked. > > The text in section 5.6 says "The management system MUST honor all customer > constraints...". Then it says "Parameters such as site location ... are > just hints." I think that the intention is that "parameters" and > "constraints" are different things. If so, the paragraph above where those > terms are introduced should at least indicate something about the diffence. > Maybe "parameters (hints) and constraints (customer requirements)"? > > It seems surprising in 5.6.4 on Access Diversity for a customer to be able > to talk about whether things are premitted to be on the same line card. > That seems a level that an operator is unlikely to expose. > > It is surprising that committed vs excess bandwidth is treated as a QoS > parameter, with no mention of it in 5.10.1 "Bandwidth". Particularly since > these are actually parameters of "<bandwidth>"
