|
Hi Per, Thank you for the valuable review. The -19 version has been submitted to address all the comments: Diff:
https://author-tools.ietf.org/iddiff?url2=draft-ietf-teas-ietf-network-slice-nbi-yang-19 Please also see my responses inline. Regards, Bo -----Original Message----- From: Per Andersson via Datatracker <noreply@xxxxxxxx>
Sent: Wednesday, January 22, 2025 10:27 PM To: ops-dir@xxxxxxxx Cc: draft-ietf-teas-ietf-network-slice-nbi-yang.all@xxxxxxxx; last-call@xxxxxxxx; teas@xxxxxxxx Subject: [Teas] Opsdir last call review of draft-ietf-teas-ietf-network-slice-nbi-yang-18 Reviewer: Per Andersson Review result: Has Issues Hi! I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review. Document editors
and WG chairs should treat these comments just like any other last call comments. Result: Ready with issues Summary: The document defines a YANG model for RFC 9543 Network Slice Services. This YANG model can be used in the Network Slice Service interface between a customer and provider that offers RFC 9543 Network Slice Services. I find the document clearly explaining the concepts. The YANG model, being fairly big, is clearly presented to the reader. Great examples in Section 5.2.4 guiding performance monitoring with YANG-Push or NETCONF and RESTCONF. Exceptional clarity suggesting how to configure such monitoring for the mentioned technologies. Major issues: I wonder how the /network-slice-services/slice-service/sdps/sdp\ /service-match-criteria/match-criterion/match-type/value leaf-list values will be used in practice. The type is "string" without any constrains and the description states "Provides a value for the Slice Service match
criteria, e.g., IP prefix, VLAN ID, or ACL name." As a model consumer I would expect the type to enforce the valid values depending on type, e.g. choice with inet:dscp, inet:ip-prefix, string with length "1..64" for acl name etc. Since there are no restrictions on
valid values, what happens when an invalid value is entered; e.g. empty string? Having the constraints defined in YANG enable errors to be found early. There is no discussion of error cases for the service-match-critera, only the examples in the appendix. [Bo Wu]Thank you for identifying this critical issue and providing modification suggestions. In the -19 version, the new YANG code has been added as follows: choice value { .. case dscp { when "derived-from-or-self(type, 'dscp')" { description "Match type is a DSCP value."; } leaf-list dscp-value { type uint8 { range "0..63"; } description "DSCP value for the match criteria."; } } case acl { when "derived-from-or-self(type, 'acl')" { description "Match type is an ACL name."; } leaf-list acl-name { type string { length "1..64"; } description "ACL name value for the match criteria."; } } Minor issues: The node "compute-only" might be better recognized if it was named "dry-run". This is commonly used in network products and also in other computer science and engineering fields. Also, the <test-option> element mentioned needs to be supported, i.e. the :validate:1.1 capability needs to be advertised). Suggest mentioning that this only works if it is supported by the server. [Bo Wu]Thank you for this suggestion. In the -19 version, we are considering to use
“test-only”
to replace "compute-only", because the AC service model, approaching RFC publication, named the same mechanism as
‘test-only,’
aligning with the NETCONF. On the ‘:validate:1.1’
capability, we think this constraints is already described in the NETCONF
‘test-only’ referenced
section, which is clear. Figure 9: Application of Match Critera; The formatting is off for the overarching NSS descriptor and it would be beneficial with a legend explaining the "x" and "%" symbols. I guess they indicate different matching
criterias per connectivity construct between SDP16 and SDP17? [Bo Wu]Thanks for the suggestion. The -19 version has been updated accordingly. The example in Figure 23 for establishing a YANG-Push subscription over RESTCONF is wrong. Since YANG-Push is used, "ietf-yang-push:datastore" needs to be in the input parameters and the correct subtree filter tag is "ietf-yang-push:datastore-subtree-filter" and not "stream-subtree-filter". Furthermore, since the example is using YANG-Push over RESTCONF, add a normative reference to RFC 8650. [Bo Wu] Thanks for catching this issue. We have corrected with "ietf-yang-push:datastore-subtree-filter".
And we also added RFC 8650 as informative reference, since this is only a example and the YANG model not limited to RESTCONF and NETCONF, also RESTful etc. The example in Figure 35 includes the JSON "status": {} But in ac-common "status" is defined as a non-presence container. It should probably not be visible if no child node exists. I know that some implementations might show NP containers even when they have no child nodes,
but this is confusing as far as an example in an RFC goes IMHO. [Bo Wu] Thanks again for identifying this issue. In -19 version, all the examples in the appendix, we added slice-level
“admin-status”
and removed the empty “status”
at other levels. We hope this is clear. "status": { "admin-status": { "status": "ietf-vpn-common:admin-up" } The following idnits warnings should be attended: draft-ietf-teas-ietf-network-slice-nbi-yang-17.txt: Miscellaneous warnings: ---------------------------------------------------------------------------- == The copyright year in the IETF Trust and authors Copyright Line does not match the current year == Line 5329 has weird spacing: '... cos-id uin...' == Line 5347 has weird spacing: '... cos-id uin...' == Line 5390 has weird spacing: '... cos-id uin...' == Line 5408 has weird spacing: '... cos-id uin...' -- The document date (20 December 2024) is 32 days in the past. Is this intentional? Checking references for intended status: Proposed Standard ---------------------------------------------------------------------------- (See RFCs 3967 and 4897 for information about using normative references to lower-maturity documents in RFCs) == Outdated reference: A later version (-19) exists of draft-ietf-opsawg-teas-attachment-circuit-18 == Outdated reference: A later version (-14) exists of draft-ietf-opsawg-teas-common-ac-13 ** Downref: Normative reference to an Informational RFC: RFC 9543 Summary: 1 error (**), 0 flaws (~~), 7 warnings (==), 1 comment (--). [Bo Wu] Thank you for reminding us. There is one issue not sure that the spacing issues are from the YANG tree generated by pyang. Perhaps wrong warning? We fixed copyright to 2025. And the AC references will be fixed when the AC document is published.
Thanks for your contribution! -- Per _______________________________________________ Teas mailing list -- teas@xxxxxxxx To unsubscribe send an email to teas-leave@xxxxxxxx |
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
