Re: [netmod] schema-mount pre09 branch

[Martin Bjorklund <[e-mail]>]

Hi Juergen,

Thanks for your review, comments inline.

Juergen Schoenwaelder <[e-mail]> wrote:
> On Wed, Jan 31, 2018 at 09:36:07PM +0000, Kent Watsen wrote:
> > 
> > The authors created a "pre09" branch on GitHub a few weeks back.  On this branch, they completed a full update of the draft.  While waiting for details on how to proceed with regards to a SM-bis, we thought it would be helpful to make this text available now so that the technical parts can be discussed.  With this in mind, can folks please have a quick look and post any technical comments they have?  
> >
> 
> I have reviewed draft-ietf-netmod-schema-mount-pre-09 that I fetched
> today from GitHub. Here are my comments:
> 
> * Perhaps the abstract can be improved. The single sentence that we
>   have is not even quite correct.
> 
>    This document defines a mechanism to combine YANG modules into the
>    schema defined in other YANG modules.
> 
>   According to YL bis, a datastore has a schema, which consists of a
>   set of YANG modules. The terminology used in RFC 7950 is 'schema
>   node' and 'schema tree'. Perhaps this sentence can be rewritten to
>   better explain the purpose of this document.

Maybe something along the lines of:

  This document defines a mechanism to extend a datastore schema with
  other schemas.

It is still a bit terse...

> * The following text is not consistent with what YLbis says:
> 
>    2.  Implementation-time: the mounted schema is defined by a server
>        implementor and is as stable as YANG library information, i.e.,
>        it may change after an upgrade of server software but not after
>        rebooting the server.  Also, a client can learn the entire schema
>        together with YANG library data.
> 
>   YLbis does allow YANG library to change. I think this text should
>   change as follows:
> 
>    2.  Implementation-time: the mounted schema is defined by a server
>        implementor and is as stable as YANG library information of the
>        server. A client can learn the entire schema by reading the
>        server's YANG library data.

Yes this is better.

>    I think 3. should also be changed. The phrase 'is part of the
>    mounted data model' may not be exactly true with NMDA - well
>    depending what the 'mounted data model' really means. Perhaps
>    something like this:
> 
>    3.  Run-time: the mounted schema is defined by instance data that
>        is associated with the mount point and reported outside the
>        server's YANG library information. If there are multiple
>        instances of the same mount point (e.g., in multiple entries of
>        a list), the mounted data model may be different for each
>        instance.

Ok.

> * Terminology - should this document import the terms client, server,
>   notification from the NMDA document instead of the NETCONF document?

Ok + "datastore schema".

>   I assume the terms are understood more broadly than just NETCONF
>   client and server.
> 
> * Definition of inline schema:
> 
>    o  inline schema: a mounted schema whose definition is provided as
>       part of the mounted data, using YANG library
>       [I-D.ietf-netconf-rfc7895bis].
> 
>    I am not sure 'as part of the mounted data, using YANG library' is
>    a good definition. Which YANG library? I think what this term means
>    is a mount point specific YANG library, not the master server's
>    YANG library.
> 
>    This is also why I was largely confused about the inline case; the
>    schema used by the mountpoint is not defined inline from the master
>    server's point of view, from which the document is written. From
>    the master server's point of view, the 'inline schema' is actually
>    the opposite, it is an 'external schema' since I have to pull the
>    schema information from an external source;

No this is not quite correct.  It is "inline" from the master's point
of view, in the sense of "inline in the data tree", as opposed to
"part of the (augmented) yang-library schema definition".

And it is *not* external.  The client still fetches this data from the
"master" server; and how the master server handles this is an
implementation detail (or possibly standardized in the future - *one*
use case is peer mount in which case the data is fetched from another
system, but again, this is just one example).


>    the schema is not
>    reported with the server's schema information - this is what I
>    would consider inline.
> 
>    It may be too late to change this term but at least for me 'inline'
>    has been very confusing since we describe everything from the
>    perspective of the master server.
> 
>   Later on, the phrase "use-schema" case is used as well. Perhaps this
>   should also be a defined term

Ok, this makes sense.

>   and a better term should be used. The
>   terms should describe the concepts and the YANG model should follow;
>   what we have is kind of backwards, there is a YANG model and then
>   model names are used a conceptual terms.
> 
> * YANG library stability
> 
>   I reported this already. The statement 'In particular, it SHOULD NOT
>   change during the same management session.' is not consistent with
>   what YL and YLbis say.

I agree.

