Hi Mahesh, Thanks for the review.
Candidate changes to take into account your review can be seen at: https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/attachment-circuit-model/draft-ietf-opsawg-ntw-attachment-circuit.txt&url_2=https://boucadair.github.io/attachment-circuit-model/AD-Review-of-AC-Common/draft-ietf-opsawg-ntw-attachment-circuit.txt. Please see inline. Cheers, Med Orange Restricted > -----Message d'origine----- > De : Mahesh Jethanandani <mjethanand...@gmail.com> > Envoyé : samedi 26 octobre 2024 07:10 > À : draft-ietf-opsawg-ntw-attachment-circ...@ietf.org > Cc : opsawg <opsawg@ietf.org> > Objet : AD review of draft-ietf-opsawg-ntw-attachment-circuit > > > Dear authors of draft-ietf-opsawg-ntw-attachment-circuit, > > Here is the last of the four documents that form this cluster. > And just like the other three, please respond to the COMMENTs > provided here, while you decide how you want to handle the NITs. > > Thanks again for a comprehensive document. > > ----------------------------------------------------------------- > COMMENT > ----------------------------------------------------------------- > > Paragraph 1 > > A Network YANG Data Model for Attachment Circuits > > draft-ietf-opsawg-ntw-attachment-circuit-12 > > Just a note. This review covers -12 version of the document. I > see there is a -13 verion that is out. > > Section 1, paragraph 6 > > .---. > > |CE6| > > '-+-' > > ac | .---. .---. > > | |CE5+------+------+CE2| > > .-----+-----. '---' | '---' > > | | |ac > > | | | > > .-+-. .-+-. .-+-. > > .-+sap+-------+sap+-. .-+sap+-------------. > > | '---' '---' | | '---' | > > | PE1 | | PE2 | > > .---. .-+-. | | | > > |CE1+--+sap| | | | > > '---'ac'-+-' | | | > > '-------------------' '-------------------' > > > > .-------------------. .-------------------. > > | | | .-+- > .ac.---. > > | PE3 | | PE4 > |sap+--+CE5| > > | | | '---' > '---' > > | | | | > > | .---. | | .---. .---. .---. | > > '-------------+sap+-' '-+sap+-+sap+-+sap+-' > > '-+-' '-+-' '-+-' '-+-' > > |ac | |ac |ac > > .-+-. | .-+-. | > > |CE3+-----ac-----' |CE4+---' > > '---' '---' > > > > Figure 1: Attachment Circuits Examples > > It would be helpful if the cluster of these four documents would > use a single diagram across the drafts, even as they define > different parts of the diagram. Specifically, in draft-ietf- > opsawg-ac-lxsm-lxnm-glue, the diagram was updated to > differentiate between CE having two ACs, one with the same bearer > id, and one with two separate bearer ids. That is not reflected > in this document. > [Med] Good point. Kept this figure because it zooms into SAPs (as this one is inspired by a figure in 9408). Added a new subsection that basically echoes the same figure we have in the glue draft. > Section 1, paragraph 7 > > The YANG data model in this document conforms to the Network > > Management Datastore Architecture (NMDA) defined in > [RFC8342]. > > Please state explicitly that this is a YANG 1.1 mode, and > providing a normative reference to RFC7950. [Med] ACK. > > Section 4, paragraph 1 > > Figure 3 shows the positioning of the AC network model in > the overall > > service delivery process. The 'ietf-ac-ntw' module is a > network > > model which augments the SAP with a comprehensive set of > parameters > > to reflect the attachment circuits that are in place in a > network. > > The model also maintains the mapping with the service > references that > > are used to expose these ACs to customers > > [I-D.ietf-opsawg-teas-attachment-circuit]. Whether the same > naming > > conventions to reference an AC are used in the service and > network > > layers is deployment-specific. > > Would it help to specify a little more context here? For example, > "The model also maintains the mapping with the service references > that are used to expose those ACs to customer using the 'ietf-ac- > sv' module defined in [I-D.ietf-opsawg-teas-attachment-circuti]. [Med] Works for me. Thanks. > > Section 5, paragraph 1 > > The full tree diagram of the 'ietf-ac-ntw' module can be > generated > > using the "pyang" tool [PYANG]. That tree is not included > here > > because it is too long (Section 3.4 of [RFC8340]). Instead, > subtrees > > are provided in the following subsections for the reader's > > convenience. > > Following the discussion on the netmod mailing list, please > include a complete tree diagram in the Appendix. > [Med] Will update this one once we have a stable conclusion in netmod. Hope this is OK with you. Thanks. > Section 5.1, paragraph 3 > > A node can host one or more SAPs. Per [RFC9408], a SAP is > an > > abstraction of the network reference point (the PE side of > an AC, in > > the context of this document) where network services can be > delivered > > and/or are delivered to customers. Each SAP terminates one > or > > multiple ACs. Each AC in turn may be terminated by one or > more peer > > SAPs ('peer-sap'). In order to expose such AC/SAP binding > > information, the SAP model [RFC9408] is augmented with > required AC- > > related information. > > The authors should follow the guidance in rfc8407-bis, Section > 4.3.1 for naming the identifiers. For example, there is no need > to prefix 'ac-svc-ref', 'ac-profile', or 'ac-parent-ref' with the > prefix 'ac', as the parent is named 'ac'. [Med] OK. Fixed. > > Also, not clear on this statement. "where network services can be > delivered and/or are delivered to customers". [Med] FWIW, this echoes the def of SAP in rfc9408 Service Attachment Points (SAPs): An abstraction of the network reference points (e.g., the PE side of an AC, or the CE side of an AC for a provider-managed CE) where network services can be delivered and/or are delivered to customers. A SAP can be bound to one or multiple ACs. I think a clear > distinction should be added to distinguish between 'sap' and > 'peer-sap' as it applies to the network or customer side of the > network. For example, from this modules perspective, and as > stated above, a SAP is a network reference point on the Provider > Edge (PE), whereas the peer-sap is remote endpoint of an AC, that > is the Customer Edge (CE). It is not always clear in the document > which end of the AC or SAP is being referred to. > > Finally, the term 'peer SAP' or 'peer-sap' seems to have been > introduced here for the first time. It has been defined in > RFC9408, so maybe just a reference needs to be added. Can the > terminology, and maybe one of the diagrams indicate where a > 'peer-sap' can exist? [Med] The new figure/text introduced to address a comment above has now: "A CE is seen by the network as a peer SAP {{?RFC9408}}." > > Section 5.4, paragraph 1 > > The 'l2-connection' container is used to manage the Layer 2 > > properties of an AC. The Layer 2 connection tree structure > is shown > > in Figure 8. > > Which side of the AC. PE or CE? [Med] The network model in the document focuses on the PE-side of an AC. Added "(mainly, the PE side of an AC)" for better clarity. If it is PE, since this is the > network AC, who is the consumer of this information? [Med] This can be a network orchestrator per Figure 3. CEs have not access to this provider internal view of ACs. If it is the > CE, then should this be in this model or in the ietf-ac-svc > model? > > Section 6, paragraph 12 > > description > > "A type for an absolute reference to an attachment > > circuit."; > > Per yangdoctors comment, albeit it was on the typedef statement, > but because they were both talking about references, this should > be modified to say, "An absolute reference to an attachment > circuit." > [Med] Works for me. Thanks. > Section 6, paragraph 12 > > description > > "A type for an absolute reference to an attachment > > circuit."; > > Same comment as above, and all description statments that start > with "A type for an absolute reference ...". > [Med] Ack > Section 7, paragraph 1 > > This section uses the template described in Section 3.7 of > > [I-D.ietf-netmod-rfc8407bis]. > > Please consider using the latest text of the template based on > the discussion on the mailing list. > [Med] Done. > Possible DOWNREF from this Standards Track doc to [IEEE802.1Qcp]. > If so, the IESG needs to approve it. [Med] This is an IEEE standard that is cited by many RFCs. I'm not sure this the downref procedure covers standards by other SDOs. > > ----------------------------------------------------------------- > NIT > ----------------------------------------------------------------- > > All comments below are about very minor potential issues that you > may choose to address in some way - or ignore - as you see fit. > Some were flagged by automated tools (via > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2 > Fgithub.com%2Flarseggert%2Fietf- > reviewtool&data=05%7C02%7Cmohamed.boucadair%40orange.com%7C5e2eb7 > 665e7740bedd0f08dcf57c7896%7C90c7a20af34b40bfbc48b9253b6f5d20%7C0 > %7C0%7C638655162182069715%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA > wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C& > sdata=FGMBVwL2DEKwPXUCjgPli6N2j8tNSa%2FCI1ZcFtkhksQ%3D&reserved=0 > ), so there will likely be some false positives. There is no need > to let me know what you did with these suggestions. > > Paragraph 1 > + COMMENT: > > Section 1, paragraph 4 > > The document leverages [RFC9182] and [RFC9291] by adopting > an AC > > provisioning structure that uses data nodes that are defined > in these > > RFCs. Some refinements were introduced to cover, not only > > conventional service provider networks, but also specifics > of other > > target deployments (cloud network, for example). > > s/in these RFCs/in those RFCs/ > s/to cover,/to cover/ > s/(cloud network, for example)/e.g., cloud network/ [Med] ACK > > Section 1, paragraph 4 > > The AC network model is designed as augmentations to both > the 'ietf- > > network' model [RFC8345] and the Service Attachment Point > (SAP) model > > [RFC9408]. An attachment circuit can be bound to a single > or > > multiple SAPs. Likewise, the model is designed to > accommodate > > deployments where a SAP can be bound to one or multiple ACs > (e.g., a > > parent AC and its child ACs). > > s/augmentations to/augmentations of/ [Med] ACK > > Section 5.3, paragraph 4 > > The exact definition of the specific provisioning profiles > profiles > > is local to each service provider. The model only includes > an > > identifier for these profiles in order to ease identifying > and > > binding local policies when building an AC. As shown in > Figure 7, > > the following identifiers can be included: > > s/profiles profiles/profiles/ [Med] ACK > > Document references draft-ietf-opsawg-teas-common-ac-11, but -12 > is the latest available revision. > > Document references draft-ietf-opsawg-teas-attachment-circuit-13, > but -17 is the latest available revision. > > Document references draft-ietf-netmod-rfc8407bis-14, but -20 is > the latest available revision. > [Med] Will fixed automatically when submitting the new rev. > > Mahesh Jethanandani > mjethanand...@gmail.com > > ____________________________________________________________________________________________________________ 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 To unsubscribe send an email to opsawg-le...@ietf.org
