Hi Qiufang, Please see responses inline with [mj.
> On Nov 27, 2025, at 8:36 PM, maqiufang (A) <[email protected]> wrote: > > Hi, Mahesh, > > Thanks for the review, and sorry for the delay. New revisions have been > posted to try to resolve your comments below, the diff against -12 is > available at > https://author-tools.ietf.org/diff?doc_1=draft-ietf-netmod-system-config-12&doc_2=draft-ietf-netmod-system-config-14 > > <https://author-tools.ietf.org/diff?doc_1=draft-ietf-netmod-system-config-12&doc_2=draft-ietf-netmod-system-config-14>. > Please also find my response below inline… > > From: Mahesh Jethanandani [mailto:[email protected]] > Sent: Wednesday, August 27, 2025 12:13 PM > To: [email protected] > Cc: NetMod WG <[email protected]> > Subject: Re: AD review of draft-ietf-netmod-system-config-12 > > Some more follow-up comments: > > Section 5.2, paragraph 0 > > This work has no impact to <operational>. Notably, it does not > > define any new origin identity as it is able to use the existing > > "system" identity defined in Section 5.3.4 of [RFC8342]. <system> > > enables system-generated nodes to be defined like configuration, > > i.e., made visible to clients in order for being referenced or > > configurable prior to present in <operational>. "config false" nodes > > are out of scope, hence existing "config false" nodes are not > > impacted by this work. > > I am not sure if there is "no impact to <operational". I would say "This > document does not change <operational> definition; it clarifies origin > reporting, i.e., items sourced from <system> report origin 'system' unless > overridden by node in <running>". > I am okay with this. Fixed with slight rewording: "This work does not change > the definition of <operational>; it clarifies origin reporting, i.e., the > origin of nodes sourced from <system> is reported as "system" unless > explicitly configured or overridden in <running>." I’ve also updated the > title to “No Changes to <operational>”. [mj] Ok. > > Section 6.3, paragraph 0 > > In some cases, a server may allow some parts of system configuration > > (e.g., a leaf's value) to be modified. Modification of system > > configuration is achieved by the client writing configuration data in > > <running> that overrides the values of matched configuration nodes at > > the corresponding level in <system>. Configurations defined in > > <running> take precedence over system configuration nodes in <system> > > if the server allows the nodes to be modified (some implementations > > may have immutable system configuration as per > > [I-D.ietf-netmod-immutable-flag]). > > What happens if <running> config is updated to remove the node that the > matched configuration node in the corresponding level in <system>? Does the > <system> value instantly show up? > Are you referring to some copied system nodes being removed from <running> > will be added back automatically in <running>? > No, if nodes copied from <system> into <running> are removed afterwards from > <running>, it disappears from <running>; but the client can add it back > manually, of course. The definition of <running> ds indicates that <running> > is controlled by the client, not the server. > However, those nodes would still be in <system> unless license or system > resources change (i.e., card removed). [mj] No, I was referring to the case where the client has previously configured a node in <running> but then decides to remove it. A corresponding node in <system> does exist. The document says that <running> overrides the matched config node in <system>. But it is not clear what happens when the <running> node is removed/unconfigured. > > Also, if the server implements an immutable flag, should the nodes under > <system> that are not overridable be annotated "immutable", enabling clients > to pre-validate edits? > Yes, immutable flag is visible in <system>, but we prefer to leave details to > I-D. ietf-netmod-immutable-flag, and keep it as informational here and just > allows the existence of modifiable and non-modifiable system configuration. [mj] Ok. But one of the documents needs to make it clear. > Section 6.4, paragraph 0 > > A server may also allow a client to add nodes to a list entry in > > <system> by writing those additional nodes in <running>. Those > > additional data nodes may not exist in <system> (i.e., an addition > > rather than an override). > > What happens if the list entry is removed because a license gets revoked or a > line card gets pulled? > Then the list entry is removed from <system> but only be present in > <running>, and thus is in <intended> (which is a merge result of <running> > and <system>). Since license or physical resource is not there, the entry > does not appear in <operational>, this is clarified in 8342 > (seehttps://datatracker.ietf.org/doc/html/rfc8342#appendix-C.3.1 > <https://datatracker.ietf.org/doc/html/rfc8342#appendix-C.3.1>), and this > draft doesn’t change that behavior. [mj] If the entries continue to be present in <running>, will that not create a problem if the client tries to modify the config for those nodes? > > Section 8.2, paragraph 2 > > This document registers two XML namespace URNs in the 'IETF XML > > registry', following the format defined in [RFC3688]. > > > > URI: urn:ietf:params:xml:ns:yang:ietf-system-datastore > > Registrant Contact: The IESG. > > XML: N/A, the requested URIs are XML namespaces. > > > > 8.2. The "YANG Module Names" Registry > > > > This document registers two module names in the 'YANG Module Names' > > registry, defined in [RFC6020]. > > > > name: ietf-system-datastore > > prefix: sysds > > namespace: urn:ietf:params:xml:ns:yang:ietf-system-datatstore > > maintained by IANA? N > > RFC: XXXX // RFC Ed.: replace XXXX and remove this comment > > The document says two modules, but registers only one. > Good catch! fixed. > > > On Aug 26, 2025, at 7:44 PM, Mahesh Jethanandani <[email protected] > <mailto:[email protected]>> wrote: > > Hi Authors, > > Thanks for working on the document. Also, thanks to Michal Vasko for > providing the YANG doctors’ review of the document. > > I found the document well-written and easy to understand. The examples were > especially useful. Here are some comments/clarifications on the document. > > Section 1.3, paragraph 2 > > This document also updates the definition of "intended" origin > > metadata annotation identity defined in Section 5.3.4 of [RFC8342]. > > The "intended" identity of origin value defined in [RFC8342] > > represents the origin of configuration provided by <intended>, this > > document updates its definition as the origin source of configuration > > explicitly provided by clients, and allows a subset of configuration > > in <intended> that flows from <system> yet is not configured or > > overridden explicitly in <running> to use "system" as its origin > > value. As per Section 5.3.4 of [RFC8342], all configuration with > > origin value being reported as "intended" MUST originate from > > <running>, which includes any configuration in <system> that has been > > copied into <running>. Configuration that is in <system> and not > > also present in <running> MUST be reported as origin "system" in > > <operational>. > > It was my understanding that configuration in <system> is NOT copied into > <running> but that it is merged with <running> before being written into > <intended>. Is my understanding wrong? Figure 1 also does not indicate any > copying of <system> into <running>. > I think your understanding is correct. System configuration is not present in > <running> by default, but merged into <intended> which is the configuration > that the device tries to apply. The client may explicitly copy some nodes > from <system> into <running>, e.g., if the node needs to be > overridden/referenced. [mj] In that case, what would be the origin of those nodes? > Section 4, paragraph 7 > > Configuration in <system> is undeletable to clients (e.g., a system- > > defined list entry can never be removed), even though a node defined > > in <system> may be overridden in <running>. If it is desired to > > enable a client to delete system configuration, it can be > > approximated using <factory-default> ([RFC8808]). If system > > initializes a value for a particular leaf which is overridden by the > > client with a different value in <running> (Section 6.3), the node in > > <running> may be removed later, in which case system-initialized > > value defined in <system> may still be in use and appear in > > <operational>. > > There is a seeming contradiction here, or the behavior is not very well > explained. Earlier in the section, the document 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>.” > > However, in this paragraph, it says that values in <running> may be removed > later, in which case system-defined values may be in use. What gives? Are > there special conditions under which the opposite is true? > Both sentences are true to me, I guess the seeming contradiction is because > they are different merge results that appear in <intended> but do not think > it wise to say too much about the merge logic (i.e., how two different data > trees are merged) in this draft. > If the desired value is in <running>, it wins out over the predefined value > in <system>; otherwise (e.g., never be added or be added but removed > afterwards) there is no conflicts when merging, system predefined value is > just the final configuration that is in use. > Any suggestion to reword it to make it more clear? [mj] This goes back to my earlier question of what happens when the client removes a node in the <running> config whose value is different from the value in <system>. My thought is that once the node is removed in <running> that the <system> configured value will show up in <intended>/<operational> datastore. If that is correct, then the above sentence could be reworded to say just that: OLD: If system initializes a value for a particular leaf which is overridden by the client with a different value in <running> (Section 6.3), the node in <running> may be removed later, in which case system-initialized value defined in <system> may still be in use and appear in <operational>. NEW: If system initializes a value for a particular leaf and that leaf was previously overridden by the client with a different value in <running> (Section 6.3), and if the node in <running> is now removed, the system-initialized value defined in <system> comes into use and appears in <operational>. > > > "A.1.", paragraph 10 > > The client may also define its customized applications. Suppose the > > configuration of applications is present in <running> as follows: > > To keep the same tune as the <system> contents, it is better to say: > > OLD: > The client may also define its customized applications. Suppose the > configuration of applications is present in <running> as follows: > NEW: > The client may also define its customized applications. Those > applications may be present in <running> as follows: > Sure, fixed. > "A.1.", paragraph 15 > > Please add some explanation for the contents of the <operational> datastore. > Something like, "since there were no conflicts in the content between > <system> and <running>, all the applications were merged as-is". > There were some, but I tried to add more details. How about the following: > OLD: As different entries of application configuration in <system> and > <running> is merged to create <intended>, <operational> might contain the > configuration of applications as follows:… > NEW: As different entries of application configuration in <system> and > <running> are merged to create <intended> and there are no merging conflicts > in the contents between <system> and <running>, <operational> might contain > the configuration of applications with the values of origin reflecting the > source of entries as follows:… [mj] Better. > "A.2.", paragraph 6 > > <interfaces xmlns="urn:example:interface"> > > <interface> > > <name>lo0</name> > > <mtu>65536</mtu> > > <ip-address>127.0.0.1</ip-address> > > <ip-address>::1</ip-address> > > </interface> > > </interfaces> > > A client modifies the value of MTU to 9216 and adds the following > > configuration into <running> using a "merge" operation: > > > > <interfaces xmlns="urn:example:interface"> > > <interface> > > <name>lo0</name> > > <mtu>9216</mtu> > > </interface> > > </interfaces> > > The merge of <system> and <running> and the net result of that in <intended>, > if shown here, would be helpful before showing the result in <operational>. > It will also demonstrate why the origin is <intended> in <operational>. > I don’t really think it needed to show the contents of <intended> here as it > should be identical to <operational> but without the origin values (origin > metadata annotation is only valid for <operational> ds). But I think it might > be helpful to add at the beginning something like "Note this section does not > show the contents of <intended> as they are related to the configuration in > <operational> assuming the intended configuration is applied successfully. ". > Would this be okay? [mj] Ok. Please add. > "A.2.", paragraph 6 > > <interfaces xmlns="urn:example:interface" > > xmlns:or="urn:ietf:params:xml:ns:yang:ietf-origin" > > or:origin="or:intended"> > > <interface> > > <name>lo0</name> > > <mtu>9216</mtu> > > <ip-address or:origin="or:system">127.0.0.1</ip-address> > > <ip-address or:origin="or:system">::1</ip-address> > > </interface> > > </interfaces> > > Can there be an explanation added here to explain why the <interfaces> origin > is "intended", but that of the <ip-address> is "system”? > > Similar to A.1, some explanatory text about how the mtu value from <running> > overrode the mtu value in <system> would be helpful. > I’ve added some, let me know if this is sufficient: "Since the MTU value > provided by the client takes precedence over the system-provided value, and > the "origin" value of configuration provided by the client is set to > "intended", the configuration of interfaces that is present in <operational> > is as follows: …" [mj] Ok. > "A.3.", paragraph 1 > > In the above example, imagine the client further configures the > > description node of a "lo0" interface in <running> using a "merge" > > operation as follows: > > > > <interfaces xmlns="urn:example:interface"> > > <interface> > > <name>lo0</name> > > <description>loopback</description> > > </interface> > > </interfaces> > > Which "above example" are you referring to? The immediate example above is as > follows: > Sorry for confusion. It should be more explicit: Based on the example in A.2. [mj] Thanks for fixing it. > <interfaces xmlns="urn:example:interface"> > <interface> > <name>lo0</name> > <mtu>9216</mtu> > </interface> > </interfaces> > > which would make the config post merge as follows: > > <interfaces xmlns="urn:example:interface"> > <interface> > <name>lo0</name> > <mtu>9216</mtu> > <description>loopback</description> > </interface> > </interfaces> > Agree, this echoes what shows in <operational>. > "B.1.", paragraph 5 > > <interfaces xmlns="urn:example:interfacemgmt" > > xmlns:or="urn:ietf:params:xml:ns:yang:ietf-origin" > > or:origin="or:system"> > > <interface> > > <name>lo0</name> > > <type>loopback</type> > > <enabled or:origin="or:default">true</enabled> > > <ip-address>127.0.0.1</ip-address> > > <ip-address>::1</ip-address> > > <description>system-defined interface</description> > > </interface> > > </interfaces> > > Should the origin of the <ip-addresses> not show where that config is coming > from, as in the above examples? Or is that left out on purpose? If so, please > state that for brevity or clarity of the example, not all origins are > indicated. > The origin of <ip-address> is inherited from its parent node “interfaces”, > which is ‘system’. RFC 8342 defines that if it is not specified, it has the > same value as the origin value of its parent node. That way, I don’t think it > needed for this document to clarify the inheritance rules suppose the reader > is rather familiar with 8342, right? [mj] Yes, that is true, but there is no harm in being explicit. Thanks. > > ------------------------------------------------------------------------------- > NIT > ------------------------------------------------------------------------------- > > All comments below are about very minor potential issues that you may choose > to > address in some way - or ignore - as you see fit. Some were flagged by > automated tools (via https://github.com/larseggert/ietf-reviewtool > <https://github.com/larseggert/ietf-reviewtool>), so there > will likely be some false positives. There is no need to let me know what you > did with these suggestions. > > "A.2.", paragraph 6 > > Then the configuration of interfaces is present in <operational> as > > follows: > > s/interfaces is present in <operational> as/interfaces that is present in > <operational> is as/ > Fixed. > These URLs in the document did not return content: > > * https://datatracker.ietf.org/wg/netmod/WG > <https://datatracker.ietf.org/wg/netmod/WG> (I think WG is extra) > It has now been reformatted. > Section 7.2, paragraph 8 > > ontent. The YANG module only defines a identity that uses the "ds:convention > > ^ > Use "an" instead of "a" if the following word starts with a vowel sound, e.g. > "an article", "an hour". > Fixed. > "A.1.", paragraph 10 > > loopback interface (named "lo0") with a MTU value "65536", a default IPv4 > > add > > ^ > Use "an" instead of "a" if the following word starts with a vowel sound, e.g. > "an article", "an hour". > Fixed. > > Mahesh Jethanandani > [email protected] <mailto:[email protected]> > > Best Regards, > Qiufang //co-author > > > > > Mahesh Jethanandani [email protected]
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
