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 operation from the to the "? 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.