As a chair… I have read through this draft, and it’s clear the authors have put a ton of work into it. Thank you for your continued focus on work to make a value resource for SP network administrators.
As a contributor… I appreciate the amount of attention paid to adding references to YANG nodes (with Section numbers) so that more clarity and information can be found. That said, this wasn’t done universally. For instance, LACP and LAG are mentioned without any references. Additionally, many descriptions are lacking details (I commented on a few below), and some are inconsistent (e.g., you describe Boolean values differently almost every time). It would be good for those looking to implement this to do a pass on relevant sections to make sure you have the details you’d expect to see when reading the module. What follows are specific items I found. Many or spelling or grammar issues, but I do have a few additional substantive comments. Abstract s/This document defines a L2VPN/This document defines an L2VPN/ Section 2 s/contains information of the service providers/contains information of the service provider’s/ s/management of the service providers network/management of the service provider’s network/ s/Is a service providers that offers/Is a service provider that offers/ …between Customer Edges (CEs) and PEs… You expand CE but never PE (until the acronym section). I thought it might be good for maximum clarity to outright define CE and PE in your terminology section. Section 6.1 s/managed in the service providers network/managed in the service provider’s network/ Section 6.5 s/Global parameters profile are defined/Global parameters profiles are defined/ You misspelled ce-vlan-cos-preservation here and in the YANG module below. Since this is a leaf, it really should have correct spelling. Section 6.6.1 s/The asigned RT can be/The assigned RT can be/ Section 6.6.2 s/Address Resolution Protocol (ARP) and Nighbour Discovery (ND)/ Address Resolution Protocol (ARP) and Neighbor Discovery (ND)/ On this, should you have a reference for ARP and ND? Section 6.7.1 You mention LAG and LACP here and below in the module. But I never see a reference or expansion for either. I think both the text and the module would benefit from both. Section 6.7.4 s/they take precendence over the one inlcudes/they take precedence over the one includes/ In your two IANA modules you have identities for PPP and VPLS and a couple of others where the description is the identity name itself. This is where I wonder if slightly better descriptions would add more clarity. At least expanding the acronym would be useful IMHO. YANG module s/The RObust Header Compression (ROHC) Framework/The Robust Header Compression (ROHC) Framework/ YANG module Concerning identity “precedence-type”: First, you misspelled “backup”. You say active and backup here, but you use primary and backup below. I typically see primary with secondary and active with backup. Either way, I think you need to be consistent. Also, since the base identity mentions the two defined sub-identities, should this really be an enumeration? Or should you just simplify this description so as not to mention any possible sub-identities? YANG module s/Indicates whether BGP auto-discovey/Indicates whether BGP auto-discovery/ s/Sets the indentifier of the VPN node/Sets the identifier of the VPN node/ s/contex of a VPN/context of a VPN/ s/Controles/Controls/g s/symetric/symmetric/ s/automatically assigned either withor/automatically assigned either with or/ s/maixmum/maximum/g YANG module Concerning leaf group-color: Okay, dumb question. What does a string for color provide here? While I’ve seen the concept of color used a lot in VPN examples, a free-form string that should be a color seems over-done here. The group-id is another open string. Why couldn’t that also be a color? I’d love to see some canonical reference that explains color use (if there is one). YANG module Concerning the number of different MTU-related leafs: The PW interface MTU is limited to 16-bits. Does it make sense to have a 32-bit service MTU here, then? Other than the pw-mtu, the other MTU leafs are all 32-bit, and I wonder if they shouldn’t all be 16. YANG module Concerning leaf color-type: Here is another use of color, but this time at least tied to an identity. Why couldn’t the other use also use this identity? Joe
_______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
