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

[Qin Wu <[e-mail]>]

Thanks Martin, see reply inline below.

-----邮件原件-----
发件人: Martin Bjorklund [mailto:[e-mail]] 
发送时间: 2019年11月4日 19:39
收件人: [e-mail]; [e-mail]
主题: Re: [netconf] WG Last Call: draft-ietf-netmod-factory-default-05

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,

[Qin]:Okay.

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?
[Qin]: It means the key word zero-touch comes from RFC8572.

o  Introduction

  OLD:

   NETCONF defines the <delete> operation that allows resetting the

  NEW:

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

[Qin]:Good catch, thanks.
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.

[Qin]: Okay, since both you and Andy suggest to delete, the text can go away now.
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.

[Qin]: Joe in earlier comments wanted to make sure reboot would happen as part of this RPC,
Kent comed up with a few suggestion as above. I also feel a little redundant since restarting the node is side effect of 
Factory-reset has already been covered in the text. I am happy to
take it out if nobody is against it.

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.
[Qin]: "MAY" key word is used in this draft, I am not sure to remove boilerplate text.

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.";
     }
[Qin]:Okay.

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.
[Qin]: originally, it was added based on chair's suggestion.
I am okay to remove it if there is nobody against it.

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.

[Qin]:Okay, thanks.

/martin