Hi Authors of draft-ietf-opsawg-teas-attachment-circuit,
First of all my apologies for taking all this time to review this document.
This was no small document, specially if you combine with the fact that there
are 3 other supporting documents. Regardless, here are some of my comments. I
would like to see some discussion around the COMMENTs, and leave you to deal
with the NITS as you feel.
-------------------------------------------------------------------------------
COMMENT
-------------------------------------------------------------------------------
Overall comments:
The four documents
- this draft,
- draft-ietf-opsawg-ntw-attachment-circuit
- draft-ietf-opsawg-teas-common-ac
- draft-ietf-opsawg-ac-lxlm-lxnm-glue
as I understand should be treated as a cluster. That includes the fact that
they should be have been shephered by a single person, and sent up for AD
review as a cluster. As I put the pieces together, here are general comments
that apply to all the drafts. Individual comments on each draft will be
provided in their own review.
Also note that this review is for -14 version of the document. The present
version of the draft would be more like -17.
First of all, the models are fairly comprehensive in themselves. So thank you
for putting all the word behind them. I can see it working as a template, where
different users pick different pieces of the model. The authors have worked to
add more deployment scenarios to explain how the model could be used in those
scenarios as part of overall discussion on the model.
The rest of the review will focus on aspects of this draft.
I found it amusing that the 5 authors and 5 contributors formed the major
portion of the "broad consensus" the document got from the WG. That combined
with the fact that the document got few WG members supporting it concerns me. I
am not sure if it is the lack of interest, or the document was difficult to
follow.
"Abstract", paragraph 2
> Also, the document specifies a set of reusable groupings. Whether
> other service models reuse structures defined in the AC models or
> simply include an AC reference is a design choice of these service
> models. Utilizing the AC service model to manage ACs over which a
> service is delivered has the advantage of decoupling service
> management from upgrading AC components to incorporate recent AC
> technologies or features.
This paragraph could be moved into the Introduction section, thus keeping the
Abstract concise.
Section 4.1, paragraph 7
> * Customers may request protection schemes in which the ACs
> associated with their endpoints are terminated by the same PE
> (e.g., CE#3), distinct PEs (e.g., CE#34), etc. The network
> provider uses this request to decide where to terminate the AC in
> the provider network (i.e., select which PE(s) to use) and also
> whether to enable specific capabilities (e.g., Virtual Router
> Redundancy Protocol (VRRP) [RFC9568]). Note that placement
> constraints may also be requested during the instantiation of the
> underlying bearers (Section 5.1).
There is no node called CE#34 in the diagram.
Section 5.1, paragraph 3
> +--rw locations
> | +--rw customer-name? string
Not clear on what the difference is between this customer-name, and the
customer name under bearers, and the customer-name under bearer. Are these
different customers?
Shouldn't bearers be under each location? Isn't the bearer an underlay between
a particular CE and PE? In other words, do all locations have access to all
bearers?
Section 5.1, paragraph 2
> | +--rw local-as? inet:as-number
> | +--rw peer-as? inet:as-number
> | +--ro location* [location-name]
> | +--ro location-name string
> | +--ro address? string
> | +--ro postal-code? string
> | +--ro state? string
> | +--ro city? string
> | +--ro country-code? string
The IVY WG has a inventory model that defines locations in the form of a
grouping. Can those definitions be used here instead of redefining them?
Section 5.2, paragraph 1
> The full tree diagram of the module can be generated using, e.g., the
> "pyang" tool [PYANG]. That tree is not included here because it is
> too long (Section 3.4 of [I-D.ietf-netmod-rfc8407bis]). Instead,
> subtrees are provided for the reader's convenience. The full tree of
> the 'ac-svc' is provided in [AC-svc-Tree].
While an abridged tree diagram is what should be used to describe a model,
depending on the discussion that is happening on the netmod mailing list, I
hope the authors will consider adding a full tree diagram in the Appendix.
Section 5.2.2.1, paragraph 1
> The 'specific-provisioning-profiles' container (Figure 6) can be used
> by a service provider to maintain a set of reusable profiles. The
> profiles definitions are similar to those defined in [RFC9181],
> including: Quality of Service (QoS), BFD, forwarding, and routing
> profiles. The exact definition of the profiles is local to each
> service provider. The model only includes an identifier for these
> profiles in order to facilitate identifying and binding local
> policies when building an AC.
I was unable to determine whether the 'specific-provisioning-profiles' defined
in RFC9181 were reused here or redefined in this model.
Section 5.2.5.5, paragraph 4
> The 'security' container specifies the authentication and the
> encryption to be applied to traffic for a given AC. Tthe model can
> be used to directly control the encryption to be applied (e.g., Layer
> 2 or Layer 3 encryption) or invoke a local encryption profile.
Can the authors provide an example of what kind of traffic would be encrypted
using this encryption profile? As I understand it, this is not end-to-end
encrypted traffic. It is traffic encrypted as it is traversing the AC. Is that
right?
Section 7, paragraph 1
> This section uses the template described in Section 3.7 of
> [I-D.ietf-netmod-rfc8407bis].
I hope this section will be updated with the new text that has been agreed upon.
Section 7, paragraph 18
> Several data nodes ('bgp', 'ospf', 'isis', and 'rip') rely upon
> [RFC8177] for authentication purposes. As such, the AC service
> module inherits the security considerations discussed in Section 5 of
> [RFC8177]. Also, these data nodes support supplying explicit keys as
> strings in ASCII format. The use of keys in hexadecimal string
> format would afford greater key entropy with the same number of key-
> string octets. However, such a format is not included in this
> version of the AC service model because it is not supported by the
> underlying device modules (e.g., [RFC8695]).
Do the authors want to add a statement that says no rpcs are defined, and
therefore no security considerations need to be documented for them.
Section 9.2, paragraph 4
> [I-D.ietf-opsawg-ac-lxsm-lxnm-glue]
> Boucadair, M., Roberts, R., Barguil, S., and O. G. de
> Dios, "A YANG Data Model for Augmenting VPN Service and
> Network Models with Attachment Circuits", Work in
> Progress, Internet-Draft, draft-ietf-opsawg-ac-lxsm-lxnm-
> glue-10, 10 June 2024,
> <https://datatracker.ietf.org/doc/html/draft-ietf-opsawg-
> ac-lxsm-lxnm-glue-10>.
>
> [I-D.ietf-opsawg-ntw-attachment-circuit]
> Boucadair, M., Roberts, R., de Dios, O. G., Barguil, S.,
> and B. Wu, "A Network YANG Data Model for Attachment
> Circuits", Work in Progress, Internet-Draft, draft-ietf-
> opsawg-ntw-attachment-circuit-11, 15 May 2024,
> <https://datatracker.ietf.org/doc/html/draft-ietf-opsawg-
> ntw-attachment-circuit-11>.
Are the YANG modules from these drafts not being imported by the YANG modules
defined in this draft? If so, these should be moved to Normative Section of
references.
"Appendix A.", paragraph 0
> This section includes a non-exhaustive list of examples to illustrate
> the use of the service models defined in this document. An example
> instance data can also be found at [Instance-Data].
I am hoping that these examples have been validated with tools such as yanglint
or similar. Since these examples are not marked as code, it is difficult to
extract them from the draft for me to validate them.
Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:
* Term "natively"; alternatives might be "built-in", "fundamental",
"ingrained", "intrinsic", "original"
-------------------------------------------------------------------------------
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://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.
Section 5.2.1, paragraph 9
> Features are used to tag conditional protions of the model in order
> to accomodate various deployments (support of layer 2 ACs, Layer 3
> ACs, IPv4, IPv6, routing protocols, Bidirectional Forwarding
> Detection (BFD), etc.).
s/protions/portions/
Document references draft-ietf-opsawg-teas-common-ac-11, but -12 is the latest
available revision.
Document references draft-ietf-netmod-rfc8407bis-14, but -20 is the latest
available revision.
Document references draft-ietf-opsawg-ntw-attachment-circuit-11, but -13 is the
latest available revision.
Document references draft-ietf-teas-ietf-network-slice-nbi-yang-13, but -16 is
the latest available revision.
Document references draft-ietf-idr-bgp-model-17, but -18 is the latest
available revision.
"OPSAWG", paragraph 0
> 024 YANG Data Models for Bearers and 'Attachment Circuits'-as-a-Service (ACa
> ^
Unpaired symbol: "'" seems to be missing.
Section 1.1, paragraph 3
> roviding programmatic means to expose 'Attachment Circuits'-as-a-Service (ACa
> ^
Unpaired symbol: "'" seems to be missing.
Section 9.2, paragraph 1
> The attachment circuit in this case use a SAP identifier to refer to the phy
> ^^^
The verb form "use" does not seem to match the subject "case".
Mahesh Jethanandani
mjethanand...@gmail.com
_______________________________________________
OPSAWG mailing list -- opsawg@ietf.org
To unsubscribe send an email to opsawg-le...@ietf.org
