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

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 09 Feb 2018 14:52:39 +0100

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.

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

     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.

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

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

**** Nits

     - sec 1. paragraph 2: s/informaton/information/

Lada

-- 
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 12 Feb 2018 13:02:12 +0100 (CET)

Hi,

Thank you for this review.  Comments inline.

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

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.

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

>      we continue modelling them with YANG.

Do you have some concrete proposal for where to introduce this term?

>      versioning schemes such as semver could be easily added via

When revision is used as a list key, we can't have a choice.

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.

>      schema. Just consider how useful the universal hashes are e.g. in

Ok.  It would be interesting to see such a scheme.  But I agree it is
not needed immediately for this document.

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

Fixed.

/martin

> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod
> 
>

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 12 Feb 2018 15:26:31 +0100

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

> to implement pre-NMDA versions or not; this will be up to each server

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?

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

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

> using a choice would not be correct in this case.

OK

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

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

I just came across another:

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

Lada

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 12 Feb 2018 15:37:49 +0100

>    advisable to consider potential impact on clients.

I like the old text because it tells the client clearly that this data
can change. And the statement has been in RFC 7895 in the exact same
wording. If you want to add a statement that servers should not change
the YANG library without reason I could live with that but any attempt
to write text that makes the server somewhat guilty when a client is
not prepared to handle a YANG library change is IMHO a fundamental
change from what RFC 7895 said.

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

Putting the data into a different place does not change the impact of
the data changing. So I do not understand which problem introducing
yet another datastore solves.

> datastores so that the client can easily localize the changes and retrieve again

With RESTCONF, you can use etags and conditional requests. NETCONF
lacks a similar generic mechanism to support caching. Instead of
adding checksum everywhere into our data models, it seems a better
solution would be to add something like etags to NETCONF. Hence, we
reduced this to a single checksum which is needed as it is carried in
the hello message.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 12 Feb 2018 16:14:29 +0100

> can change. And the statement has been in RFC 7895 in the exact same

My problem with the current text is that it seems to make no difference between
YANG library and any other state data.

> to write text that makes the server somewhat guilty when a client is

Not guilty but careful. There is no requirement that clients check YANG library
between every two operations, and notifications are optional.

> yet another datastore solves.

Nothing except emphasizing the difference between data and metadata, which is
IMO an important one.

> reduced this to a single checksum which is needed as it is carried in

Etags work, but my point here is to have the checksum as a globally unique
identifier of a given data model, schema or module set. For example, it would
allow for checking that multiple servers use the same data model.

Lada

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 12 Feb 2018 18:29:36 +0100

> YANG library and any other state data.

The sentence starts with 'The YANG library information' and what
follows is all scoped to 'YANG library information'.
 
> between every two operations, and notifications are optional.

So let me try to make an alternate proposal. (I only added the second
sentence.)

NEW:

   The YANG library information can be different on every server and
   it can change at runtime or across a server reboot. Servers may
   schedule YANG library changes in way that minimizes the impact on
   active clients. 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.
 
> Nothing except emphasizing the difference between data and metadata, which is

So its a different topic - one that we closed before I thought.
 
> allow for checking that multiple servers use the same data model.

I was commenting on your proposal to have multiple checksums.

Concering your other proposal, namely to specify a detailed algorithm
how to calculate these checksums, I have reservations as well but for
other reasons. First, RFC 7895 does not specify this. Second, for the
usage in the NC hello exchange, it is not necessary that there is a
common way to calculate the checksum. Third, the current definition in
RFC 7895 (which has not been changed by the update) allows efficient
implementations since the number is essentially a version number.
Fourth, I have not seen a proposal for a robust algorithm that easily
produces the exact same checksum across a number of equivalent
configurations (the root problem is that the notion YANG library
equivalence is nowhere really defined - you can't simply serialize
YANG library data and checksum the result since there are only limited
serialization ordering requirements).

/js

---

From: Andy Bierman <[email-masked]>
Date: Mon, 12 Feb 2018 11:39:50 -0800

> change from what RFC 7895 said.
I strongly oppose changing this text.
Any server that can load or unload modules at run-time can change
the YANG library at run-time.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Mon, 12 Feb 2018 11:44:41 -0800

On Mon, Feb 12, 2018 at 9:29 AM, Juergen Schoenwaelder <
[email-masked]> wrote:

> it
> > > >    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
> > > > 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.
> > >
I agree that the YANG library should not mandate a checksum algorithm.
I do not even like calling this field checksum (or having multiple fields).

Andy