Re: [netmod] review of draft-ietf-netmod-schema-mount-08

[Kent Watsen <[e-mail]>]

> Hi Kent,
>
> thanks for the thorough review, see my responses inline.

Hi Lada, please see below.


>> 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.

Yes, a little bit like a symlink inside the mount jail.  Good to hear
that it was discussed.  I still think the above "impossible" word is
overstating things.  Perhaps s/impossible such/possible when/?


>> 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.

Okay.



>> 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. 

I don't know if I agree that it can't be an issue.  What if the module
has a top-level /widgets container, and there is a parent-reference to
a /widgets container, and there is an Xpath to /widgets - seems like
you could have both false-positives and false-negatives...




>> 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.

ok


>> 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.

I don't know, but it seems that this draft is introducing the concept.
Not to mention, rfc6536bis is already with the IESG.  In either case,
let's work out what it means and then decide which draft it needs to
go into.



>> 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?

YANG Library is only available in <operational> (it's all config false).
I think you mean the embedded YANG Library instances under the mount 
points.  Yes, you may have a problem here.  Still, this isn't my question,
is more about schema-mounting requirements across datastores.  E.g., if
a schema-mount exists in <running>, must it existing in <intended> and
<operational> as well?  Conversely, if a schema-mount exists in
<operational>, must it exist in <running>?  I think this draft should
clarify such things.



>> What if the mounted schema has deviations in <operational>.
>
> I don't understand why this could be an issue.

I'm not saying it's a problem yet.  I'm just stating that such things
can occur and it's unclear that, if they do, could it cause problems.
For instance, a mounted schema may be looking for a parent reference
that doesn't exist because of a deviation.




>> 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.

okay


>> - 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.

Here I'm using "needs" not to mean that it "has to be done" but more that,
if it is done, it would have to be done in an update to the YANG data 
modeling language.  The current sentence seems too open-ended...


>> - 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.

"approach" is fine.


>> 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.

So then, when we define yang-next, we'd be forced to either incorporate
schema-mount or simultaneously publish an update to this RFC to allow
it to work with the new version of YANG?  Even if YANG-next defined a
built-in equilvalent, would we not want this mechanism to continue to
be supported, for backwards compatibility?


>> - A "container" or "list" node
>> + A 'container' or 'list' node
>
> Actually, following RFC 7950 conventions, no quotes should be used here.
>

I'm confused about these conventions - are they written down somewhere.
In Martin's review of zerotouch draft, he dinged me on my mis-use of
single-quotes, for the second time.  Looking at RFC 7950, I don't see
the convention listed anywhere, or are you saying that the convention
is throughout the RFC and folks are expected to implicitly understand
what it is?



>> - 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.

Yes, but this is the first time in the RFC that /schema-mounts is
mentioned.  The prefix is there, but why not guide the user a little
more?


>>  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.

Yes!


>> 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.

agreed.


>> - 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.

okay


> This is another reason why I would prefer to have only one YANG library
> at the top - it is not regular state data.

Does this need to be discussed some more?


> 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.


Yes, the comment.  Well, this is a nit, so take it for what it's worth,
but it did seem a bit unusual when I read it.


>> 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.

thanks.



>> 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.

okay


>> 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.

great, anything that helps.


>> 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.

glad to hear there might be 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?

What you have isn't wrong, per se, I'm just trying to make the text
more readable.  How about:

  which should include at least an instance of
  ietf-yang-library:modules-state and ietf-interfaces:interfaces-state,
  containing only the entries relevant to the mount-point, as follows:

- or use your own words?



> Lada

Kent