Subject: [Netconf] Fwd: ops-dir review of draft-ietf-netconf-restconf-15

From: Benoit Claise <[email-masked]>
Date: Mon, 18 Jul 2016 12:47:02 +0200

FYI.
Thanks Lionel

Regards, B.

-------- Forwarded Message --------
Subject: 	ops-dir review of draft-ietf-netconf-restconf-15
Resent-Date: 	Mon, 18 Jul 2016 03:39:04 -0700
Resent-From: 	[email-masked]
Resent-To: 	[email-masked], [email-masked], [email-masked], 
[email-masked], [email-masked], [email-masked], 
[email-masked]
Date: 	Mon, 18 Jul 2016 10:38:54 +0000
From: 	[email-masked]
To: 	[email-masked] <[email-masked]>
CC: 	[email-masked] 
<[email-masked]>, [email-masked] 
<[email-masked]>

I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review.  Document editors and WG chairs should treat these comments just like any other last call comments.

Document: draft-ietf-netconf-restconf-15
   
Summary:   This document describes an HTTP-based protocol that provides a programmatic interface for accessing data defined in YANG (v1 or v1.1), using the datastore concepts defined in NETCONF.
Status: Ready with issues/clarification
     
I'm (still) not (so) familiar with YANG and NETCONF and I've read the document as the newbie that I am. Therefore some of my comments may be irrelevant if out of context.

Summary:

The document is well written, the last version capturing most of the comments received so far.
The rational for this solution and the use of the protocol is clearly explained. And all the protocol aspects are clear enough for implementors.
Comments:
*Major comment: this solution defines an HTTP-based solution but with HTTP/1.1 in mind. It is said that HTTP/2 can be used but there is very few details on this possibility. More details are required if HTTP/2-based implementations are really foreseen .
*Major comment: the possible colocation of RESTCONF and NETCONF server could be improved. There are a lot of implied assumptions regarding the interactions between both servers that are not so trivial IMHO.
*Minor comment: Numerous cross-references between RESTCONF, NETCONF, YANG and HTTP may make the life difficult to the reader.

Detailed review provided below.

Regards,

Lionel

*************************
1.  Introduction

    This document defines an HTTP [RFC7230] based protocol called
    RESTCONF, for configuring data defined in YANG version 1 [RFC6020] or
    YANG version 1.1 [I-D.ietf-netmod-rfc6020bis], using the datastore
    concepts defined in NETCONF [RFC6241].

LM: What about HTTP/2 that "enables a more efficient use of network resources and a reduced perception of latency by introducing header field compression and allowing multiple concurrent exchanges on the same connection.  It also introduces unsolicited push of representations from servers to clients."?

1.1.3.  YANG

    The following terms are defined in [I-D.ietf-netmod-rfc6020bis]:

LM: It is missing, lately introduced in rfc6020bis:

    o  non-presence container

    o  presence container

1.3.  Data Model Driven API

    The RESTCONF protocol operates on a conceptual datastore defined with
    the YANG data modeling language.  The server lists each YANG module
    it supports using the "ietf-yang-library" YANG module, defined in
    [I-D.ietf-netconf-yang-library].  The server MUST implement the
    "ietf-yang-library" module, which MUST identify all the YANG modules
    used by the server.

LM: Could be clarified where it is listed: "and listing all implemented modules in the "/modules-state/module" list"

    The conceptual datastore contents, data-model-specific operations and
    event notifications are identified by this set of YANG modules.

LM: either this sentence is part of the previous paragraph or the sentence should be:

    "The conceptual datastore contents, data-model-specific operations and
     event notifications are identified by the set of YANG modules listed
     in the "/modules-state/module" list."

    The classification of data as configuration or non-configuration is
    derived from the YANG "config" statement.  Data ordering behavior is
    derived from the YANG "ordered-by" statement.

LM: "non-configuration data" is "state data" as per the definition of the "config" statement:
    
    "When a node is tagged with "config false",
    its subhierarchy is flagged as state data.  If it is tagged with
    "config true", its subhierarchy is flagged as configuration data."

1.4.  Coexistence with NETCONF

    RESTCONF can be implemented on a device that supports NETCONF.

