[netconf] Re: WGLC on draft-ietf-netconf-list-pagination-rc

["Rob Wilton (rwilton)" <[e-mail]>]

Hi,

I support the publication of this draft, subject to my comments on the base pagination draft.

Here are my last call comment/questions on this draft:

Moderate level comments:

(1) p 2, sec 1.  Introduction

   This document updates RFC 8040, as described in Section 2.
   Declaring "list" and "leaf-list" as valid resource targets for the
   GET operation is necessary for list pagination.  Declaring these
   nodes as valid resource targets for the DELETE operation merely
   completes the solution for RESTCONF.

I'm slightly uncomfortable with fixing the DELETE operation in a pagination document because it means that the behaviour of servers may well differ depending on whether they implement this RFC or not.  Arguably, it would possibly be better to split this out (basically section 2.2, and the examples) into its own separate RFC, rather than to combine it with the pagination draft.


(2) p 5, sec 2.3.  Query Parameters

   Per the conformance defined in Section 3.1 of
   [I-D.ietf-netconf-list-pagination], all of these parameters MUST be
   supported for all lists and leaf-lists, but servers MAY disable the
   support for some or all "config false" lists, as described in
   Section 3.3 of [I-D.ietf-netconf-list-pagination].

I find this text confusing since it both says that they MUST be supported for all lists at the same time as MAY not be supported for some lists.  I also question whether you want to repeat the 2119 language here, but perhaps that is necessary because the RESTCONF parameters are encoded differently in HTTP?


(3) p 5, sec 2.3.2.  The "offset" Query Parameter

   If the offset value exceeds the number of entries in the working
   result set, then a "416 Range Not Satisfiable" status-line MUST be
   returned with the error-type value "application", error-tag value
   "invalid-value", and SHOULD also include the "offset-out-of-range"
   identity as error-app-tag value.

The cursor parameter seems to be optional, and it provides guidance for the error to return if it isn't supported.  Similar text may be helpful here.  For some implementations and data-structures, supporting the offset parameter is potentially a lot more expensive to implement that the cursor parameter.  E.g., if you have a sorted tree data structure, then a lookup by key-name is O(log(n)), but iterating the data structure to first the element at a given index may be O(n).



Minor level comments:

(4) p 5, sec 2.3.1.  The "limit" Query Parameter

   If the limit value is invalid, then a "400 Bad Request" status-line
   MUST be returned with the error-type value "application" and error-
   tag value "invalid-value".

What is an invalid value?  I presume that means that the value is out of allowed range specified in 3.1.7 of the base draft, and I further presume that asking for a limit of 200, against a list that only contains 100 is not an invalid value.


(5) p 6, sec 2.3.3.  The "cursor" Query Parameter

   If the cursor value is unknown, i.e. the key does not exist, a "404
   Not Found" status-line MUST be returned with the error-type value
   "application" and error-tag value "invalid-value", and SHOULD also
   include the "cursor-not-found" identity as error-app-tag value.
   If the "cursor" query parameter is not supported on the target node,
   then a a "501 Not Implemented" status-line MUST be returned with
   error-type value "application" and error-tag value "operation-not-
   supported".

Note, from RFC 8040, section 4.8, a regular RESTCONF server would be expected to return 400 if this query parameter was not supported.  Hence I presume that a client would need to accept both 400 and 501?


(6) p 6, sec 2.3.4.  The "direction" Query Parameter

   If the direction value is invalid, then a "400 Bad Request" status-
   line MUST be returned with the error-type value "application" and
   error-tag value "invalid-value".

As per my comments on the base draft, some of these parameters in this section and the following ones are likely to not be supported by some implementations, and hence it would probably be helpful to indicate the error code to return if a given parameter is not supported.  I.e., should this just be 400, or should it be 501?


(7) p 6, sec 2.3.7.  The "where" Query Parameter

   Prefixes in the XPath expression MUST be YANG module names.

What does this actually mean/look like?  Is this even valid?  In an XPath expression, don't the prefixes have to map to XML namespaces?  Also, taking this description, then I would presume that the module name would be used at every level in the path.  Should this instead reuse the text from Section 8.4.8 of RFC 8040?



Nit level comments:

(8) p 5, sec 2.3.3.  The "cursor" Query Parameter

   The "cursor" querey parameter corresponds to the "cursor" parameter
   defined in Section 3.1.6 of [I-D.ietf-netconf-list-pagination].

querey => query


(9) p 7, sec 2.3.8.  The "sublist-limit" Query Parameter

   If the sumlist-limit value is invalid, then a "400 Bad Request"
   status-line MUST be returned with the error-type value "application"
   and error-tag value "invalid-value".

sumlist-limit => sublist-limit

Regards,
Rob


From: Mahesh Jethanandani <[e-mail]>
Date: Tuesday, 12 November 2024 at 22:43
To: NETCONF WG <[e-mail]>
Cc: NETCONF WG Chairs <[e-mail]>
Subject: [netconf] WGLC on draft-ietf-netconf-list-pagination-rc
This email begins a second two-week WGLC on:

draft-ietf-netconf-list-pagination-rc-05<https://datatracker.ietf.org/doc/draft-ietf-netconf-list-pagination-rc/>

Please take time to review this draft and post comments by Nov 26.
Favorable comments or concerns are equally welcomed.

Cheers.

Mahesh Jethanandani
[e-mail]<mailto:[e-mail]>