Re: [Netconf] I-D Action: draft-wu-netconf-restconf-factory-restore-02.txt

[Qin Wu <[e-mail]>]

Thanks Juergen, please see my comments inline.

-Qin
-----邮件原件-----
发件人: Juergen Schoenwaelder [mailto:[e-mail]] 
发送时间: 2018年9月30日 21:04
收件人: Qin Wu <[e-mail]>
抄送: Netconf <[e-mail]>
主题: Re: [Netconf] I-D Action: draft-wu-netconf-restconf-factory-restore-02.txt

On Sun, Sep 30, 2018 at 09:35:26AM +0000, Qin Wu wrote:
> v-(02) has been posted to addressed comments discussion on the list.
> The diff is:
> https://www.ietf.org/rfcdiff?url2=draft-wu-netconf-restconf-factory-re
> store-02
> Your review and input are welcome.
>

   This document introduces a new datastore resource named 'Factory
   default setting' that represents datastore with its preconfigured
   initial state.  This datastore resource is available using the
   following resource path:

This is backwards terminology wise. You introduce a new datastore and then that datastore automatically maps to a RESTCONF resource, not the other way round. You should not mix datastore definition and protocol specifics like the RESTCONF resource path.

[Qin]: Agree, will fix this.
I am also not sure what exactly the factory-default datastore contains. It sounds like simple static data but in reality many devices do some probing of hardware to determine what the "factory default" is, i.e., factory-default is not necessarily static.

[Qin]: Good point, regarding what content is included in factory default datastore, I think the documentation defining the datastore should answer this question, but I am not sure this should be in the scope of this draft.
Yes, I agree with you factory default datstore is not necessarily a static datastore and will update the draft to reflect this.

Why do we need a capability identifier?

   i.e.,a new <factory> XML element is added to the input for
   the <copy- config> <get-config>,<get-data> operations.

I think this is wrong and not needed for get-data. And if NMDA is a prerequisite for this, then we do not need to talk about get-config.

[Qin]: For get-data operation, I agree with you but we need to add a new datastore identity which is derived from identity defined in ietf-datstores module of RFC8342.
For get-config, the choice-based strategy from get-config operation doesn't allow future extension. That's why we proposed to add a new <factory> XML element which is identity type.
If mixing use of identity to identify factory with choice based strategy from get-config is not a good design,i.e.,
"
augment "/nc:copy-config/nc:input/nc:source/nc:config-source" {
       description
         "Add factory default Datastore as source.";
       leaf factory {
         type ds:datastore-ref;
         must "derived-from-or-self(current(), 'fd:factory')" {
          error-message "config source is only applicable to factory.";
        }
"
I think we should define a new operation for reverting content of any datastore into factory default setting. What do you think of this?

   For <copy-config> operation, the server MUST reset the target
   datastore to factory default setting according to the value of this
   element and return <ok> element in the NETCONF <rpc-reply> messages
   in success case.

If you include a RESTCONF example, make sure it is correct.

[Qin]: Okay.
Appendix A of RFC 8342 provides guidelines how to define new datastores. Should they not be followed?

The text says factory-default but the YANG module and the RESTCONF example define / use <factory>.

[Qin]: Will make them consistent,thanks.

I do not understand the reset operation (nor I am convinced yet that copy-config is the right thing necessarily):

    rpc reset {
      nacm:default-deny-all;
      description
        "Request to reset the device to factory default setting
         and make configuration update to take effect.
         A server SHOULD send an rpc reply to the client before
         reset the device.";
    }

What is 'make configuration update to take effect.' How are the various datastores modified if I invoke 'reset'? How do copy-config and reset relate to each other? Why is reset nacm:default-deny-all while copy-config etc. are not?

[Qin]: I think nacm:default-deny-all should apply to config-config if you think extending copy-config is a right thing.
In the current draft, the key difference between copy-config and reset, is reset operation can revert all datastores in the device to factory-default-setting while copy-config can only revert one target datastore to factory default setting.
 
/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <https://www.jacobs-university.de/>