LM: is it "RESTCONF server + NETCONF server" or any combination "client/server"?
LM: "colocated" is only relevant if there is protocol interaction between RESTCONF and NETCONF. And this interaction is described     in this section.
LM: when both servers are colocated, is it assummed that the RESTCONF server acts as a NETCONF client and "normal" NETCONF operations are performed between both?

    If the device supports :writable-running, all edits to configuration
    nodes in {+restconf}/data are performed in the running configuration
    datastore.  The URI template "{+restconf}" is defined in
    Section 1.1.6.

LM: Just for sake of clarity, "If the device supports" refers to the "if the NETCONF server colocated with the RESTCONF server supports", right? Applies to the rest of the section.

    Otherwise, if the device supports :candidate, all edits to
    configuration nodes in {+restconf}/data are performed in the
    candidate configuration datastore.  The candidate MUST be
    automatically committed to running immediately after each successful
    edit.

LM: "automatically" could be understood as "implicitly", that is not possible with NETCONF. If I'm correct, what is meant is that the edit operations in the candidate configuration datastore MUST be followed by a commit operation.

    Any edits from other sources that are in the candidate
    datastore will also be committed.  If a confirmed-commit procedure is
    in progress, then this commit will act as the confirming commit.

LM: s/If a confirmed-commit procedure is in progress/If a confirmed commit procedure is in progress,
LM: "in progress" means by another client?

    If
    the server is expecting a "persist-id" parameter to complete the
    confirmed commit procedure then the RESTCONF edit operation MUST fail
    with a "409 Conflict" status-line.

LM: s/If the server/If the NETCONF server
LM: this point is not clear for me. I understand from NETCONF that if a Persist-Id is not expected, the confirming commit must be issued on the same session that issued the confirmed commit. So even if the Persist-Id is not expected, the operaion will still fail. But I have certainly missed something.
LM: Would it be useful to give the associated error-tag to use in this case?

    If the device supports :startup, the device MUST automatically update
    the non-volatile "startup datastore", after the running datastore has
    been updated as a consequence of a RESTCONF edit operation.

LM: "automatically" here means "edit operation followed by a <copy-config> operation from the <running> to the <startup>"?

    If a datastore that would be modified by a RESTCONF operation has an
    active lock from a NETCONF client, the RESTCONF edit operation MUST
    fail with a "409 Conflict" status-line.

LM: Would it be useful to give the associated error-tag to use in this case?

2.  Transport Protocol Requirements

2.1.  Integrity and Confidentiality

    HTTP/2 [RFC7540] MAY be used for RESTCONF.  The server MUST respond
    using a single HTTP/2 stream for all client requests from a stream.
    The server MAY respond using same HTTP/2 stream that was used for the
    corresponding request.

LM: the rest of the document focuses on HTTP/1.1. Is this section is enough to know how to support RESTCONF over HTTP/2?

3.  Resources

    [...]

    A resource can be considered a collection of data and the set of
    allowed methods on that data.

LM: s/A resource can be considered a collection of data/A resource can be considered as a collection of data

3.1.  Root Resource Discovery

    [...]
    
    If the XRD contains more than one link relation, then only the
    relation named "restconf" is relevant to this specification.

LM: please expand XRD (Extensible Reference Descriptor)

3.4.1.1.  Timestamp

    The last change time is maintained and the "Last-Modified"
    ([RFC7232], Section 2.2) header is returned in the response for a
    retrieval request.  The "If-Unmodified-Since" header can be used in
    edit operation requests to cause the server to reject the request if
    the resource has been modified since the specified timestamp.

LM: s/The "If-Unmodified-Since" header/The "If-Unmodified-Since" header ([RFC7232], Section 3.4)

3.5.1.  Timestamp

LM: the use of the timestamp regarding configuration and non-configuration data is quite confusing. it said in the same section:

    For configuration data resources, the server MAY maintain a last-
    modified timestamp for the resource, and return the "Last-Modified"
    header when it is retrieved with the GET or HEAD methods.
    [...]
    This timestamp is only affected by configuration data resources, and
    MUST NOT be updated for changes to non-configuration data.

    For non-configuration data resources, the server MAY maintain a last-
    modified timestamp for the resource, and return the "Last-Modified"
    header when it is retrieved with the GET or HEAD methods.  The
    timestamps for non-configuration data resources are updated in an
    implementation-specific manner.

