Hi Kent, thanks for the thorough review, see my responses inline.
Kent Watsen <[email protected]> writes: > Hi, > > I have read this document and think that is almost ready for > publication. I have five discuss items and a bunch of nits. > > Kent // contributor > > > 1. From Section 4: > > Routing configuration inside an NI often needs to refer to interfaces (at > least those that are assigned to the NI), which is impossible unless such > a reference can point to a node in the parent schema (interface name). > > This seems overstated. Rather it is a result of an earlier design decision. > An alternate solution might have exported the global interfaces into a > config false list inside the mount jail. Was such a solution > discussed? Do you mean something like having "symlinks" to interfaces inside the mount jail? Yes, this was discussed and rejected. According to Martin, tail-f had made a very bad experience with this approach. > > > 2. Also from Section 4: > > For every schema mounted using the "use-schema" method, it is possible > to specify a leaf-list named "parent-reference" that contains zero or more > XPath 1.0 expressions. Each expression is evaluated with the node in the > parent data tree where the mount point is defined as the context node. > > If you can nested-mounts, can you also have nested parent-references? Yes, but you can only refer to the level directly above. > > > 3. Also from Section 4 (same paragraph): > > For the purposes of > evaluating XPath expressions within the mounted data tree, the union > of all such nodesets is added to the accessible data tree. > > Could this ever result in name collision? Potentially yes, but sibling nodes with the same name are not an issue for XPath evaluation. > > > 4. Regarding Security Considerations, what about > /yangmnt:schema-mounts? You are right, the security considerations should be similar to those in RFC 7895. It is the same type of risk. > Also, should how NACM interacts with mounted instance data be > specified? This is a good question because, in principle, top-level NACM rules can address instances of mounted schemas. On the other hand, ietf-netconf-acm can also be a part of the mounted schema - and if so, can its rules also address instances in the parent schema via the parent-reference mechanism? But I would say it is up to rfc6536bis to deal with these questions. > > > 5. This document does not say anything about how it relates to NMDA. > Clearly all this is targeted to the conventional datastores, but how > is it reflected in e.g., <operational>? Does anything need to be said > here? The "use-schema" case shouldn't pose big problems because it is essentially an externally specified augment. The "inline" case is somewhat disturbing though: could the embedded YANG library instances be different in different datastores? > What if the mounted schema has deviations in <operational>. I don't understand why this could be an issue. > > > Nits (line-break separated): > > Is "other optional choices" being vague on purpose? Should it just > call out features and deviations? Yes, it should. > > "the YANG library data" seems odd. Maybe "the instance of the YANG > Library module"? It is the same but I prefer the former. Note that the term "instance" is not defined in RFC 7950. > > - document, and could be possibly dealt with in a future revision of the YANG > data modeling language > + document, as it needs to be dealt with as an update to the YANG data > modeling language > I am not sure that it *needs* to be done, because it could have far-reaching consequences. > - Schema mount applies to the data model > + Schema mount regards the data model OK > > - This document allows mounting of complete data models only. > + This document allows mounting of complete modules only. Correct > > - may extend this model by defining > + may extend this solution by defining Or perhaps "approach"? I would leave "solution" to marketing folks. > > In S3, replace "YANG 1.1" with "YANG 1.1 and its continuances"? Not sure. For example, next version of YANG can/should turn "mount-point" into a built-in statement. > > - A "container" or "list" node > + A 'container' or 'list' node Actually, following RFC 7950 conventions, no quotes should be used here. > > - of "container" and "list" statements. > + of the "container" and "list" statements. OK > > - Mounted schemas for all mount points > + The schema for all mount points OK > > - in the "yangmnt:schema-mounts" container. > + in the top-level "yangmnt:schema-mounts" container defined in the > "ietf-yang-schema-mount" module defined in [Section 8]. Section 2.2 defines the relationship between the "yangmnt" prefix and that module. > > The "refers through its key" part is not clear - are you talking > about the mount-point's argument/label? Yes. Would it be more clear to say this? Every entry of this list refers through its key to a mount point label and specifies the schema for all mount points with that label. > > I don't understand "above those that are defined in the parent > schema." - mostly the word "above" is throwing me… Yes, "apart from" is better. > > - If multiple mount points with the same name > + If multiple mount points with the same label (wasn't it called a > "label" before?) Right > > Regarding "Note, that in this case a mount point", beyond the missing > comma, an example would be very helpful. I don't know if I understand > it right. You don't, because that sentence is completely wrong - it is yet another example of mixing up the schema and instance. The idea is that if (for some strange reason) the mounted schema includes the ietf-yang-library module, then all *instances* of this (mounted) YANG library must be identical and have the same content as the "use-schema" data. This is another reason why I would prefer to have only one YANG library at the top - it is not regular state data. > > In the YANG itself, "State data nodes" didn't parse well, "Protocol > accessible nodes" instead? Do you mean the comment? The same or similar comment is in many existing modules - for example, ietf-yang-library has "Operational state data nodes". It is true though that it doesn't make much sense here as long as there are no configuration data. > > Regarding the first paragraph in Appendix A, I took me some time to realize > that the rtgwg-device-model included > ietf-network-instance and that those modules define mount-points and > where. Please make this easier for first-time readers. OK, we should try. > > In A1, is ietf-network-instance missing? - might want to double-check > all No, because A1 describes only the top (device) level, ietf-network-instance is a part of the LNE schema in A.2. > > In all the examples, but beginning with A2, it might help to show the > RESTSCONF protocol operation that illustrates the result, so that it's > clear where in the data model the particular instance is located. > Anything that can be done to provide more context would be helpful. It seems to make sense in A.3 - to demonstrate an URI of a resource inside an NI. > > For the 2nd half of A2, what happens if there is an "lne-2", will it > also get "eth0"? I think "ietf-logical-network-element:bind-lne-name" leaf should contain the name of a LNE to to which the interface "eth0" is assigned, so it looks like a mistake. > > - which should include at least > + which should include at least an instance of > ietf-yang-library:modules-state and ietf-interfaces:interfaces-state, > as follows: But the important point is that only interfaces assigned to the LNE need to be included, so it is not just "an instance of ietf-interfaces:interfaces-state". Why do you think the existing text isn't OK? Lada > > > Thanks again, > Kent > > > _______________________________________________ > netmod mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/netmod -- Ladislav Lhotka Head, CZ.NIC Labs PGP Key ID: 0xB8F92B08A9F76C67 _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
