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

[Martin Bjorklund <[e-mail]>]

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