Hi Med, On Wed, Sep 22, 2021 at 09:40:17AM +0000, mohamed.boucad...@orange.com wrote: > Hi Ben, > > Thank you for the review. The changes to address your review can be tracked > at: > https://github.com/IETF-OPSAWG-WG/lxnm/commit/108707e8b2aea6590b0a9695756c7546887c9614 >
Thanks. Also inline... > > Please see inline. > > Cheers, > Med > > > -----Message d'origine----- > > De : Benjamin Kaduk via Datatracker [mailto:nore...@ietf.org] > > Envoyé : mercredi 22 septembre 2021 06:19 > > À : The IESG <i...@ietf.org> > > Cc : draft-ietf-opsawg-vpn-com...@ietf.org; opsawg-cha...@ietf.org; > > opsawg@ietf.org; adr...@olddog.co.uk; adr...@olddog.co.uk > > Objet : Benjamin Kaduk's No Objection on draft-ietf-opsawg-vpn-common- > > 10: (with COMMENT) > > > > Benjamin Kaduk has entered the following ballot position for > > draft-ietf-opsawg-vpn-common-10: No Objection > > > > When responding, please keep the subject line intact and reply to all > > email addresses included in the To and CC lines. (Feel free to cut this > > introductory paragraph, however.) > > > > > > Please refer to https://www.ietf.org/blog/handling-iesg-ballot- > > positions/ > > for more information about how to handle DISCUSS and COMMENT positions. > > > > > > The document, along with other ballot positions, can be found here: > > https://datatracker.ietf.org/doc/draft-ietf-opsawg-vpn-common/ > > > > > > > > ---------------------------------------------------------------------- > > COMMENT: > > ---------------------------------------------------------------------- > > > > I have a bold proposal for consideration: since we are defining a new > > common set of groupings for VPN and, in some cases, proposing to rename > > existing terminology already, can we consider moving away from the term > > "VPN" itself? The word "private" is ambiguous as to what level of > > privacy is being provided (e.g., network routing vs cryptographic) and > > who the conveyed content is to be private from. A term like "virtual > > network" removes that ambiguity, and lets us use the explicit attributes > > to convey whether encryption is in use when appropriate. > > > > There is no particular triggering point (at least, not yet) at which it > > becomes clearly correct to make a breaking change like this, so we may > > end up just having to pick a time somewhat arbitrarily, if we are to > > make such a change at all. Perhaps now is that time; perhaps not. > > [[Med]] I hear some of your observations but VPN is a well-established name > for a widely deployed technology. I don't think it is a good idea to touch > this. Please note that there is already generic model for VNs > (https://datatracker.ietf.org/doc/draft-ietf-teas-actn-vn-yang/) that is > distinct from what we are doing here. Well, I don't think that just because VPN is well-established we should ignore the ambiguity in what it describes. So, I still want to touch it at some point, but do not insist on doing so right now. I am sure that we can find some non-VPN term other than VN if there is desire to distinguish...but we can defer that conversation, of course. > > > > Section 3 > > > > grouping vpn-profile-cfg > > +-- valid-provider-identifiers > > +-- external-connectivity-identifier* [id] > > | {external-connectivity}? > > | +-- id? string > > > > Presumably I am just misreading RFC 8340, but I thought that the list > > key was implicitly mandatory, so it would appear as just "id" (as > > opposed to "id?"). (Many instances.) > > [[Med]] Good question. Actually we trusted what is in Section 3.2 of 8340. > That's the output we get from pyang. Note that ? is not displayed when the > grouping is used in another model (L2NM/L3NM). Will double check this with > Martin. Thanks. > > > > 'vpn-route-targets': > > * A YANG grouping that defines Route Target (RT) import/export > > rules used in a BGP-enabled VPN (e.g., [RFC4364][RFC4664]). > > > > On very quick skim, it's not clear what motivates the RFC 4664 reference > > -- "route target" does not appear, for example, nor does "import" or > > "export". > > [[Med]] This is just to insist that it can be used for both L3 and L2VPNs. > Unlike L3NM, there are plenty L2VPN technologies; hence the choice to point > to a generic L2 framework. Okay, so the references are just generic "VPN" (or "BGP-enabled VPN") references, and I was misreading to ascribe more meaning to them. If we wanted to spend a few more words (far from clear), maybe "(suitable VPN technologies include but are not limited to [RFC4364][RFC4664])". > > > > Section 4 > > > > identity pim-proto { > > if-feature "pim"; > > base routing-protocol-type; > > > > This is in the section with the group-management-protocols; is "routing- > > protocol-type" really the intended base? > > [[Med]] Yes. Both PIM and other group management protocols fall under: > > /* > * Identities related to multicast > */ > > > > > identity embb { > > [...] > > identity urllc { > > [...] > > identity mmtc { > > > > Similar to Roman's comment, a reference seems useful for these. > > If we intend to implicitly rely on RFCs 8299 and/or 8466 for > > terminology, we should say so in the terminology section. > > [[Med]] Added this note under the terminology section: > > The document inherits many terms from [RFC8299] and [RFC8466] > (e.g., Enhanced Mobile Broadband (eMBB), Ultra-Reliable and Low > Latency Communications (URLLC), Massive Machine Type Communications > (mMTC)). Thanks! > > > > leaf vpn-id { > > type vpn-common:vpn-id; > > description > > "A VPN identifier that uniquely identifies a VPN. > > This identifier has a local meaning, e.g., within > > a service provider network."; > > > > Thank you for indicating the scope of validity of the identifier! > > (Elsewhere as well.) > > > > grouping service-status { > > [...] > > leaf last-change { > > type yang:date-and-time; > > description > > "Indicates the actual date and time of the service status > > change."; > > > > What's the motiviation behind leaving this as "config true"? When would > > it be useful to set it manually? > > [[Med]] This can be set, for example, when a new administrative status is > set. But would it be set to a time other than the time when the new administrative status was set, i.e., a value that can be automatically determined? > > > > list vpn-target { > > [...] > > leaf id { > > type int8; > > description > > "Identifies each VPN Target."; > > > > I wasn't able to find the motivation for limiting to int8 in RFC 4364, > > though I mostly assume I'm just looking in the wrong place. > > [[Med]] I confirm that there is no such concept in 4364. The intuitive design > would be to use leaf-list, but the reason for having this is a list with an > id is recorded in this note: > > Note that this is modelled as a list to ease the reuse of this > grouping in modules where a pointer is needed (e.g., associate > an operator with RTs). > > We don't see the need to go beyond uint8. As illustrated, for example, in > L3NM a typical usage of this would be simply: Okay. I am not sure if "pointer" or "index" is better for conveying the desired meaning ("pointer" might suggest "leafref" to some readers). > == > "vpn-targets": { > "vpn-target": [ > { > "id": "1", > "route-targets": [ > "0:65550:1" > ], > "route-target-type": "both" > } > ] > } > == > > > (But why *signed* int8?) > > [[Med]] This should be unsigned. Fixed. > > > > > Section 5 > > > > While there may be no direct security considerations from what is > > effectively just defining some new compound types for reuse, I think we > > may want to consider some privacy considerations. For example, we have > > the "customer-name" field in the vpn-description, and it is sometimes > > the case that customers want to remain confidential. Thus, any > > instantiations of the grouping would incur a potential need for > > confidentiality and minimizing the scope of distribution. > > > > [[Med]] We do already discuss this for example in the L3NM. No problem to add > a note here as well. I added this NEW text: > > Modules that use the groupings that are defined in this document > should identify the corresponding security considerations. For > example, reusing some of these groupings will expose privacy-related > information (e.g., customer-name). Disclosing such information may > be considered as a violation of the customer-provider trust > relationship. Thanks, that looks good. > > > NITS > > > > Section 4 > > > > feature bearer-reference { > > description > > "Indicates support for the bearer reference access constraint. > > That is, the reuse of a network connection that was already > > ordered to the service provider apart from the IP VPN site."; > > > > I echo Roman's comment that this feature would benefit from a reference > > or further discussion; the current description leaves me unclear on what > > is meant and with no recourse for learning more. (RFCs 4026 and 4176 > > are presented as generic references for VPN-related terms, but do not > > cover the concept of "bearer reference".) > > [[Med]] The description is basically echoing RFC8299: > > == > o The "bearer-reference" parameter is used in cases where the > customer has already ordered a network connection to the SP apart > from the IP VPN site and wants to reuse this connection. > == > > We can update the description to first introduce the concept of bearer: > > "A bearer refers to properties of the CE-PE attachment that > are below Layer 3. Thanks; combined with the note in the intro about 8299 terminology that should be enough for people to figure it out. And thanks for the other updates as well, Ben > > > > reference > > "I-D.ietf-teas-ietf-network-slice-framework: > > Framework for IETF Network Slices"; > > > > draft-ietf-teas-ietf-network-slice-framework is replaced by draft-ietf- > > teas-ietf-network-slices. > > [[Med]] Shame on me as I'm contributing to that work :-( Fixed. Thanks. > > > > > identity rsvp-te { > > base protocol-type; > > description > > "Transport is based on RSVP-TE."; > > reference > > "RFC 3209: RSVP-TE: Extensions to RSVP for LSP Tunnels"; > > > > Is the transport itself RSVP-TE, or is it that RSVP-TE is used to > > provision the tunnels used as transport? (Similar question for bgp-lu?) > > [[Med]] This means "relies upon" to setup the underlay transport. > > OLD: > Transport is based on > NEW: > Transport setup relies upon > > > > > identity dot1q { > > if-feature "dot1q"; > > base encapsulation-type; > > description > > "Dot1q encapsulation."; > > > > I suppose the feature declaration does so, but maybe some "802" is in > > order here as well? > > [[Med]] We didn't as we though this is redundant with the reference under the > dot1q feature: > > reference > "IEEE Std 802.1Q: Bridges and Bridged Networks"; > > > > > identity ethernet-type { > > base encapsulation-type; > > description > > "Ethernet encapsulation type."; > > } > > > > identity vlan-type { > > base encapsulation-type; > > description > > "VLAN encapsulation."; > > } > > > > identity untagged-int { > > base encapsulation-type; > > description > > "Untagged interface type."; > > [...] > > > > Should we be more consistent about whether the description ends in > > "type"? > > [[Med]] Indeed. Fixed. > > > > > identity lag-int { > > if-feature "lag-interface"; > > base encapsulation-type; > > description > > "LAG interface type."; > > reference > > "IEEE Std. 802.1AX: Link Aggregation"; > > > > lag-int is the only encapsulation-type identify element that has a > > reference stanza. We did mention LAG in the context of 802.1AX earlier, > > so maybe it's not needed? > > [[Med]] Right. We should be consistent. > > > > > identity web { > > base customer-application; > > description > > "Web applications (e.g., HTTP, HTTPS)."; > > [...] > > identity file-transfer { > > base customer-application; > > description > > "File transfer application (e.g., FTP, SFTP)."; > > > > Maybe we could just list HTTPS and SFTP as the (respective) examples, in > > 2021? > > [[Med]] These are only examples. I tend to keep the same description as in > RFC8299 as this is related to the customer traffic. > > > > > leaf vpn-name { > > type string; > > description > > "A name used to refer to the VPN."; > > > > Should we say a little more about how the name differs from the id, > > given that both are "type string"? > > [[Med]] The main difference is provided as part of the "id" description: the > id is what is used to uniquely identify the VPN. > > That's said, I updated the description for better clarity as follows: > > NEW: > "A name used to associate a name with the service > in order to facilitate the identification of > the service"; > > > > > leaf import-policy { > > type string; > > description > > "Defines the 'import' policy."; > > } > > leaf export-policy { > > type string; > > description > > "Defines the 'export' policy."; > > > > Does it "define" or merely "identify" the policy? Naively, a > > "definition" would be a rather long string... > > [[Med]] "identifies" is more appropriate. Fixed. > > > > > grouping vpn-components-group { > > [...] > > container groups { > > description > > "Lists the groups to which a VPN node,a site, or a network > > > > space after comma. > > > > > > [[Med]] Fixed. Thank you. > > _________________________________________________________________________________________________________________________ > > Ce message et ses pieces jointes peuvent contenir des informations > confidentielles ou privilegiees et ne doivent donc > pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu > ce message par erreur, veuillez le signaler > a l'expediteur et le detruire ainsi que les pieces jointes. Les messages > electroniques etant susceptibles d'alteration, > Orange decline toute responsabilite si ce message a ete altere, deforme ou > falsifie. Merci. > > This message and its attachments may contain confidential or privileged > information that may be protected by law; > they should not be distributed, used or copied without authorisation. > If you have received this email in error, please notify the sender and delete > this message and its attachments. > As emails may be altered, Orange is not liable for messages that have been > modified, changed or falsified. > Thank you. > _______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
