|
Hi Mahesh, Many thanks for your review. Please see my responses inline. Regards, Bo -----Original Message----- A couple of more comments on the YANG types. > On Jan 26, 2025, at 6:35 AM, Per Andersson <per.ietf@xxxxxxxx> wrote: > > Hi Bo! > > On Sun, Jan 26, 2025 at 12:35 PM Wubo (lana) <lana.wubo@xxxxxxxxxx>
wrote: >> >> 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 > > This import has changed prefix from what is in the originat module
> ietf-te-packet-types from RFC 8776 from "te-packet-types" to "te-pkt". > > import ietf-te-packet-types { > - prefix te-pkt; > + prefix te-packet-types; > > I suggest to revert this change and use the modules prefix. The
> benefit in a familiar module prefix is much larger than the cost of the extra characters. RFC BBBB, RFC CCCC, and RFC DDDD, are cited in the YANG module, but there are no references to them normatively. Instead, the actual draft names are listed in Normative References. There should be some note in the
draft that should say that RFC BBBB maps to I-D.ietf-opsawg-teas-attachment-circuit, etc. [Bo Wu] Thank you for identifying this issue. In version r20, I have added references to the three documents in the Introduction section. Diff:
https://author-tools.ietf.org/iddiff?url2=draft-ietf-teas-ietf-network-slice-nbi-yang-20 There are several identities defined in the module. Are these identities specific to network slicing or could they be used by other modules? If so, it would make sense to have an IANA module for the type definitions. [Bo Wu] I think most identities are only applicable to the Network Slice Service model. However, 'service-slo-metric-type' might be used by other models. That said, I wonder if it would
be better to use the groupings 'service-slos' and 'service-sles' defined in the model, or the 'slo-sle-template'? For example, draft-ietf-ccamp-yang-otn-slicing-08 (see https://datatracker.ietf.org/doc/html/draft-ietf-ccamp-yang-otn-slicing-08#section-5.2.2)
reuses the 'slo-sle-template' from the network slicing service model. Cheers. > > >> 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: > > Good! > > >> 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. > > Good to me! > > >> 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. > > Thanks! > > >> 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". > > You still need to add the "ietf-yang-push:datastore" node to the
> establish-subscription RPC input parameters, e.g. something like > > "ietf-yang-push:datastore":"ietf-datastores:running" > > >> 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. > > Of course, I just want the example in Figure 23 to be correct YANG-Push. > > >> 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" >> >> } > > Yes, good! > > >> 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. > > I just thought I would at least mention them, thanks! > > I don't know about the YANG warning, maybe that should be looked into
> by YANG Doctors. > > > -- > Per > > _______________________________________________ > OPS-DIR mailing list --
ops-dir@xxxxxxxx To unsubscribe send an email
Mahesh Jethanandani |
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
