Thanks Adrian for valuable review, please see rely inline below.
-Qin
-----邮件原件-----
发件人: Adrian Farrel [mailto:[email protected]]
发送时间: 2022年1月16日 20:23
收件人: [email protected]
抄送: [email protected]
主题: A review of draft-ietf-netmod-node-tags
>Hi,
>Thanks for this document which I found clear and easy to read. I think you
>have defined a small, but particularly valuable feature.
>I have a few comments and a large number of nits (sorry).
>Best,
>Adrian
>== Major ==
>There is some contradiction between and within sections 4.3 and 4.4
>1. If a user tag is defined as any tag that has the prefix "user:"
> how can you then day that users are not required to use the "user:"
> prefix? That would mean that a user tag is any tag that does or
> does not have the prefix "user:"
[Qin Wu] Correct.
>2. If any tag not starting with "ietf:", "vendor:", or "user:" is
> reserved, how can a user create a tag that doesn't start with
> "user:"?
[Qin Wu] :I understand your concern, but my understanding on reserved tag, upon
such reserved tag is allocated, it should start with "xxx:",
Therefore such reserved tag can not be seen as user tag. It seems
unlikely there is contraction if my understanding is correct.
Correct me if I am wrong.
>3. Section 4.4 could usefully point at Section 9.1 and say that
> "further tags may be assigned by future specifications".
[Qin Wu] :Useful to add reference to Section9.1 and thanks.
---
>Section 9.1 reasonably uses the "Specification Required" assignment policy.
>But, according to 8126, that policy requires the appointment of a designated
>expert. According to section 5.3 of 8126...
> When a designated expert is used, the documentation should give clear
> guidance to the designated expert, laying out criteria for performing
> an evaluation and reasons for rejecting a request.
>So you need to add a section to cover this. It can be simple if the rule is
>"read the specification, check it is permanent and readily available, and
>watch for inappropriate use of language." Or it might be more complex if you
>want the DE to check more stuff.
[Qin Wu] Thanks for the reference, I re-read section 5.3 of RFC8126, it also
said:
"
In the case where
there are no specific documented criteria, the presumption should be
that a code point should be granted unless there is a compelling
reason to the contrary (and see also Section 5.4).
"
My understanding is documented criteria is not mandatory, if you have code
point value (i.e., prefix value in section 9.1)that needs to be allocated based
on IANA request.
I think section 9.1 may fall into this case.
For section 9.2, I agree to add one rule, i.e., "IANA assigned tags must
conform to Net-Unicode as defined in [RFC5198], and they shall not need
normalization". I believe this is the guidance given to the designated expert.
Hope this address your comment.
>== Minor ==
>
>The Abstract need to explicitly call out "This document updates RFC 8407 by
><doing what?>
[Qin Wu] I think it is by " providing guidance to future model writers ", this
require update to RFC8407, I will make this clear in the text.
>The Introduction should repeat that statement and add some detail. In fact we
>don't even find a mention of 8407 until Section 8, and even then there is no
>hint about what the "update" is. Although, in Section 1 you have
[Qin Wu] Will update introduction to add text about RFC8407 update.
Section 8 provides guidelines for authors of YANG data models.
>That would be the ideal place to describe the update.
[Qin Wu] Agree. Thanks.
---
>Figure 1 and its text are a little confusing.
>1. It would help to lift the top Object Tag one line so there is a pipe
> ('|') to separate it from Data Object 1
[Qin Wu] Fixed.
>2. The term "have" is not explained. Given the direction of the arrow,
> I think this means "is contained by".
[Qin Wu] I proposed to change have to contain since a parent data object can
contain multiple child data objects.
>3. The text says "In Figure 1, data objects can contain other data
> objects called subobjects." That's fine, but it would be easier to
> read the figure is data objects 2, 3, and 4 were marked as
> sub-objects.
[Qin Wu] Good point, will update to reflect your comment.
>4. The text has:
> A data object may contain one single object tag, or one single
> property tag, or one single metric tag. In many cases, a data object
> only contains one single metric tag.
> That's a bit odd. I mean, the first sentence says, "A, B, or C", and
> the second sentence says "In many cases C". How does the second
> sentence add anything?
[Qin Wu] Agree, I will remove the second sentence.
>5. The text has:
> the data object tagged
> with the metric tag also can have one or multiple MetricType tags
> and/or one single multi-source tag.
> These additional tags are not shown on the figure.
[Qin Wu] Good catch, will update the figure with additional tags.
---
>Section 5 has
> As the main consumer of
> data object tags are users, users may also remove any tag from a live
> server, no matter how the tag became associated with a data object
> within a YANG module.
>Suppose there are two users accessing the same YANG data objects on a live
>server. This doesn't seem unreasonable in the case of different users or
>monitoring tools reading data about the network or devices.
>Doesn't this text lead to "warring tag removal" where one user adds a tag, and
>another user removes it?
>Maybe this is limited to user tags so that each user may have their own tags.
>But, in this case, it needs to be clearer what a user tag contains and how it
>is used.
>It would still be pretty annoying is Benoit added user:benoit to some data
>objects, and I went and removed them.
[Qin Wu] Yes, I believe it is limited to user tags, since IETF tags are design
time tags, so does implementation tags. It is unlikely to face the situation
"warring tag removal".
But for user tag, your are right, user has its own tags and each user may have
different privilege therefore. User with low privilege can not remove the tag
owned by high privilege user.
But I am not sure this is the scope of this document, It seems to me
implementation specific and should not in the scope of this document, agree?
(See also section 5.3)
>...Reading the YANG module itself, I wonder whether "add" and "remove"
>are ambiguous. Sometimes you may mean adding or removing a tag to/from a data
>object. Sometimes you may mean adding or removing to/from a filter.
[Qin Wu] I think we are referred to the former. Filter is used to sort data
objects. I am not feeling ambiguous unless you think otherwise.
---
>9.1
>OLD
> Other standards organizations (SDOs) wishing to allocate their own
> set of tags should allocate a prefix from this registry.
>NEW
> Other standards organizations (SDOs) wishing to allocate their own
> set of tags should request the allocation of a prefix from this
> registry.
>END
[Qin Wu] Fixed.
---
>9.2
> Each YANG data object can have one 'opm' tag, zero or one metric-type
> tag, zero or one multi-source tag.
>Is this an instruction for IANA for the management of the registry? I don't
>think it is, and so it should be removed from this section.
[Qin Wu] Agree to remove, thanks.
---
>9.3 and 9.4
>These sections include the text "the following registration has been made:",
>but it hasn't been (yet). You just need to phrase this text as a request.
[Qin Wu] Will fix this.
---
>Should section 10 talk about the risks associated with an attacker adding or
>removing tags so that a requester gets the wrong data?
[Qin Wu] User tag is not registered, how user tag is defined and removed is not
scope of this document, in my opinion. Take a look at RFC8819, RFC8819 also
doesn't flag this as a issue, do you think we should do this?
>Should the section also talk about how the presence of tags may reveal
>information about the way in which data objects are used and so providing
>access to private information or revealing an attack vector?
[Qin Wu] Agree, I proposed to add the following text
OLD TEXT:
"
The Network Configuration Access Control Model (NACM) [RFC8341] provides the
means to restrict access for particular NETCONF or RESTCONF users to a
preconfigured subset of all
available NETCONF or RESTCONF protocol operations and content.
"
NEW TEXT:
"
The Network Configuration Access Control Model (NACM) [RFC8341] provides the
means to restrict access for particular NETCONF or RESTCONF users to a
preconfigured subset of all
available NETCONF or RESTCONF protocol operations and content, e.g., the
presence of tags may reveal information about the way in which data objects are
used and therefore providing
access to private information or revealing an attack vector should be
restricted.
"
>== Nits ==
>Throughout
>You need to choose between "self describing" and "self-describing".
>The hyphenated version is probably correct.
[Qin Wu] I will choose to use "self-describing", thanks.
---
>Abstract
>s/input, instruction, indication/input, instructions, indications/ s/selection
>filter/selection filters/ s/is very large/are very large/
[Qin Wu]:Fixed.
---
>1.
>s/associated only with a module/associated with a module/
[Qin Wu]:Fixed.
---
>1.
> At the time of writing this document (2020), there are many data
> models that have been specified or are being specified by various
> SDOs and the Open Source community.
>How about...
> Many data models have been specified by various SDOs and the Open
> Source community and it is likely that many more will be specified.
[Qin Wu]:Accepted, thanks.
---
>1.
>OLD
> management categories information at different locations in various
> different ways.
>NEW
> management information at different locations and categorised in
> various different ways.
>END
>OLD
> Let alone lack consistent classification criteria
> and representation for a specific service, feature, or data source.
>NEW
> Further, there is no consistent classification criteria or
> representation for a specific service, feature, or data source.
>END
[Qin Wu]:Fixed.
---
>1.
>s/Indicate relationship/Indicate a relationship/ s/identify characteristics
>data/identify characteristic data/ s/In addition, it can provide/In addition,
>they can provide/ s/input, instruction, indication/input, instructions,
>indications/ s/selection filter/selection filters/
[Qin Wu]:Fixed.
---
>1.
> e.g., return specific object type of operational state
> related to system-management.
>I think this may be meant to say
> e.g., return specific objects containing operational state
> related to system-management.
[Qin Wu]:Fixed.
---
>1.
>s/of <get-schema>/of the <get-schema>/
[Qin Wu]:Fixed.
---
>1.
> or some place where offline document are kept
>I don't think a "place" advertises. Maybe "an application or server where
>offline documents are kept"
[Qin Wu]:I think somewhere can be a website, Maybe or websites where offline
documents are kept, make sense?
---
>1.
> This document defines a YANG module [RFC7950] which augments the
> module tag model
>Would another reference to 8819 be appropriate?
[Qin Wu]:OK.
---
>2.2
> OPM Object Property Metric
>Should that be
> OPM Object, Property, Metric
[Qin Wu]:Fixed.
---
>3.
>OLD
> Among data object tags, the 'opm' (object, property, metric) tags can
> be used to tackle massive data objects collection, indicate
> relationship between data objects and only capture YANG data objects
> associated with performance metrics data modelled with YANG
> (Figure 1).
>NEW
> Among data object tags, the 'opm' (object, property, metric) tags can
> be used to tackle massive data object collections, indicate
> relationships between data objects, and capture YANG data objects
> associated with performance metrics data modelled with YANG
> (as shown in Figure 1).
>END
>OLD
> Figure 1: The Relation between Object, Property and Metric NEW
> Figure 1: The Relation between Object, Property, and Metric END
[Qin Wu]:Fixed.
---
>3.
>s/In Figure 2, 'tunnel-/In Figure 2, the 'tunnel-/
[Qin Wu]:Fixed.
---
3.
>OLD
> Consider
> 'tunnel-svc' data object and tunnel-svc/name data object as an
> [Qin Wu] example, 'tunnel-svc' data object has one single object tag (i.e.,
> 'ietf:object') while tunnel-svc/name data object has one single
> property subobject tag (i.e., 'ietf:property').
>NEW
> Consider
> the 'tunnel-svc' data object and the tunnel-svc/name data object as
> an example: the 'tunnel-svc' data object has one single object tag
> (i.e., 'ietf:object') while the tunnel-svc/name data object has one
> single property subobject tag (i.e., 'ietf:property').
>END
>OLD
> e.g., only specific category
> such as loss-related metric subobjects need to be tagged with metric-
> [Qin Wu] type tag which can further reduce amount data to be fetched.
>NEW
> e.g., specific categories,
> such as loss-related metric subobjects, need to be tagged with a
> metric-type tag which can further reduce amount data to be fetched.
>END
[Qin Wu]:Fixed.
---
>5.
>s/the need of live server/the need of a live server/ s/main consumer of/main
>consumers of/
[Qin Wu]:Fixed.
---
>3. has "MetricType tags"
>5.1 has "Metric type tag"
>9.2 has "IETF Metric Type Tags"
[Qin Wu] Will make them consistent, thanks.
---
>5.1
>s/data objects classification/data object classifications/ s/related to metric
>subobject/related to a metric subobject/ s/. 'aggregated' multi-source/. The
>'aggregated' multi-source/ s/being combined/to be combined/ s/.
>'non-aggregated'/. The 'non-aggregated'/
>s/interface) be/interface) to be/
>s/correlate data object/correlate data objects/ s/defined in Section 9/defined
>in Section 9.2/
[Qin Wu]:Fixed.
---
>5.2
>s/or vendor specific tags/or vendor tags/
[Qin Wu]:Fixed.
---
>7.
>Given that the YANG module is extractable and standalone, you should probably
>expand "opm" somewhere near the top.
[Qin Wu] OK.
---
>7. opm-tag
>OLD
> Opm Tag is used to
> classify operation and management data object into object,
> property, and metric three categories.
>NEW
> 'opm-tag' is used to
> classify operation and management data objects into the
> three categories, object, property, and metric.
>END
>s/list. Data Object/list. The Data Object/ s/node. Data Object/node. The Data
>Object/ s/belonging to specific/belonging to a specific/ s/contain One single
>object tag or/contain one single object tag, or/ s/tag (These/tag (these/ s/As
>such the/As such, the/
[Qin Wu] Fixed.
---
>7. multi-source-tag
>s/identify multi-source/identify a multi-source/ s/'aggregated'
>multi-source/The 'aggregated' multi-source/ s/being combined/to be combined/
>s/and report as/and reported as/ s/'non-aggregated' multi-source/The
>'non-aggregated' multi-source/
>s/interface) be/interface) to be/
[Qin Wu] Fixed.
---
>8.1
>There needs to be a mention of Figure 3 in the text.
>s/defined below, see Section Section 9.2/defined in Section 9.2/
[Qin Wu] Fixed.
---
>Appendix C
>The figure should have a figure number and caption, and should be referred to
>by the figure number from the text.
[Qin Wu]OK
---
>Appendix D
>I think you want to include a note to the RFC Editor to remove this appendix
>before publication.
[Qin Wu] OK
_______________________________________________
netmod mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/netmod
