Hi Qin, > On Feb 6, 2022, at 6:10 PM, Qin Wu <[email protected]> wrote: > > Thanks Mahesh for valuable review. Please see reply inline below. > > -Qin >> -----邮件原件----- >> 发件人: Mahesh Jethanandani via Datatracker [mailto:[email protected]] >> 发送时间: 2022年2月1日 13:25 >> 收件人: [email protected] >> 抄送: [email protected]; [email protected]; >> [email protected] >> 主题: Yangdoctors last call review of draft-ietf-netmod-node-tags-04 > >> Reviewer: Mahesh Jethanandani >> Review result: On the Right Track > >> Summary: > >> This document defines a method to tag data objects associated with operation >> and management data in YANG Modules. This YANG data object tagging method >> can be used to classify data objects from different YANG modules and >> identify characteristics >> data. > >> Nits > >> /subobjects/sub-objects/g >> [Qin] Okay. >> Comments: > >> If the document updates RFC 8407, it needs to mention that in the Abstract. >> Also the abstract can be shortened to what the document defines, and move >> everything else into the introduction. > > > [Qin] Good point, similar comment was brought up by Adrian, I will make > Abstract short. > >> The document says "This document defines an extension statement ...". Is >> only extension statement defined? > [Qin]:I am not sure I capture your comment. But this document define one YANG > model, three extension statements and one IANA registry for IETF tags. Maybe > I should tweak this sentence as follows: > "This document defines three extension statements..."
Yes, that would be helpful. > >> Text like "data object tags may be registered as well as assigned during >> module definition" follow the pattern of RFC 8819 and should be referred to >> rather than duplicated. > [Qin]:Agree to reference to RFC8819, but this document focuses on data object > tags while RFC8819 focuses on model tag. I will see how to tweak the text to > reflect your comment. > >> If assigned during implementation, is there a possibility that the same tag >> is assigned by two different implementations? What is the scope of a given >> data object tag? > [Qin]: Note that the data object tags aim at data object classification. > Therefore the same tag can be assigned even by one implementation to > different data nodes. If the tag is the IETF tag defined in this document, we > need to make sure different > implementation or different device can assign the same tag to the same data > node in the module. For IETF tag, we should make sure the tag is unique. Same > rule is applied to other vendor tag or user tag, But we don't suggest to use > IETF tag together with > either vendor tag or user tags. Hope this clarifies. > >> Similarly, the draft says "objects can be one of container, leaf-list and >> list". Did you mean to say "objects can be one of type container, leaf-list >> and list"? > [Qin]: Correct, I can tweak the text as you suggested. > >> The example in Figure 2 can be improved. For example, if all the data >> objects are for the module name "tunnel-pm", do you need the last column. > [Qin]: I can take out the last column or keep the last column and remove > duplicated text in each row by merging all the rows associated with last > column into one row. > >> More importantly, it is not clear why tunnel-src/max-latency (why a gap >> between / and max-latency), is not an object tag? Can a sub-object tag exist >> if the node is not an object tag? > > [Qin] As shown in figure 1 and figure 2, you will see only root node will be > tagged as object tag, in figure 2, only tunnel-svc can be seen as root node, > tunnel-svc/max-latency is just a child node and therefore can not be tagged > with *theobject tag *. Please also refer to section9.2 table for clear > definition of object tag. > Secondly sub-object tag and object tag can not tag the same node, only root > node will be tagged with object tag, Sub-object will be tagged with > sub-object tags such as property tag, metric tag, metric-type tag, > multi-source tag. > >> In Section 4, Data Object Tag Values, it says tags can be any value except >> carriage-returns, newlines and tabs. Does it mean spaces are allowed? Can a >> data object have multiple tags? What does it mean "No further structure is >> imposed ..."? > [Qin]: I think tabs is similar to spaces, maybe 2 spaces or 4 spaces but with > less disk space / memory / compiler resource. In that case, it would be helpful to mention it so. > Secondly, a data object can have multiple tags, see figure 1, a data object > can have one metric tag, one metric-type tag or one multi-source tag. > Third, no further structure is imposed means we don't further define detailed > format for the value following "ietf:" or "vendor:" or "user:", it can be any > YANG type 'string', e.g., we will not require the value following "ietf:" to > start from 'AAA' or 'BBB'. > >> Section 4.2 introduces the concept of vendor prefix for tags. It says >> vendors include extra identification in the tag to avoid collision. But what >> is to say that two organizations may not use the same identification? And is >> this identifier part of the tag or is >> separated from the tag with a :. > [Qin] Yes, Each vendor or organization can define its own extra > identification, such identifier can be part of the vendor tag. > >> Similarly, it says that user prefix is RECOMMENDED. If not using it can >> cause collision, why is use prefix RECOMMENDED and not a MUST? > > [Qin] Becos user prefix has two forms, one is prefixed with "user:", the > other is without prefix "user:", that is why we choose RECOMMENDED rather > than MUST. > > >> The draft has just one example. And it shows mostly ietf prefixed tags. More >> examples showing use of different types of tags are needed. It would be >> helpful to know how tags can be removed. > > > [Qin] In this document, only IETF prefixed tags are registered, user tags and > vendor tags are not predefined or registered, in addition, the example for > vendor tag and user tag has no big Difference with ietf prefixed tags. That > is why we don't provide example for other type tags. The idea of providing examples is to demonstrate how different tags, specially those that are not predefined or registered, are defined/used. It can be in the Appendix and therefore informative in nature. Thanks. > > Secondly, similar to module tag defined in RFC8819, we defined a list of > masked-tags in the module tag extension module in this document which allow > user remove tag from operational datastores. > > >> Section 5 - YANG Module. > >> The section does not reference the RFCs that it imports modules from, e.g. >> ietf-netcom-acm. > [Qin]: Good catch, will add. >> Inside the YANG model, import statements need to carry reference statement. > [Qin]:Okay. >> The WG link needs to refer to datatracker.ietf.org and not tools.ietf.org > [Qin]:Good catch, will fix this. >> The Copyright statement has 2021 as the year. > [Qin]:Okay >> Line length should be limited to 72 columns. > [Qin]:Okay >> No need to repeat parent name in child node, e.g. object-name -> name. > [Qin]:Okay. >> Indentation is off in places, specially in the example. > [Qin]:Okay. >> A pyang compilation of the model with —ietf and —lint option was clean. > >> A idnits run of the draft reveals a few issues. Please address them. > [Qin]:thanks, will fix the in v-05. >> draft-ietf-netmod-node-tags-04.txt: > >> Checking boilerplate required by RFC 5378 and the IETF Trust (see >> https://trustee.ietf.org/license-info): >> ------------------------------------------------------------------- > >> No issues found here. > >> Checking nits according to >> https://www.ietf.org/id-info/1id-guidelines.txt: > ------------------------------------------------------------------- > >> No issues found here. > >> Checking nits according to https://www.ietf.org/id-info/checklist : > ------------------------------------------------------------------- > >> ** There are 70 instances of too long lines in the document, the >> longest one being 15 characters in excess of 72. > >> -- The draft header indicates that this document updates RFC8407, > but the abstract doesn't seem to mention this, which it should. > >> Miscellaneous warnings: > ------------------------------------------------------------------- > >> == The copyright year in the IETF Trust and authors Copyright Line > does not match the current year > >> == Line 404 has weird spacing: '...ct-name nac...' > >> == Line 493 has weird spacing: '...dentify multi...' > >> == Line 656 has weird spacing: '...resents a pro...' > >> == Line 999 has weird spacing: '...dentify multi...' > >> -- The document date (November 2021) is 77 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) > >> == Missing Reference: 'I-D.netconf-notification-capabilities' is >> mentioned on line 139, but not defined > >> == Missing Reference: 'I-D.ietf-netmod-yang-instance-file-format' >> is mentioned on line 1106, but not defined > >> Summary: 1 error (**), 0 flaws (~~), 7 warnings (==), 2 >> comments (--). > >> Run idnits with the --verbose option for more detailed >> information about the items above. > > Mahesh Jethanandani [email protected]
_______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