> * YANG library has a drawing of the underlying conceptual model. It
>   may help to agree on how schema mount is extending the model. Here
>   is a drawing that I created for myself:
> 
>         +-----------+
>         | datastore |
>         +-----------+
>              |
>              | has a
>              V
>         +-----------+            +--------+                +------------+
>         | datastore |  union of  | module |  consists of   | modules +  |
>  .----->|  schema   |----------->|  set   |--------------->| submodules |
>  |      +-----------+            +--------+                +------------+
>  |           |
>  |           | has
>  |           v
>  |      +-----------+
>  '------|   mount   |------>  external YANG library
>    uses |   points  | uses	schema reference
>         +-----------+
> 
>   Note that this makes drawing makes my struggle with the term
>   'inline' even more obvious.

I like the drawing, but I don't see the problem with the terms.  Maybe
just b/c I'm used to them.  I think we can include this figure but
s/external/inline/

>   If you look at this figure, then inline
>   would mean the exact opposite of what 'inline' means today.
> 
> * Where is the schema defined in the external aka 'inline' case?
>   The current text says:
> 
>    In case 1, the mounted schema is determined at run time: every
>    instance of the mount point that exists in the parent tree MUST
>    contain a copy of YANG library data [I-D.ietf-netconf-rfc7895bis]
>    that defines the mounted schema exactly as for a top-level data
>    model.  A client is expected to retrieve this data from the instance
>    tree, possibly after creating the mount point.  Instances of the same
>    mount point MAY use different mounted schemas.
> 
>   I think this does not work for datastores except the operational
>   datastore. In fact, I think the model must be that if /foo is an
>   external 'inline' mountpoint, then the /foo instance in the
>   operational state datastore must include a YANG library
>   instantiation in which a client can find the schema definition for
>   /foo in any datastore where /foo is supported. In other words, if
>   /foo is config true and a client wants to know how the /foo schema
>   in <running>, then the client has to fetch the YANG library residing
>   under /foo in <operational> in order to obtain the schema
>   information for the <running> datastore. The text does not say any
>   of this.

I agree.  We can write:

  A client is expected to retrieve this data from the instance
  tree in <operational>,

and possibly add your example.

> * Where is the schema defined in the inline aka 'not inline' case?
>   The current text says:
> 
>    In case 2, the mounted schema is defined by the combination of all
>    "schema" entries referred to in the "use-schema" list.
> 
>   I think the YANG model refers to a single schema. I think this text
>   needs to be revised and aligned with the new data model.

Yes.  There is just one schema, and we should write "yanglib:schema"
to clarify that the "schema" node is defined in YL(bis).

>   This like
>   applies to other places in the document where "use-schema" is
>   discussed.
>   I do not understand the following statement:
> 
>    [...] Note, that in this case a mount point may include a mounted
>    YANG library module and the data contained in the mounted module MUST
>    exactly match the data contained in the "schema" entries associated
>    with the mount point.
> 
>   There is text about a "schema" list in 3.2 that I think is not
>   needed anymore.

Yes, will remove.

> * RPC operations and notifications
> 
>   "clients can invoke this operation by representing it as an action
>   defined for the corresponding mount point"
> 
>   The wording 'invoke ... by representing" seems wrong. I think this
>   should be "clients can invoke this operation as if it were defined
>   as an action for the corresponding mount point" or something like
>   this.

Ok.

>   Should there be additional text discussing how parameters of
>   operations and notification content is interpreted? I assume
>   everything is scoped to the mount point but perhaps this needs to be
>   stated explicitly.

I think this handling follows from "as if it was defined as an
action...".

> * Data model
> 
>   The model is augmenting /yanglib:yang-library/yanglib:schema. In
>   the case where I had two different schemas defined for say the
>   operational state datastore and conventional datastores, this
>   implies that I have to duplicate the schema-mounts specific
>   definitions even in the likely case where the schema mounts are
>   identical in both datastores.

Yes, but you can define a sinlge module-set and refer to it from both
schemas.

>   (For the extern 'inline' case they
>   actually have to be the same or at least the operational state
>   datastore schema-mounts have to be a superset of the conventional
>   datastore schema-mounts since otherwise I can't get the schema
>   information. So the question is whether schema-mount should be a
>   list directly under /yanglib:yang-library and then
>   /yanglib:yang-library/yanglib:schema would be augmented by a leaf
>   (or a leaf-list) referring to schema-mount entries.
> 
> * YANG definitions
> 
>   I think the description of the leaf 'inline' [yeah, this should be
>   'external'] needs to be updated to explain where in an NMDA system
>   the YANG library information can be found.

Ok, we should add that it is available in <operational>.

>   And yes, 'use-schema' is really what I call 'inline' - the schema
>   information is provided as part of the YANG library defining the
>   mount point.
> 
> * Security considerations
> 
>   I was expecting text that explains how NACM works on a server that
>   has schema mounts, discussing both cases, the inline and external
>   schema mount cases.

We had a discussion about this on the ML, I have to get back and
re-read it.


/martin


> 
> * Examples
> 
>   I have not reviewed the examples.
> 
> /js
> 
> -- 
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   [phone]         <https://www.jacobs-university.de/>
> 
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod
>