Thank you Michal -éric
From: "mva...@cesnet.cz" <mva...@cesnet.cz> Date: Thursday, 15 December 2022 at 08:42 To: Jean Quilbeuf <jean.quilb...@huawei.com>, Eric Vyncke <evyn...@cisco.com>, The IESG <i...@ietf.org>, <benoit.cla...@huawei.com> Cc: "draft-ietf-opsawg-service-assurance-y...@ietf.org" <draft-ietf-opsawg-service-assurance-y...@ietf.org>, "opsawg-cha...@ietf.org" <opsawg-cha...@ietf.org>, "opsawg@ietf.org" <opsawg@ietf.org>, "m...@sandelman.ca" <m...@sandelman.ca>, "tpa...@apple.com" <tpa...@apple.com> Subject: Re: Éric Vyncke's Discuss on draft-ietf-opsawg-service-assurance-yang-10: (with DISCUSS and COMMENT) Hi, I have looked at the changes and they seem fine, I could not find any major problems except for the file revisions not matching the YANG revisions, for all the modules, so please remember to fix that when you do the next update. file "ietf-service-assura...@2022-04-07.yang"<mailto:ietf-service-assura...@2022-04-07.yang> ... revision 2022-08-10 { description "Initial version."; reference "RFC xxxx: YANG Modules for Service Assurance"; } Also, I have found some restrictions in descriptions of some nodes that can be expressed in YANG (were already in draft v6 but I have missed it then), but it is up to you to decide whether that makes sense or not. In general, it is always better because validating tools are then able to check the constraint automatically. The affected nodes are: leaf assurance-graph-last-change (depends on last-change) leaf health-score-weight (depends on health-score or rather the other way around) leaf stop-date-time (depends on start-date-time) Regarding the comments and proposed changes, I have nothing to add, all my potential comments have already been mentioned. Regards, Michal On 15. 12. 2022 1:54, Jean Quilbeuf wrote: Hi Eric, Thanks for your comment. No new draft revision yet, but you’ll find below answers to discuss and comment, including modification that we plan to do. Michal Vaško did the early YANG doctor review. Since there are modifications planned to the YANG module in this mail, I added him in Cc. Best, Jean -----Original Message----- From: Éric Vyncke via Datatracker [mailto:nore...@ietf.org] Sent: Tuesday 13 December 2022 08:13 To: The IESG <i...@ietf.org><mailto:i...@ietf.org> Cc: draft-ietf-opsawg-service-assurance-y...@ietf.org<mailto:draft-ietf-opsawg-service-assurance-y...@ietf.org>; opsawg- cha...@ietf.org<mailto:cha...@ietf.org>; opsawg@ietf.org<mailto:opsawg@ietf.org>; m...@sandelman.ca<mailto:m...@sandelman.ca>; m...@sandelman.ca<mailto:m...@sandelman.ca>; tpa...@apple.com<mailto:tpa...@apple.com> Subject: Éric Vyncke's Discuss on draft-ietf-opsawg-service-assurance-yang- 10: (with DISCUSS and COMMENT) Éric Vyncke has entered the following ballot position for draft-ietf-opsawg-service-assurance-yang-10: Discuss 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/about/groups/iesg/statements/handling-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-service-assurance-yang/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- # Éric Vyncke, INT AD, comments for draft-ietf-opsawg-service-assurance- yang-10 CC @evyncke Thank you for the work put into this document. A very interesting piece of work and a well-written piece of text (even if I am balloting DISCUSS). The examples are also helping. Please find below some DISCUSS points (+ suggestions), some non-blocking COMMENT points (but replies would be appreciated even if only for my own education). Special thanks to Michael Richardson for the shepherd's detailed write-up including the WG consensus *but* it lacks the justification of the intended status. It would have been nice to list the implementations (even if I know one). Please note that Tommy Pauly is the Internet directorate reviewer (at my request) and you may want to consider this int-dir reviews as well when Tommy will complete the review (no need to wait for it though): https://datatracker.ietf.org/doc/draft-ietf-opsawg-service-assurance- yang/reviewrequest/16806/ Also, thanks to the WG chairs and the responsible AD to bundle this I-D and its companion to the same IESG telechat: it helps a lot! I hope that this review helps to improve the document, Regards, -éric ## DISCUSS As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a DISCUSS ballot is a request to have a discussion on the following topics: ### BCP14 template As noted by https://author- tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf- opsawg-service-assurance-yang-10.txt and Lars, the BCP14 template is not correct even if it is required for a proposed standard (it mentions BCP13 ;-) ). As I have further DISCUSS issues below, I am raising the trivial BCP14 issue to a blocking DISCUSS. Thanks to Lars and you for pointing that out. We will fix it. ### Section 3.3 To my SQL eyes, it hurts to use a -1 value for health-score when there is no value. There is no "mandatory true" statement for this leaf, i.e., it can be absent in the telemetry. Is there a semantic difference between the absence of health-score and the value of -1 ? Is the SAIN collector expected to process those 2 cases differently ? Suggest to either remove the -1 sentinel value, or add "mandatory true" attribute, or be specific about the difference (if any). We propose to change to: leaf health-score { type int8 { range "-1 .. 100"; } config false; mandatory true; default -1; description "Score value of the subservice health. A value of 100 means that subservice is healthy. A value of 0 means that the subservice is broken. A value between 0 and 100 means that the subservice is degraded. A value of -1 means that the health-score could not be computed."; } We need the -1 to be able to state that the health-status is not available anymore, notably for some model-driven telemetry protocol that do not support stating the fact that a node does not exist anymore, but only that the value of that node has changed. ### Section 4 It is unclear from this section whether it applies to IETF-specified YANG modules only? I.e., may a vendor augment this IETF YANG module in its own namespace ? I guess so, but worth writing it (or restrict this section to future IETF work only). 1) Yes, we need to specify that we focus in IETF-specified modules only in this section. We propose to change: The base YANG module defined in Section 3.3 only defines a single type of subservices that represent service instances. As explained above, this model is meant to be augmented so that a variety of subservices can be used in the assurance graph. In this section, we propose some guidelines in order to build theses extensions. To The base YANG module defined in Section 3.3 only defines a single type of subservices that represent service instances. As explained above, this model is meant to be augmented so that a variety of subservices can be used in the assurance graph. In this section, we propose some guidelines for specifying such extensions at IETF. 2) Yes, we need to specify that a vendor can define subservices using they own namespace. We propose to add the following as last paragraph for section 4. Vendors can specify their own subservices types by defining the corresponding modules in their own namespace. An example of such a vendor-specific module is specified in Appendix A. Vendors can also augment existing IETF-specified subservices to add their own vendor-specific information. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- ## COMMENTS ### Downref As noted by https://author- tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf- opsawg-service-assurance-yang-10.txt and Lars, there is a downward reference that was not mentioned in the IETF LC. I will leave that one to Benoît in his answer to Lars. ### Section 1 ``` [I-D.ietf-opsawg-service-assurance-architecture] specifies an architecture and a set of involved components for service assurance, called Service Assurance for Intent-Based Networking (SAIN). ``` The SAIN architecture draft is informational, so it cannot "specify". Please use "describes". Will change ### Section 3.1 ``` The two subservices presented above need different sets of parameters to fully characterize one of their instance. An instance of the device subservice is fully characterized by a single parameter allowing to identify the device to monitor. For ip-connectivity subservice, at least the device and IP address for both ends of the link are needed to fully characterize an instance. ``` s/two subservices presented/two example subservices presented/ Will change While I am not English native speaker, I wonder whether the plural form should be used for "IP address for both ends" ? Will check and adapt `The dependencies are modelled as an adjacency list,` or simply `The dependencies are modelled as a list,` ? I would like to keep "adjacency list" because of https://en.wikipedia.org/wiki/Adjacency_list ### Section 3.2 ``` The date of last change "assurance-graph-last-change" is read only. It must be updated each time the graph structure is changed by addition or deletion of subservices, dependencies or modification of their configurable attributes. ``` Is 'under-maintenance' a triggering change to update the last change time ? Perhaps worth mentioning Yes it is, I will add it. ### Section 3.3 Should the under-maintenance.contact be more specified? I.e., as a URI like phone:+12309182309 or mailto:j...@example.net ? One goal of this document (section 1) is to be 'machine readable' ;-) We had many discussions about it, maybe URI is the good solution. We did not find any existing yang type to model a contact. leaf health-score-weight, the use of this leaf is rather under specified. Should it rather be a float between 0.0 and 1.0 ? I also wonder whether the last sentence of the description applies to this leaf ? We use 0..100 to be aligned with the health-score. I will let to my fellow ART AD to check whether the waiver `Therefore, no mechanism for language tagging is needed.` is acceptable for lack of i18n support (for me: it is ok). ### Section 5 Is a device always a 'physical' device or can it be virtual as well ? Should there be a link to the miscellaneous 'inventory models' in OPSAWG ? If not, should there be more information about the device, e.g., its location. There should be an inventory mapping the "device" parameter to other information such as location. I don’t think we have an answer yet in OPSAWG for such a model . ### Section 6 I am not familiar enough with YANG, but should there be some text or YANG statement to declare the 'device' leaf in this module as a foreign key to the device module ? Not in this case, since the ACME device subservice is totally independent from the IETF device subservice. However, another option would have been for ACME to extend the IETF device subservice, in which case the "device" leaf would have been the same in both models. Should the interface model handle the sub-interface (e.g., a specific VLAN on a GigEthernet interface) ? Following ietf-interfaces YANG module, the name of the interface (a string) is sufficient to be the key identifying an interface on a given device, including virtual interfaces stacked on top of other interfaces. So I don’t think that we need to model it. I will add this information in the in the text. ### Appendix C Thanks for using an IPv6 example ;) ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments
_______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
