Re: [RTG-DIR] rtgdir Last Call Review requested: draft-ietf-netmod-factory-default

[Qin Wu <[e-mail]>]

Thanks Antoni for valuable review, see reply inline.
-----邮件原件-----
发件人: Antoni Przygienda [mailto:[e-mail]] 
发送时间: 2020年3月14日 6:30
收件人: [e-mail]; [e-mail]; [e-mail]
主题: Re: rtgdir Last Call Review requested: draft-ietf-netmod-factory-default

I have been selected to do a routing directorate “early” review of this draft.
​https://datatracker.ietf.org/doc/draft-ietf-netmod-factory-default/

Document: draft-ietf-netmod-factory-default
Reviewer: Tony Przygienda
Review Date: 03/15/20
Intended Status: Standards Track    

Summary:

I have some minor concerns about this document that I think should be resolved before it is submitted to the IESG.

Comments:

Good, clear, concise draft. Clear references, easy read. 

Following items need to be explained, treated IMO:

* it is not clear how "default configuration" defined in [RFC8342] interacts/is correlated with the "factory-default" store. Can "factory-default" overwrite the default configuration? Probably not since "it's in the data model" (RFC8342 is quite sparse on the subject). Generally I think it would be helpful to the reader to point out in a sentence that "default store" and "factory-default store" are utterly un-correlated since the language is suggesting they are closely related which in fact they are not.  
[Qin]:Correct, they are completely unrelated, there is no default store such thing.
as you clarified, default configuration is in the model instead of in any datastore.
default configuration is handled by the server based on RFC6243.

There is a subtle but big danger here with making it "read-only" and people misunderstanding this.
[Qin] "read-only " means the content in it can not be changed by management operations via NETCONF, RESTCONF, the CLI etc.

 If the device has been shipped with image X containing the store and then upgraded to image Y the factory-default-store for X may brick the device. It would be possibly a good thing to add somehow to the draft this consideration as in "when upgrading software version or Yang model of the server/device/node factory-default store in normally upgraded with it". 
[Qin]:How upgrading software version or Yang model affects other configuration datastore,e.g., running, operational.
Do you think factory default datastore handling should be different from other datastores handling.
In addition, the content of the datastore is set by the server in an implementation dependent manner.
not sure we should add implementation dependent restriction for factory default datastore after software upgrading or yang model upgrading.

* the draft says 

   o  Origin: This document does not define a new origin identity as it
      does not interact with the <operational> datastore.

This seems somewhat doubtful to me. Even if no new origin is defined here (maybe it should be) the draft should define what origins needs to be set or whether old origin should be preserved (I doubt that since .e.g. factory reset can overwrite default origin on current indented/operational)
[Qin]In some case, there is no factory default datastore, we only have rpc command factory-reset.
Secondly, "factory-reset" RPC will only directly affect read-write configuration datastore. Therefore I think it should not be
visible to intended or operational datastore.
In addition, there is no datastore for default origin or default store you described above.
* is the device clock reset/pertained or is the operation undefined as to what it does to internal device clock. This is often a very important operational consideration. 
[Qin]: We can not enumerate all other resetting tasks, but we do have already added statement to consider other resetting tasks in the draft. In addition, I am not sure device clock should be really affected by factory-reset operation.
* what happens to hardware installed keys on e.g. MACSEC? Are they reset/pertain/expected to be on factory-default and installed (vendors do different things here AFAIK), is that outside scope of this draft? 
[Qin]: Yes, it is beyond scope of this document, if I remember correctly, this was discussed in netconf ML, which is related to another draft.
* serialization, can other operations be performed @ the same time as factory reset? If so, which values do persist and when can the reset be executed? Will existing sessions be shut-down/warned/time-out'ed? If I missed the implications due to previous RFCs, which RFC governs the behavior here?
[Qin] This is something related to rfc8572, but based on discussion in the ML, it was suggested to remove this reference since there may be other implementation choice.
* Can the factory-default RPC lead to device reboot? Is that undefined/undesired (I think either is fine for the draft to say)? This is often an important operational consideration. The model says " after
          being reset, the device may become unreachable on the
          network". Does that pertain to this "reset" operation here or a reboot?
[Qin] Not directly, but "reset" operation may trigger device reboot only when the device is not reachable in the network.
 IMO following terms should be added & defined/distinguished in the glossary "reset" and "reboot". What is "on the network"? management interfaces/uplinks/inband ports. Either treat that more explicitly as in "after factory-default the device SHOULD be reachable via management ports and may not be reachable via uplinks/inband ports" or drop the "network".
[Qin]:I think it is close to the former, good suggestion, thanks.
 There is a big difference between "bricking it", "needs physical removal or hook up to short-range e.g. USB/WLAN" and "can only be reached out-of-band" ... Using vague descriptions like "on the network" is probably worse than not saying anything at all.