LM: what is the difference between the "two" last-modified timestamps?

3.5.2.  Entity tag

LM: same comment as above regarding the use of ETag for configuration and non-configuration data.

3.5.3.  Encoding Data Resource Identifiers in the Request URI

    [...]

    A RESTCONF data resource identifier is not an XPath expression.

LM: what does it mean "is not"? Can be removed IMHO

3.5.3.1.  ABNF For Data Resource Identifiers

    [...]
    Note 1: The syntax for "api-identifier" and "key-value" MUST conform
    to the JSON identifier encoding rules in Section 4 of
    [I-D.ietf-netmod-yang-json].

LM: either it is a note and for information or it is a requirement. Cannot be both. I guess that the note could be moved as body text with normative language.

3.5.4.  Defaults Handling

LM: should it be "Default Handling" or "Default handlings"? The name of the URI is "defaults"

3.6.  Operation Resource

    [...]

    If the "rpc" or "action" statement has an "input" section then
    instances of these input parameters are encoded in the module
    namespace where the "rpc" or "action" statement is defined, in an XML
    element or JSON object named "input", which is in the module
    namespace where the "rpc" or "action" statement is defined.

    If the "rpc" or "action" statement has an "input" section and the
    "input" object tree contains any child data nodes which are
    considered mandatory nodes, then a message-body MUST be sent by the
    client in the request.

    [...]

LM: why not skip this text (and the following part) and describe the input/output + message-body cases in the sections 3.6.1 and 3.6.2. This will avoid some misalignements, especially regarding the mandatory presence of the message-body. For instance, here it is said in section 3.6:

    If the "rpc" or "action" statement has an "input" section and the
    "input" object tree contains any child data nodes which are
    considered mandatory nodes, then a message-body MUST be sent by the
    client in the request.

    If the "rpc" or "action" statement has an "input" section and the
    "input" object tree does not contain any child nodes which are
    considered mandatory nodes, then a message-body MAY be sent by the
    client in the request.

LM: whereas in section 3.6.1:

    If the "rpc" or "action" statement has an "input" section, then the
    "input" node is provided in the message-body, corresponding to the
    YANG data definition statements within the "input" section.

LM: in which it is assumed that a message-body is always there when "input" is present.
LM: Moreover, in section 3.6, it is not said what is the payload body carried in the message-body.

3.7.  Schema Resource

    [...]
    To retrieve a YANG module, a client first needs to get the URL for
    retrieving the schema, which is stored in the "schema" leaf.  Note
    that there is no required structure for this URL.  The URL value
    shown below is just an example.

LM: why do not recommend a default structure?

3.9.  Errors YANG Data Template

    [...]  It
    is not considered a resource type because no instances can be
    retrieved with a GET request.

LM: s/It is not considered a resource type/It is not considered as a resource type

4.  Operations

    The RESTCONF protocol uses HTTP methods to identify the CRUD
    operation requested for a particular resource.

LM: CRUD operation/CRUD operations

    The following table shows how the RESTCONF operations relate to
    NETCONF protocol operations and edit operations, which are identified
    with the NETCONF "nc:operation" attribute.

LM: as the table gives the mapping between RESCONF and NETCONF operations, it could be explain why OPTIONS and HEAD as no equivalence in NETCONF, even if obvious.

4.2.  HEAD

LM: it is not said if the server MUST implement this method (as for OPTIONS). Same comment for all the other methods.

4.3. GET

LM: when relevant, the error-tag used with error indicated in the status-line header would be useful to ensure a correct mapping of error cases with NETCONF error cases. Comment valids for other methods where error cases are described.

4.5.  PUT

    The PUT method is sent by the client to create or replace the target
    data resource.  A request message-body MUST be present, representing
    the new data resource, or the server MUST return "400 Bad Request"
    status-line.

LM: for creation of data resource, are POST and PUT operations? If not, what is the difference? If they are, is there any preference for creation of a data resource?

LM: No specific comments on the rest of the document. Thank you.

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

.