Subject: [netconf] WG Last Call: draft-ietf-netmod-factory-default-05

From: Kent Watsen <[email-masked]>
Date: Fri, 1 Nov 2019 14:57:09 +0000

This begins a two-week Working Group Last Call (WGLC) on draft-ietf-netmod-factory-default-05.  The WGLC ends on Nov 15 (two days before the NETCONF 106 session).  Please send your comments to the working group mailing list.

Positive comments, e.g., "I've reviewed this document and believe it is ready for publication", are welcome!  This is useful and important, even from authors.  Objections, concerns, and suggestions are also welcomed at this time.

Thank you,
NETCONF Chairs

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Fri, 1 Nov 2019 15:08:12 +0000

> On Nov 1, 2019, at 10:57, Kent Watsen <[email-masked]> wrote:
> 

In general, I think it’s ready to go.  Two nits:

* My name is misspelled in the acknowledgements

* In Section 2:

OLD:

2. by vendors using YANG Instance Data […] file format in vendor’s website or other places where off-line document is kept;

NEW:

2. by vendors using a file in YANG Instance Data […] format on the vendor’s website or in other places where similar off-line documents are kept;

Joe

---

From: Kent Watsen <[email-masked]>
Date: Fri, 1 Nov 2019 15:18:25 +0000

I have reviewed -05 and support it so long as the following comments are considered:

Kent // contributor

==== review ====

Section 1 is missing a NMDA-compliance statement, per
 https://tools.ietf.org/html/rfc8407#section-3.5 <https://tools.ietf.org/html/rfc8407#section-3.5>.

Section 2 says:

   Factory-default content SHALL be specified by one of the following
   means in descending order of precedence

   1.  For the <running>,<candidate> and <startup> datastores as the
       content of the <factory-default> datastore, if it exists;

The (1) sentence doesn't flow from the sentence before.   Maybe you
mean something like:

   1.  Network management protocol (e.g., NETCONF, RESTCONF)
        operations may be used to access the contents  of <factory-default>.

Section 2 says:

   For the server supporting zero touch bootstrapping mechanisms, the
   factory default configuration causes the bootstrapping process to
   execute,e.g.,the server might reset configuration to device's factory
   default configuration,for the version of operating system software it
   is running.

s/the server might reset /the server resets /

Section 2 says:
   In addition,the "factory-reset" RPC might also be used
   to trigger some other restoring and resetting tasks such as files
   cleanup, restarting the node or some of the software processes,
   setting some security data/passwords to the default value, removing
   logs, or removing any temporary data (from datastore or elsewhere),
   etc.

s/the "factory-reset" RPC might /the "factory-reset" RPC MAY / ???

Section 3 says:

   this document introduces a new datastore resource named
   'Factory-Default' ...

'Factory-Default' should not be capitalized.

Section 3 says:

    The contents of the datastore can be read using NETCONF, 
    RESTCONF <get-data> and <get-config> operations.

Which doesn't make sense.  Perhaps:

    The contents of the datastore can be read using NETCONF 
     <get-data> and <get-config> operations, and the RESTCONF
    protocol equivalents.

Section 3 says:

      The operation <factory-
      reset> can be used to copy the factory default content to a set of
      read-write configuration datastores and then the content of these
      datastores is propagated automatically to any other read only
      datastores, e.g., <intended> and <operational>.

This is confusing.  I think what you want to say is

      The operation <factory-
      reset> copies the factory default content to <running> and,
      if present, <startup>.

Section 4 says:

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

These statements are missing "reference" statements.

Section 4 says:

    description "The read-only datastore contains the configuration that
      will be copied into e.g., the running datastore by the
      factory-reset operation if the target is the running
      datastore.";

which excludes <startup> and confusingly mentions a "target" when
the RPC itself has no parameters.  Perhaps:

    description "The read-only datastore contains the configuration
    that  will be copied into <running> and, if present, <startup>.";

Section 5.

Please make the registrations have single-spaced lines.

Section 6.

The last paragraph doesn't make a point.  Perhaps conclude with
something like:

  "This module does not itself set "nacm:default-deny-write" on the 
   'factory-reset' RPC, leaving it to applications to configure the
    access control settings."

Appendix B should have a note to the RFC Stream Editor to 
remove it when the draft is published.

Kent 

> On Nov 1, 2019, at 10:57 AM, Kent Watsen <[email-masked]> wrote:
> 
> _______________________________________________
> netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

---

From: Kent Watsen <[email-masked]>
Date: Fri, 1 Nov 2019 15:20:57 +0000

ABORT  - sorry wrong working group!   :egg-face: 

Kent  // hiding hat

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 04 Nov 2019 12:39:23 +0100 (CET)

Hi,

I have reviewd version 06 of this draft, and have some comments:

o  Abstract and Introduction

  These both contain:

   Optionally a new "factory-default" read-only datastore is defined,

  Perhaps change to:

   A new optional read-only datastore "factory-default" is defined,

o  Abstract and Introduction

  These both contain:

    The reset operation may be used e.g. during initial
    zero-touch configuration 

  and in the Introduction there's a reference to RFC 8572.

  But what does this actually mean?

o  Introduction

  OLD:

   NETCONF defines the <delete> operation that allows resetting the

  NEW:

   NETCONF defines the <delete-config> operation that allows resetting the

o  Section 2

  This section says:

    Factory-default content MAY be specified by one of the following
    means in descending order of precedence

    1.  <factory-default> datastore, if it exists;

    2.  by vendors using a file in YANG Instance Data
        [I-D.ietf-netmod-yang-instance-file-format] format or some other
        format in vendor's website or other places where similar off-line
        documents are kept;

    3.  In some implementation specific manner;

  I don't think this defines any useful behaviour, and suggest this
  text is removed.

o  Section 2

  This section says:

    For the server supporting zero touch bootstrapping mechanisms, the
    factory default configuration causes the bootstrapping process to
    execute,e.g.,the server resets configuration to device's factory
    default configuration,for the version of operating system software it
    is running.

  It is not clear to me what this is supposed to mean.  I think it
  means "if the factory default configuration specifies that ztp is to
  be run, then ztp will be run after a factory reset".  But this is
  kind of obvious...  So I suggest that this sentence is removed.

o  Section 4

  The YANG module contains the boilerplate text from RFC 8174, but the
  module doesn't use 8174-language, so I suggest the boilerplate text
  is removed.

o  Section 4

  Suggest:

  OLD:

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

  NEW:

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

o  Section 4

  The module augments a new leaf 'factory-default' into <get-config>.
  I suggest that we remove this.  Other datastores like intended are
  not available through get-config, and I see no reason why
  factory-default should be treated differently.

o  Section 4

  I recommend that you run the YANG module through

    pyang -f yang --keep-comments --yang-line-length 69 <FILE>

  in order to fix some formatting and indentation issues.

/martin