Re: [netmod] LL review of draft-ietf-netconf-rfc7895bis-04

[Ladislav Lhotka <[e-mail]>]

On Mon, 2018-02-12 at 13:02 +0100, Martin Bjorklund wrote:
> Hi,
> 
> Thank you for this review.  Comments inline.
> 
> Ladislav Lhotka <[e-mail]> wrote:
> > Hi,
> > 
> > here is my review of the rfc7895bis document:
> > 
> > *** General comments
> >     - This revision is a significant improvement necessary for
> >       supporting NMDA. However, it is also flexible enough in that it
> >       allows for implementing the NMDA rules in an effective way but
> >       doesn't preclude the use of other datastores and datastore
> >       architectures.
> > 
> >     - Another benefit is that the new YANG library schema can be
> >       easily integrated with schema mount information.
> > 
> > *** Specific comments
> > 
> > **** Sec. 1 - backward compatibility
> > 
> >      Given that the old YANG library schema is carried over as a
> >      deprecated subtree, how can a server implementor actually cater
> >      for backward compatibility of e.g. RESTCONF clients supporting
> >      only RFC 8040?  Could the same YANG modules that comprise NMDA
> >      datastore schemas be advertized also via the old YANG library? Or
> >      is it necessary to also have pre-NMDA versions of all modules?
> > 
> >      Some more explanation might be useful here.
> 
> The text says:
> 
>   The recommended approach to populate /modules-state is to
>   report the schema for YANG modules that are configurable via
>   conventional datastores and for which config false data nodes are
>   returned via a NETCONF <get> operation, or equivalent.
> 
> Do you think that more guidance is needed?
> My opinion is that a server that wasnt to be backwards compatible
> probably advertise exactly what it used to advertise (in
> /modules-state), even if new NMDA-compatible models are added and
> advertised in /yang-library for new clients.
> 
> In general it is of course not possible to advertise everything that
> is availabile in /yang-library also in /modules-state - if this was
> possible we wouldn't have done YLbis.

Yes, I guess the question here is what "backward-compatible" means (given that
it is a SHOULD). One interpretation could be that it is sufficient to make sure
that, e.g., all resources required by RFC 8040 are present. So a lazy
implementor could just use an (almost) empty "modules-state" list. Does this
qualify as backward-compatible?

> 
> I don't think this document should provide recommendations on whether
> to implement pre-NMDA versions or not; this will be up to each server
> implementor to decide.

So it means that (in RESTCONF) /modules-state is bound to the legacy resource

{+restconf}/data

and /yang-library to the new datastore resources as defined in draft-ietf-
netconf-nmda-restconf, and these two are basically unrelated?

> > **** Sec. 1 - YANG library stability
> > 
> >      The text basically says that the YANG library information can
> >      change at any time. This has been recently discussed but I
> >      haven't seen any conclusion yet. I understand it is difficult to
> >      enumerate all the situations when this information can change,
> >      but it should also be emphasized that YL info is not just another
> >      subtree of state data and that it should not change haphazardly.
> 
> I agree, but I think that YANG library's job is to report what the
> server implements.  If the server dynamically changes its set of
> loaded modules, then YL should adapt.
> 
> I welcome more discussion on this topic, but I don't think it has to
> be documented in this draft.

What about this?

OLD
   The YANG library information can be different on every server and it
   can change at runtime or across a server reboot.  If a server
   implements multiple network management protocols to access the
   server's datastores, then each such protocol may have its own
   conceptual instantiation of the YANG library.

NEW
   The YANG library information represents a management API for a given server, 
   and should therefore be as stable as possible. The circumstances under which 
   this information can change are outside the scope of this document but it is 
   advisable to consider potential impact on clients.
    
> 
> >      It is like with database schemas, REST APIs and the like. Of
> >      course, these can change as well, but everybody has to understand
> >      that doing so means transition problems, broken clients etc.
> > 
> >      For this reason, it might be useful to set YL and schema mount
> >      data aside and call them metadata or schema information - even if
> >      we continue modelling them with YANG.
> 
> Do you have some concrete proposal for where to introduce this term?

In RESTCONF it could be a separate well-known resource outside all datastores.

> 
> > **** Sec. 3 - semantic versioning
> > 
> >      Some placeholders for future inclusion of semantic versions in
> >      YANG library information might be useful. To this end, I would
> >      suggest to introduce a new choice node and make "revision" (or,
> >      even better, "revision-date") its only case. This way, other
> >      versioning schemes such as semver could be easily added via
> >      augmentation.
> 
> When revision is used as a list key, we can't have a choice.

So it means that revision dates statements will have to be retained in the
modules and properly updated.

> 
> And when it is not a key, it is an optional leaf, so adding a 'semver'
> optional leaf in the future is also ok.
> 
> The proposal I have seen adds semver as an addition to revision, so
> using a choice would not be correct in this case.

OK

> 
> 
> > **** Sec. 4 - checksum
> > 
> >      I think it would be very useful (even if not immediately) to
> >      standardize the procedure for computing the checksum. What I
> >      envision are systems that construct and process YANG schemas
> >      (such as the YANG Catalog). They could benefit from having a
> >      universal hash string as a characteristic of any particular
> >      schema. Just consider how useful the universal hashes are e.g. in
> >      git.
> 
> Ok.  It would be interesting to see such a scheme.  But I agree it is
> not needed immediately for this document.

Checksums are mandatory, so every implementation has to invent some scheme.

Actually, it might be useful to have checksums also on module-sets, schemas and
datastores so that the client can easily localize the changes and retrieve again
only necessary data.

> 
> > **** Nits
> > 
> >      - sec 1. paragraph 2: s/informaton/information/
> 
> Fixed.

I just came across another:

    - sec. 1 paragraph 3: s/compatability/compatibility/

Lada

> 
> 
> /martin
> 
> 
> > 
> > Lada
> > 
> > -- 
> > Ladislav Lhotka
> > Head, CZ.NIC Labs
> > PGP Key ID: 0xB8F92B08A9F76C67
> > 
> > _______________________________________________
> > netmod mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/netmod
> > 
> >
-- 
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67