[netmod] Balazs Review of draft-ma-netmod-with-system-02

[Balázs Lengyel <[e-mail]>]

Hello,
I did a detailed review of the system draft. My comments questions are below.
Regards Balazs

===========================================================

General)
I think this work is important and valuable, but it needs quite a lot improvements.

The term system-configuration is used confusingly. Does
system-configuration reside in the <system> datastore only or can it
reside in the <running> datastore too? If system-configuration is copied by
the client (<get-data>+<edit-config>) into the <running> datastore is it
still system-configuration? It is set by the client this time not the system.

Some terminology is needed to indicate that you mean a specific data node
IN A SPECIFIC DATASTORE. The same data node (according to the path in the
data tree) in different datastores need to be referenced separately.

Does the solution allow conditional system configuration?
(E.g.,  if the client creates an OSPF interface the system inserts a child leaf into it)

1.1) If system shares the same schema as running that would force it to
populate mandatory nodes.  That might be a problem.
State that mandatory or min-elements might not be enforced in <system>.

1.3) "client may overwrite values of configurations defined in <system>"
However it also states: The contents of <system> datastore are read-only
These seem to contradict. Please clarify.

1.4) Shoudn't copy-config also be effected? Copy-config
might also need system configured items. It should be mentioned that the same
"resolution" is also needed after a node-restart.

What does populate mean? Is it the same as "copy from system to running" ? If
yes please use that terminology. Populate is not as specific.

2) In the subchapters (and later) you use the terms provided, activated, applied. I am not
sure what this means. Is a not yet applied item present in the <system>
datastore or only when it is applied? If I do a get-data on <system>
will I receive not-applied data nodes?

What is the difference between an applied and an activated data node and an
applied but not activated data node?

I would rather see terminology like:
- is present in the <system> datastore
- is not yet present in the <system> datastore, but the system will create it
in the <system> datastore when a condition is fulfilled.

How is it defined for specific schema nodes which kind of system-data it is ?
Free English text?
Is it needed to define this formally or is it enough if the server knows this?

2.2) Isn't the best example for this, when the functionality is licensed and
the license key is inserted?

3.1) <factory-default> is also read-only so why is that better to store
deletable data ? Did you mean that system-config originated data cannot be
delete even if it is copied over to running? Is that true both for explicit
NBI originated copy and copy due to resolve-system?

3.2) If something was populated/copied over to running/candidate will/should
any changed system values be copied over again thereby updating the
running/candidate datastores?
Can this result in the running becoming invalid?

4.1)  You write
"The client may reference nodes defined in <system>, overwrite values
   of configurations defined in <system>"
IMHO the data nodes in <running> and <system> are 2 different things even
if they reside on the same path in the data tree. You need to find
terminology to differentiate between the same(-path) data nodes in different
datastores. The current terminology is confusing, I need to guess which
datastore you mean. I think this guessing process might hide problems.
Do you mean here: "The client may reference nodes defined in <system> if they are
copied into <running>/<candidate> as a result of an explicit copy or
resolve-system parameter." For me referencing a data node in running and
referencing a data node in <system> (even if they share the same address in
the data tree) are 2 separate things. I don't think you want to create a
reference that point between datastores.
Do you mean here: "overwrite values of the data nodes that were created by
copying from the <system> datastore."

"<running> MAY overwrite and/or extend <system>" this means that the
data nodes in system are modified although they are readOnly.
Is this what you mean? Clarify!

"Note that only <system> aware clients copy
   referenced system nodes from <system>"
How does the server know if the client is system-aware? It would be better to
state something like:
'In order for the system configuration to affect validation the client needs to
either use the resolve-system parameter or explicitly copy system configuration
into running'

Last para: The server has no way to know if the client is system aware. Once
the data nodes are copied into <running> there is no need to say more.

4.2
"If the "resolve-system" parameter is not given by the client, the server
   MUST NOT modify <running> in any way not specified by the client."
I very strongly OBJECT.
- It is a bad idea.
- This is a big NBC change to Netconf/YANG.
- Other SDOs (3GPP, O-RAN) depend on the capability to modify <running>. They
have data nodes where it is stated that list entries are not created by the client.
- This would need a revision 2 of YANG.
- It is also unenforceable. It would be possible to work around it.
The system instantiates an onboard client to do the changes AND the system
prohibits the change for other clients.
However this is just a more complicated way of stating that the system itself
modifies running; we gain nothing but make the world more complicated.

4.3
Paragraph-1 sentence 2 & 3 are trivial thus not needed. If you configure
something in running it becomes part of running independent of this draft.

Mention that the system itself  can also copy over parts or the complete
system configuration into running.

4.4
In some cases, a server may allow some parts of system configuration
   to be modified.  List keys in system configuration can't be changed
   by a client, but other descendant nodes in a list entry may be
   modifiable or non-modifiable.

 This contradicts the statement that the <system> datastore is readOnly.

"Client configuration statements in <running> take precedence over system
configuration nodes in <system>"

Instead of hiding this sentence in the middle of a subchapter, there should be
a separate chapter about merging running and system into intended, stating that running has precedence.
This a tier 1 important statement !
There could be some interesting corner cases.

Once the data is in running, AFAIK the knowledge about why is it there is lost,
so terms like "client configuration" are hard to understand. That sounds more
like a use-case than a rule.

"While modifying (overriding) system configuration nodes may be
   supported by a server, there is no mechanism for deleting a system
   configuration node."

Once the node is in the <running> datastore if it is not mandatory it is
possible to remove it. What prevents it? What if it was the client that copied
the configuration into <running>? Is the client forbidden to remve something
that it created itself? I don't think so.

5.
"datastore does not have to persist across reboots."
'I would say: The content of the datastore is removed at reboot and
re-created by the system with the same or changed content.'
IMHO it is important to state that there will be some reasonable content in the
<system> datastore even if it might have changed.

7.1
"Comment: How does a RESTCONF client know if the RESTCONF server
   implements the "resolve-system" parameter?"
Make it a capability in the hello message like with-defaults.

7.2
The placement of resolve-system is sometimes incorrect. It shall be inside the
<edit-config> element.