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

From: Qin Wu <[email-masked]>
Date: Tue, 2 Oct 2018 08:52:40 +0000

Thanks Juergen, please see my comments inline.

-Qin
-----邮件原件-----
发件人: Juergen Schoenwaelder [mailto:[email-masked]] 
发送时间: 2018年9月30日 21:04
收件人: Qin Wu <[email-masked]>
抄送: Netconf <[email-masked]>
主题: 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: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 2 Oct 2018 11:35:41 +0200

> Yes, I agree with you factory default datstore is not necessarily a static datastore and will update the draft to reflect this.

I thought this draft defines this new datastore and hence the draft
should define what it contains and how the content relates to other
datastores.
 
> [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.

datastore identity != capability identifier. I agree that you need a
datastore identify, I do not think you need a capability identifier.

>           error-message "config source is only applicable to factory.";

But why would one do this? Use get-data instead of get-config.

>    element and return <ok> element in the NETCONF <rpc-reply> messages

Whether this is meaningful or not I can't tell without having a more
precise definition of what <factory> contains. The phrase 'reset the
target datastore to factory default' is a bit unclear. Is this meant
to be an exact copy of <factory> to the target or is there more to
this notion of 'reset'?

> 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 t…

I am still unsure whether 'reset' is the same as copy of <factory> to
all (configuration?) datastores or whether 'reset' means different
things to different datastores and what does this operation mean when
more datastores are added in the future or how do I determine the set
of datastores affected by such a 'reset'.

Concerning nacm:default-deny-all, I think we should be consistent. It
is kind of strange if an operation is nacm:default-deny-all but you
can achieve the same effect using a sequence of other operations that
are not nacm:default-deny-all.

/js

---

From: Qin Wu <[email-masked]>
Date: Mon, 8 Oct 2018 09:36:08 +0000

Juergen:
-----邮件原件-----
发件人: Juergen Schoenwaelder [mailto:[email-masked]] 
发送时间: 2018年10月2日 17:36
收件人: Qin Wu
抄送: Netconf
主题: Re: [Netconf] I-D Action: draft-wu-netconf-restconf-factory-restore-02.txt

On Tue, Oct 02, 2018 at 08:52:40AM +0000, Qin Wu wrote:
> 发件人: Juergen Schoenwaelder 
> [mailto:[email-masked]]
> 发送时间: 2018年9月30日 21:04
> 主题: Re: [Netconf] I-D Action: 
> draft-wu-netconf-restconf-factory-restore-02.txt
> 

I thought this draft defines this new datastore and hence the draft should define what it contains and how the content relates to other datastores.

[Qin]: Okay, point taken. I think the factory datastore should include minimal set of modules that can make the netconf session being established between network device and the management system, which include:
NACM model
IP Model
Interface Model
System management model
Common data types model
[email-masked]
[email-masked]
[email-masked]
[email-masked]
It serves as basic device profile. Factory datastore may also include vendor specific models or additional standard models.
The content of factory is related to all configuration datastore, if the content of factory need to be related to operational datastore as well, a separate factory datastore is needed, in my opinion.
> [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.

datastore identity != capability identifier. I agree that you need a datastore identify, I do not think you need a capability identifier.

[Qin]: Yes, I understand datastore != capability identifier, my impression is if you define a new operation for copying content of factory to any datastore, capability identifier is not needed.
However if you modify the existing operation such as copy-config and allow it accept <factory> parameter, then you need to define capability identifier.
RFC6241 D1 appendix provides an guideline for this:
"
   The capabilities template is intended to be used in
   cases where the YANG mechanisms are not powerful enough (e.g., for
   handling parameterized features) or a different data modeling
   language is used.

"

> If mixing use of identity to identify factory with choice based 
> strategy from get-config is not a good design,i.e., "

But why would one do this? Use get-data instead of get-config.
[Qin]: I am talking about copy-config, not get-config, do you suggest to define copy-datastore new operation? If the answer is yes, I will agree the capability identifier is not needed.

>    element and return <ok> element in the NETCONF <rpc-reply> messages

Whether this is meaningful or not I can't tell without having a more precise definition of what <factory> contains. The phrase 'reset the target datastore to factory default' is a bit unclear. Is this meant to be an exact copy of <factory> to the target or is there more to this notion of 'reset'?

[Qin]: See above.

> 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 t…

I am still unsure whether 'reset' is the same as copy of <factory> to all (configuration?) datastores or whether 'reset' means different things to different datastores and what does this operation mean when more datastores are added in the future or how do I determine the set of datastores affected by such a 'reset'.

[Qin]: I think it is the former, but reset might also mean it is client forced behavior. Just like when the client detect the operating system down, you can push reset button.
When more configuration datastore are introduced, I think reset could also be applied to them.

Concerning nacm:default-deny-all, I think we should be consistent. It is kind of strange if an operation is nacm:default-deny-all but you can achieve the same effect using a sequence of other operations that are not nacm:default-deny-all.

[Qin]:I agree.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 8 Oct 2018 11:51:15 +0200

On Mon, Oct 08, 2018 at 09:36:08AM +0000, Qin Wu wrote:
> 
> The content of factory is related to all configuration datastore, if
> the content of factory need to be related to operational datastore
> as well, a separate factory datastore is needed, in my opinion.

This is too vague and does not help me. You need to explain what the
data flow is between other datastores.

> I am talking about copy-config, not get-config, do you suggest to
> define copy-datastore new operation? If the answer is yes, I will
> agree the capability identifier is not needed.

If we need a better copy-config, we should create a better copy-config
instead of introducing multiple ways to refer to datastores. I know
that early versions of draft-ietf-netconf-nmda-netconf-06 mentioned
copy-config, I need to dig deeper why this was taken out at some point
in time.

/js

---

From: Balazs Lengyel <[email-masked]>
Date: Mon, 8 Oct 2018 17:54:50 +0200

Hello,

1) The function needs to support devices that have a factory
reset functionality, but which are not capable to show the actual
default values as a full datastore. Not all YANG servers will be
full featured, big machines.

2) We have a number of writable datastores: running, startup,
candidate, dynamic-datastores.  We need to document what happens
with each. After reseting a datastore in the normal case

