Subject: [Netconf] Genart last call review of draft-ietf-netconf-nmda-restconf-04

From: Russ Housley <[email-masked]>
Date: Thu, 28 Jun 2018 12:47:56 -0700

Reviewer: Russ Housley
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-netconf-nmda-restconf-04
Reviewer: Russ Housley
Review Date: 2018-06-28
IETF LC End Date: 2018-07-09
IESG Telechat date: unknown

Summary: Ready

Major Concerns:

None.

Minor Concerns:

The last paragraph of Section 3.1 says:

   If a server implements the example datastore "ds-ephemeral" in the
   module "example-ds-ephemeral", it would implement the resource
   {+restconf}/ds/example-ds-ephemeral:ds-ephemeral.

It is unclear to me why this datastore is not included in the bullets
at the beginning of the section.  Obviously, it is optional to
implement, but so are two of the datastores that are included in
the list.

The last bullet of Section 3.2 says that [RFC8040], Section 3.5.4,
paragraph 3 does not apply when interacting with any resource under
{+restconf}/ds.  The referenced paragraph says:

   If the target of a GET method is a data node that represents a leaf
   or leaf-list that has a default value and the leaf or leaf-list has
   not been instantiated yet, the server MUST return the default value
   or values that are in use by the server.  In this case, the server
   MUST ignore its "basic-mode", described in Section 4.8.9, and return
   the default value.

I suspect that this paragraph does not apply because the leaf and
leaf-list will always be instantiated.  A sentence to say one way or
the other would be useful to the implementer.

Nits:

There is a missing ')' in the last paragraph of Section 5.

---

From: Robert Wilton <[email-masked]>
Date: Fri, 29 Jun 2018 12:01:30 +0100

Hi Russ,

Thanks for the review, please see inline ...

> implement, but so are two of the datastores that are included in
So the difference is that:
  "operational", "running" and "intended" are all defined by RFC 8342, 
and we expect these datastores to be actually implemented.

"ds-ephermeral" is just an example of a new configuration datastore that 
hasn't yet been defined anywhere yet.  It could for example, be defined 
by something like I2RS in future to handle dynamic configuration, or 
perhaps a vendor would define their own dynamic datastore.

> the other would be useful to the implementer.
The aim here is to remove the above CLR that is in RFC 8040.

I.e. for NMDA datastores (including <running>), don't treat the GET 
method for leaf/leaf-list any differently than the GET method on any 
other data node, and respect both the clients request of whether default 
values should be returned, and also the servers properties (as to 
whether it always returns defaults).

As you say, this particularly helps for the operational state datastore, 
which handle default values in a different way. But it also helps with 
the configuration datastores, since it allows a client to easily 
determine whether a particular leaf has actually been configured just by 
querying for it.  With the rule as stated in RFC 8040 you cannot so 
easily find out, because you would have to make the query on the parent 
data node instead, and check whether the child data node in question has 
been included in the response.

Finally, we felt that it is easier to fix this in NMDA in a backwards 
compatible way that only applies to the new datastore specific paths, 
rather than making a breaking change to the behavior of the /data path 
described in RFC 8040 that could cause inconsistencies between existing 
implementations.

> There is a missing ')' in the last paragraph of Section 5.
Thanks, I'll fix this.

Thanks,
Rob

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

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 29 Jun 2018 14:12:43 +0200

On Fri, Jun 29, 2018 at 12:01:30PM +0100, Robert Wilton wrote:
> "operational", "running" and "intended" are all defined by RFC 8342, and we
> expect these datastores to be actually implemented.
> 
> hasn't yet been defined anywhere yet. It could for example, be defined by
> something like I2RS in future to handle dynamic configuration, or perhaps a
> vendor would define their own dynamic datastore.
>

Perhaps we can simply remove this entire paragraph - it is not
strictly needed and I can see that this paragraph talking about a
hypothetical future datastore may be misunderstood.

/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: Russ Housley <[email-masked]>
Date: Fri, 29 Jun 2018 11:36:01 -0400

> On Jun 29, 2018, at 7:01 AM, Robert Wilton <[email-masked]> wrote:
> 
> Hi Russ,
> 
>  "operational", "running" and "intended" are all defined by RFC 8342, and we expect these datastores to be actually implemented.
> 
> "ds-ephermeral" is just an example of a new configuration datastore that hasn't yet been defined anywhere yet.  It could for example, be defined by something like I2RS in future to handle dynamic configuration, or perhaps a vendor would define their own dynamic datastore.

Perhaps something like this:

   If a server implements other datastores, such as the example
   datastore "ds-ephemeral" in the module "example-ds-ephemeral",
   the server would implement the resource {+restconf}/ds/example-
   ds-ephemeral:ds-ephemeral.

> I.e. for NMDA datastores (including <running>), don't treat the GET method for leaf/leaf-list any differently than the GET method on any other data node, and respect both the clients request of whether default values should be returned, and also the servers properties (as to whether it always returns defaults).

Saying the above in the would help me.

> As you say, this particularly helps for the operational state datastore, which handle default values in a different way. But it also helps with the configuration datastores, since it allows a client to easily determine whether a particular leaf has actually been configured just by querying for it.  With the rule as stated in RFC 8040 you cannot so easily find out, because you would have to make the query on the parent data node instead, and check whether the child data node in question has been included in the response.
> 
> Finally, we felt that it is easier to fix this in NMDA in a backwards compatible way that only applies to the new datastore specific paths, rather than making a breaking change to the behavior of the /data path described in RFC 8040 that could cause inconsistencies between existing implementations.

Backwards compatibility is a good thing.

Russ