Hi Jan,
> On Aug 15, 2024, at 9:25 AM, Jan Lindblad (jlindbla)
> <[email protected]> wrote:
>
> Quifang, WG,
>
> I have read the latest (-08) version of the draft carefully. I think it is
> well written, but I do have a few comments/questions.
>
> #1) Removal of system config list entries
>
> Section 5.1 states:
>
> To ensure the validity of <intended>, configuration in <running> is
> merged with <system> to become <intended>, in which process,
> configuration appearing in <running> takes precedence over the same
> node in <system>.
>
> This is very good of course. If the client wishes for a different leaf value
> than one in the system datastore, just write it in running (assuming the
> server allows it). But what if the client wishes that some part (e.g. list
> entry) of the system config is left out from running?
Do you mean the last word above to be “intended”?
> With the approach described above, it does not seem obvious that anything
> from system would be left out from intended, even if it is not mentioned in
> running.
My reading is that nothing in <system> is left out when <intended> is created,
sans:
- any “inactive” nodes
- any unreferenced "shared objects”
- any templates that are expanded (if templates supported in <system>)
- other things like this.
Back to the point, clients do not get to delete <system> configure.
> Reading further in section 5.2:
>
> When declaring a node having descendants, clients MUST also declare all
> descendant nodes, including any leafs, leaf-lists, lists, presence
> containers, non-presence containers that have any child nodes.
>
> Does this imply the client could indeed prevent selected parts of the system
> datastore configuration to not appear in intended by simply not including
> them in running?
I don’t think deletion is intended. Rather the goal is to ensure a full-copy
is in <running>, hence the “MUST”. IIRC, this approach is a direct response
to a previous comment of yours...
> Here’s a concrete example sketch intended to clarify the question.
>
> The following YANG is loaded on a firewall server:
>
> container site-groups {
> list site-group {
> key name;
> leaf name { type string; }
> list site {
> key name;
> leaf name { type string; }
> leaf max-rate { type uint32; }
> }
> }
> }
>
> container aces {
> list ace {
> …
> leaf allow-access-to-site-group {
> type leafref {
> path /site-groups/site-group/name;
> }
> }
> }
> }
>
> The server is running with the following instance data in “list site-groups”
> in the system datastore:
>
> site-groups {
> site-group “social” {
> name “youtube” { max-rate 400 }
> name “facebook” { max-rate 100 }
> name “twitter” { max-rate 100 }
> }
> site-group “news” {
> name “wall-street-journal” { max-rate 200 }
> }
> }
>
> The client configures the following instance data in “list site-groups” in
> the running datastore:
> site-groups {
> site-group “social” {
> name “youtube” { max-rate 600 }
> name “facebook” { max-rate 25 }
> }
> }
>
> Finally the client configures the server’s access control in the running
> datastore:
>
> aces {
> ace {
> …
> allow-access-to-site-group “social”
> }
> }
>
> The question now: will the server allow traffic to twitter? Since it’s not
> present in running, I’d expect traffic is blocked. But then the first
> paragraph in section 5.1 sounds a bit odd:
>
> Clients MAY reference nodes defined in <system>, override system-
> provided values, and configure descendant nodes of system-defined
> configuration in <running>, as detailed in Section 5.2, Section 5.3,
> and Section 5.4.
>
> In this case, wouldn’t it be fair to say clients cannot reference nodes in
> system, but only nodes that have been copied to running one way or another?
> Wouldn’t the topic of this draft then be more about mechanisms for copying
> data from system to running than about how system is merged into intended?
It is sad how the short-term “running alone must be valid” position is causing
such issues. The original goal was (is still?) to let vendors define some
system-config, some of which makes sense for clients to reference (for
convenience). We then added overrides and descendants, and seemingly lost
sight of the goal. I question if the current approach is best. Maybe a “take
it as it is defined in <system> or define your own in <running> approach” would
be better?
> #2) System + Running datastore merge process
>
> Section 5.1 also states:
>
> If <running> includes configuration that requires
> further transformation (e.g., template expansion, removal of inactive
> configuration defined in [RFC8342]) before it can be applied,
> configuration transformations MUST be performed before <running> is
> merged with <system>.
>
> This seems to imply that the system datastore cannot contain any kind of
> templating mechanism, e.g. some sort of interface template in systems with
> lots of interfaces. I don’t have any particular opinion, just raising the
> question if this is what we want.
My reading is that the text doesn’t imply that, so much as, if <system> uses a
templating mechanism, that its templates must also be expanded before the merge
can occur. We (presumably) define how to merge configuration, but how to merge
templates is not defined (templates aren’t defined yet but, if they were,
merging them might not be defined). If we agree, it would be good for the
document to be clear about it.
> #3) Undefined term “declare”
>
> Wording in section 5.2 has been carefully crafted to use a term “declare”. I
> can see great care has been taken to introduce this term in the text, but I
> am not quite sure of the exact meaning, and I don’t think it has been defined
> in this or any referenced document.
>
> It is possible for a client to explicitly declare system
> configuration nodes
>
> The explicit declaration of system-defined nodes
>
> Clients MUST declare the
> system configuration that
>
> When declaring a node having descendants, clients MUST also declare
> all descendant nodes
>
> After reading this multiple times, I think “declare” could be replaced by
> “configure”. If not, I would be interested to understand why this word has
> been chosen. I can tell it was done with great care and deliberation, but I
> don’t understand why. If there is a good reason for this choice, maybe the
> term should be properly defined?
Agree that “configure” is the better word.
> #4) Will validation make changes?
>
> Section 7.2 states that
>
> The
> client may use the "resolve-system" parameter in one of the following
> situations:
> …
> * The client issues a <validate> operation.
>
> If a client issues a validate with the resolve-system flag set, will that
> modify the target datastore?
>
> If so, let’s say a client removes a reference to a system datastore object
> and runs validate with resolve-system again. Would the system datastore
> objects copied earlier be removed this time?
>
> Regardless of which way it is, it appears rather complicated to me to
> implement the logic in section 7.2 as worded right now. Maybe only allowing
> resolve-system on edits and not on validate and commit would reduce the
> complications?
I do not think validation should be allowed to make changes. Stated
differently, the <validate> RPC (in NETCONF) should be (is already defined to
be?) idempotent.
If “resolve-system” expansion in <running> is needed (and I’m unsure if it ever
is, on a server), then I envision that expansion to be in an internal/ephemeral
"running” that is discarded immediately after the validation completes.
> #5) Conflict resolution
>
> Section 7.2 also speaks of conflict resolution.
>
> In particular, [I-D.ietf-netconf-privcand] defines the concept of
> conflict, the server's copy referenced system nodes triggered by
> "resolve-system" parameter might conflict with the contents of
> <running>, the conflict resolution is no different than the
> resolution of conflict caused by configuration explicitly provided by
> the client.
>
> I’m afraid I don’t understand what this paragraph is saying. Could it be
> rephrased? If the conflict concept is important, the reference should
> probably be normative.
I also don’t know what this paragraph is trying to say.
Kent // as a contributor
> #6) Example interface names
>
> Nit: It seems that the examples in appendix A.2 and A.4 are mixing references
> to et-0/0/0 and eth-0/0/0.
>
>
> Thank you for your efforts with this draft!
>
> Best Regards,
> /jan
>
>
> From: Kent Watsen <[email protected]>
> Date: Sunday, 11 August 2024 at 16:11
> To: [email protected] <[email protected]>
> Subject: [netmod] 2nd WGLC on system-config-08
>
> NETMOD WG,
>
> We did a WGLC in May on the -05 version of this document. The diffs since
> then are substantial
> (https://author-tools.ietf.org/iddiff?url1=draft-ietf-netmod-system-config-05&url2=draft-ietf-netmod-system-config-08&difftype=--html)
> and so it seems prudent to run a fresh WGLC on this document now.
>
> This email begins a two-week WGLC on:
>
> System-defined Configuration
> https://datatracker.ietf.org/doc/html/draft-ietf-netmod-system-config-08
>
> Please take time to review this draft and post comments by August 25. Whilst
> favorable comments are welcomed, given that this document already has a lot
> of support, the primary focus now is to determine if there are any objections
> or concerns. If no objections or concerns are raised, this document will
> automatically progress to the next step.
>
> From the IPR poll in March, there is no known IPR for this document:
> https://mailarchive.ietf.org/arch/msg/netmod/IpzWIAbgifXoKaNfLhEDmYbyXkY/
>
> Kent // co-chair
>
> _______________________________________________
> netmod mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
_______________________________________________
netmod mailing list -- [email protected]
To unsubscribe send an email to [email protected]
