Hi, Mahesh:
发件人: Mahesh Jethanandani [mailto:[email protected]]
发送时间: 2022年2月8日 3:17
收件人: Qin Wu <[email protected]>
抄送: [email protected]; [email protected]; 
[email protected]; [email protected]
主题: Re: Yangdoctors last call review of draft-ietf-netmod-node-tags-04

Hi Qin,


On Feb 6, 2022, at 6:10 PM, Qin Wu 
<[email protected]<mailto:[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]<mailto:[email protected]>
抄送: 
[email protected]<mailto:[email protected]>;
 [email protected]<mailto:[email protected]>; 
[email protected]<mailto:[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.

[Qin Wu] Thanks


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.

[Qin Wu] okay and will do 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.

[Qin Wu] Okay, I will take similar approach in RFC8819 and update examples with 
other type tags in the appendix to reflect your comments. Thanks!
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<http://datatracker.ietf.org> 
and not tools.ietf.org<http://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]<mailto:[email protected]>





_______________________________________________
netmod mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/netmod

Reply via email to