Dale, thank you for your review. I have entered a No Objection ballot for this document. Lars > On 2022-5-11, at 4:44, Dale Worley via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Dale Worley > Review result: Ready with Nits > > 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 > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-opsawg-l2nm-15 > Reviewer: Dale R. Worley > Review Date: 2022-05-10 > IETF LC End Date: 2022-05-13 > IESG Telechat date: [not known] > > I have not checked the Yang module itself, as the Yang Doctors will do > a better job than I can. Similarly, I have assumed that the specific > information required for configuring VPNs has been set correctly by > the members of the working group. I have reviewed it from the point > of view that I am qualified to, a reader with beginning knowledge > about VPNs and Yang learning more about the subject. > > Summary: > > This draft is basically ready for publication, but has nits that > should be fixed before publication. > > Nits/editorial comments: > > 2. Terminology > > Clarifying the wording here so as to make clear the relationships > between these concepts would ease the learning curve for the > inexperienced. For example, > > VPN node: Is an abstraction that represents a set of policies > applied on a PE and belonging to a single VPN service. > > This is likely known in the VPN community, but I'm having a problem > following it: What exactly is a PE, or rather, what is its conceptual > scope? A "Customer Edge-to-Provider Edge attachment circuit" is clear > to the naive, because it's the tangible thing that connects the > customer to the service provider. That suggests that the CE is the > logical entity on the customer end of the attachment, and similarly > the PE. But is the PE unique to the attachment circuit, and thus the > VPN has a lot of PEs that it interconnects, or is there a single PE in > the VPN? > > Also, is there only one VPN node that is applied to any one PE, or can > there be many? This determines whether VPN nodes are one-to-one with > PEs or whether they may have a wider scope. It seems to be known that > a VPN can have multiple VPN nodes, but that isn't stated in the > definition either. > > VPN network access: [...] > > A reference to the bearer is maintained to allow keeping the link > between the L2SM and the L2NM when both data models are used in a > given deployment. > > This sentence is correct, but it doesn't seem to belong in this > location, as it seems to describe an aspect of the data concerning an > attachment circuit, whereas "VPN network access" is an abstraction of > just one end of an attachment circuit. Or does the conceptual model > not have an abstraction of the other end of attachment circuits, thus > allowing the network interface and the attachment circuit to be > conflated, and thus the reference to the bearer can be considered to > be a property of the VPN network access? > > 4. Reference Architecture > > In Figure 1, some of the module names are missing the initial "ietf-". > > The bottom section of Figure 1 is: > > +------+ Bearer +------+ +------+ +------+ > | CE A + ---------- + PE A + + PE B + ------- + CE B | > +------+ Connection +------+ +------+ +------+ > > Site A Site B > > Shouldn't there be some indication of connection between PE A and PE > B? > > Also, it's not clear why this set of boxes is integrated with the rest > of Figure 1, as the lines in the figure don't seem to show any > particular connection between these four boxes and the boxes above > them. This segment seems to be a generic VPN between two sites, but > "Site A" and "Site B" don't seem to be referenced elsewhere. > > If the intention is that "Network" embraces all 4 of these boxes, then > the ends of the dashed line above "Network" need to be aligned with > the left and right edges of the sites, and possibly some "|" added to > show the various interconnections, perhaps in this style: > > +----+----+ | | > | Config | | | > | Manager | | | > +----+----+ | | > | | | > | NETCONF/CLI.................. > | | | > +---------------------------------------------------------+ > | Network | > > +------+ Bearer +------+ +------+ +------+ > | CE A + ---------- + PE A +======+ PE B + ------- + CE B | > +------+ Connection +------+ +------+ +------+ > > Site A Site B > > But if you want to emphasize that L2NM only describes the part of the > network between the PE's, you could do something like this: > > +----+----+ | | > | Config | | | > | Manager | | | > +----+----+ | | > | | | > | NETCONF/CLI.................. > | | | > +-------------------------------+ > | Network | > > +------+ Bearer +------+ +------+ Bearer +------+ > | CE A + ---------- + PE A +======+ PE B + ------- + CE B | > +------+ Conn. +------+ +------+ Conn. +------+ > > Site A Site B > > 7.2. VPN Profiles > > The 'vpn-profiles' container (Figure 5) is used by a VPN service > provider to define and maintain a set of VPN profiles [RFC9181] that > apply to one or several VPN services. > > This document does not make any assumption about the exact definition > of these profiles. > > I am having a hard time understanding what is intended. The first > sentence says that the container provides a way to define profiles, > but the rest of the document doesn't provide any way to define the > profiles. Is the intended meaning that the container lists the names > of profiles that are defined somewhere else, so that once the names > are listed in the container, the profiles can be referenced in the > definitions of services? Or is there an implication that additional > Yang nodes can be added in vpn-profiles to provide the definitions? > > In addition, despite the phrasing, there seems to be no constraint > that a particular profile is in fact applied to one or several > services. Is the intended meaning that the listed profiles *can be > applied* to services? > > 8.1. IANA BGP Layer 2 Encapsulation Types > > description > "ATM transparent cell transport"; > > For consistency with the other items, there should be a "." at the end > of the description. > > 10.1. YANG Modules > > Perhaps this section would better be named "Registrations". > > 10.2. BGP Layer 2 Encapsulation Types > > There are a number of ways the wording of this section could be > improved. > > This document defines the initial version of the IANA-maintained > "iana-bgp-l2-encaps" YANG module (Section 8.1). IANA is requested to > add this note for both modules: > > "both modules" isn't correct here, as only one module has been > mentioned by this point in the section. > > BGP Layer 2 encapsulation types must not be directly added to the > "iana-bgp-l2-encaps" YANG module. They must instead be > respectively added to the "BGP Layer 2 Encapsulation Types" > registry [IANA-BGP-L2]. > > It's not clear what the word "respectively" means in this context. > It would be clearer if the second sentence was expanded to: > > BGP Layer 2 encapsulation types must not be directly added to > the "iana-bgp-l2-encaps" YANG module. They must instead be > added to the "BGP Layer 2 Encapsulation Types" registry > [IANA-BGP-L2], and then a derived identity added to > "iana-bgp-l2-encaps". > > -- > > The name of the > "identity" is the lower-case of the encapsulation name provided in > the description. > > "the description" is ambiguous here; you mean "the description in the > registry". > > But for most of the existing encapsulation types, the identity name is > not, strictly speaking, the lower-case of the encapsulation name in > the registry, but rather something similar. Suitable wording could be > "a lower-case version of the encapsulation name". > > "reference": Replicates the reference from the registry and add the > title of the document. > > Possibly better phrased "Replicates the reference from the registry > with the title of the document added." > > 10.3. Pseudowire Types > > Similar nits as for section 10.2. > > A.4. VPWS-EVPN Service Instance > > Figure 29 would be clearer if a little more space was used to separate > "ESI{1,2}" from the network elements. > > > |<-------- EVPN Instance --------->| > | | > | V V | > | +-----+ +--------------+ +-----+ | > +----+ | | PE1 |===| |===| PE3 | | +----+ > | +-------+ | | | | +-------+ | > | | | +-----+ | | +-----+ | | | > | CE1| | | Core | | |CE2 | > | | | +-----+ | | +-----+ | | | > | +-------+ | | | | +-------+ | > +----+ | | PE2 |===| |===| PE4 | | +----+ > ^ | +-----+ +--------------+ +-----+ | ^ > | | | | > | ESI1 ESI2 | > | | > |<-------------- Emulated Service ---------------->| > > Figure 29: An Example of VPWS-EVPN > > > A.5. Automatic ESI Assignment > > Figure 32 would be a little clearer if "LACP" was moved down a line > (in parallel with how ES is raised a line). > > ES > | +-----+ +--------------+ +-----+ > +----+ | | PE1 |======| |===| PE3 | +----+ > | +-------+ | | | | +-------+ CE3| > | | | +-----+ | | +-----+ +----+ > | CE1| | | Core | > | | | +-----+ | | +-----+ +----+ > | +-------+ | | | | +-------+ CE2| > +----+ | | PE2 |======| |===| PE4 | +----+ > | +-----+ +--------------+ +-----+ > LACP > > Figure 32: An Example of Automatic ESI Assignment > > [END] > > > > -- > last-call mailing list > last-call@xxxxxxxx > https://www.ietf.org/mailman/listinfo/last-call
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