the running datastore is reset to mirror the content of the
factory-default-datastore

candidate datastore is reset to the same state as if the
<discard-changes> operation was executed

startup datastore is reset to the same state as if
<copy-config>  running->startup was executed

Any dynamic datastores are reset to empty

The system MAY provide YANG-Instance-Data based or other
implementation specific documentation to specify other content
that a datastore will have after a reset operation.

I wrote an example YANG module that could describe the above:

----------------------------
module ietf-factory-reset {
namespace urn:ietf:params:xml:ns:yang:ietf-factory-reset ;
prefix fres ;
yang-version 1.1;

import ietf-netconf { prefix nc ; }
import ietf-datastores { prefix ds; }

feature factory-default-as-datastore {
description "Indicates that the factory default configuration is
also available as a separate datastore";
}

rpc reset-datastore {
description "The target datastore is reset to it's factory
default content. The server MUST allow reset for the running
datastore and MAY allow reset for any other writable datastore.";

input {
choice target-datastore {
leaf target {
type identityref { base "ds:datastore" ; }
}
leaf all-datastores {
type empty;
description "All writable datastores including running,
startup, candidate and any dynamic datastores that exist
are reset to their default value.";
}
}

// Do we need an extra parameter that may order a restart of
// the YANG-server or the whole system?
}
}

identity factory-default-running-datastore {
description "The read-only datastore contains the configuration that
will be copied into the running datastore by the reset-datastore
operation if the target is the running datastore.";
if-feature "factory-default-as-datastore";
}

augment /nc:copy-config/nc:input/nc:target/nc:config-target {
description " Allows the copy-config operation to use the
factory-default-running-datastore as a source";
if-feature factory-default-as-datastore;
leaf factory-default { type empty ;}
}
}

regards Balazs

On 2018. 10. 08. 11:51, Juergen
Schoenwaelder wrote:

/js

--
Balazs Lengyel Ericsson Hungary Ltd.
Senior Specialist
Mobile: +36-70-330-7909 email: [email-masked]

---

From: Qin Wu <[email-masked]>
Date: Tue, 9 Oct 2018 06:28:34 +0000

-----邮件原件-----
发件人: Juergen Schoenwaelder [mailto:[email-masked]] 
发送时间: 2018年10月8日 17:51
收件人: Qin Wu
抄送: Netconf
主题: Re: [Netconf] I-D Action: draft-wu-netconf-restconf-factory-restore-02.txt

On Mon, Oct 08, 2018 at 09:36:08AM +0000, Qin Wu wrote:
> the content of factory need to be related to operational datastore as 
> well, a separate factory datastore is needed, in my opinion.

This is too vague and does not help me. You need to explain what the data flow is between other datastores.

[Qin]: We think factory may affect at least <running> and <intended>, it may also affect <startup>, it should help revert <running><intended> to preconfigured initial state.
It can coexist with <startup> when the device get booted.

> agree the capability identifier is not needed.

If we need a better copy-config, we should create a better copy-config instead of introducing multiple ways to refer to datastores. I know that early versions of draft-ietf-netconf-nmda-netconf-06 mentioned copy-config, I need to dig deeper why this was taken out at some point in time.

[Qin]Actually draft-ietf-netconf-nmda-netconf-06 did augment lock and unlock operation with additional 'datastore' parameter. copy-config operatio can also be augmented some cases. Anyway I think it will be better also to have a separate RPC for datastore level operation. It may look like clone operation.
/js

---

From: Qin Wu <[email-masked]>
Date: Tue, 9 Oct 2018 06:28:46 +0000

Good input, Balazs, see my comments inline.

发件人: Balazs Lengyel [mailto:[email-masked]]
发送时间: 2018年10月8日 23:55
收件人: Qin Wu; Netconf
主题: Re: [Netconf] I-D Action: draft-wu-netconf-restconf-factory-restore-02.txt

Hello,

1)  The function needs to support devices that have a factory reset functionality, but which are not capable to show the actual default values as a full datastore. Not all YANG servers will be full featured, big machines.

[Qin]: This can be resolved by introducing a new factory datastore, with existing operation, we can be able to know what contents are included in such datastore.

2) We have a number of writable datastores: running, startup, candidate, dynamic-datastores.  We need to document what happens with each. After reseting a datastore in the normal case

  1.  the running datastore is reset to mirror the content of the factory-default-datastore

[Qin]: you missed intended, can intended also be reset to mirror the content of the factory-default-datastore?

  1.  candidate datastore is reset to the same state as if the <discard-changes> operation was executed
  2.  startup datastore is reset to the same state as if <copy-config>  running->startup was executed
  3.  Any dynamic datastores are reset to empty

The system MAY provide YANG-Instance-Data based or other implementation specific documentation to specify other content that a datastore will have after a reset operation.

[Qin]: yeah, additional new supported module can be specified by using YANG-Instance-data documentation.

    prefix fres ;

    yang-version 1.1;

    }

    rpc reset-datastore {

        input {

                leaf target {

                }

                leaf all-datastores {

                    type empty;

                }

            }

        }

    }

[Qin]: Sounds good proposal,in my earlier version of draft, I defines an extra parameter to indicate whether restart is needed.

           If such parameter is needed, we can define it as empty type.

    }

}

      [Qin]: So in this case, we don’t need to define capability identifier. Instead, we are using feature to indicate whether a new functionality is needed, right?

}

regards Balazs
On 2018. 10. 08. 11:51, Juergen Schoenwaelder wrote:

in time.

/js

--

Senior Specialist

Mobile: +36-70-330-7909              email: [email-masked]<mailto:[email-masked]>