Encoding of Data Modeled with YANG in the Concise Binary Object Representation (CBOR)
draft-ietf-core-yang-cbor-20

[S4.6] [comment]

* If I'm understanding things correctly, it seems like the 'anyxml' name
  is straining the bounds of sensible naming -- in 7951 it seems to mean
  arbitrary JSON and here it seems to mean arbitrary CBOR.

  Oh well.

I support Benjamin's second DISCUSS point.

Section 2 imports the term "schema tree" which is not used in this document.

In Section 9.1, "Required Parameters" should be "N/A", not "none".  See RFC 6838 Section 5.6.

The layout of the table in Section 9.3 is a little confusing.  For instance, the first two rows are collectively describing a single entry, I think, but that's not obvious given there are horizontal lines between them.

Thank you to Shawn Emery for the SECDIR review.

Thanks for the efforts on this document.

Thank you for the work put into this document.

Special thanks to Marco Tiloca for his shepherd's write-up, which contains a good summary of the WG consensus and the doc reviews.

Please find below 2 non-blocking COMMENT points.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==

A generic comment about the operational issue of supporting TWO ways to encode a data node: either normal string or the SID. This means that either there is a 2-way negotiation mechanism or that all CORE nodes must support both encoding and have agreed on a common SID mappings. Section 7 only briefly touches this issue with "Content-Type" but not with "Accept".

-- Section 4.2.1 & 4.4.1 --
BTW, I like the idea of encoding a container with sequential SID and the delta CBOR encoding ;-)

Thanks for addressing my previous Discuss points!
With respect to my previous point (1) (relating to the handling of the
'bits' type if there are trailing bytes with no bits set), it now looks
like a recipient should be able to handle such things using the normal
rules without any issue, and given that, rejecting them doesn't make
much sense.  Do we want to say anything about how no special handling
on the recipient is needed in order to do the right thing if such zero
bytes are generated in violation of the requirement?

One new nit in the -17, in Section 7:

   This media-type represents a CBOR YANG document containing one or
   multiple data node values.  If the media-type parameter id is
   present, depending its value, each data node is identified by its
   associated namespace qualified name as defined in Section 3.3
   (id=name), by its associated YANG SID (represented as a SID delta or
   via tag 47) as defined in Section 3.2 (id=sid).  If no id parameter
   is given, both forms may be present.

This used to be a three-item list, but the last item is now a standalone
sentence.  That means that we need to replace the comma with the conjunction
"or".

The remainder of this comment section was generated by taking my comments
from the -16 and attempting to remove those that no longer apply.
It is possible I missed a change and erroneously left in some comments
that no longer apply.

I can see why it would not make sense to do so in this document that is
so tightly integrated with CORECONF, but is there any work to produce a
CBOR encoding for the "structure" extension from RFC 8791?

As a general note, I did not look particularly carefully at the example
CBOR encodings, on the assumption that they were automatically generated
as part of the document production process.

Section 4.2

   This specification supports two types of CBOR keys; SID as defined in
   Section 3.2 and names as defined in Section 3.3.

I suggest making an explicit statement about whether the two types of
keys can be comingled in the same container/etc, possibly just by
forward-reference to the media-type parameter.

Section 4.2.2

   [...] A namespace-qualified name
   MUST be used each time the namespace of a schema node and its parent
   differ.  In all other cases, the simple form of the name MUST be
   used.  [...]

We have a similar statement in §3.3 that is only "the simple form of the
name SHOULD be used" for the latter sentence.  It's not entirely clear
to me what causes the strength of requirement to be different between
the two cases.

Section 4.6

Do we want to explicitly acknowledge the mismatch between the "xml" in
"anyxml" and the actual CBOR contents?

Section 5.2

   The yang-data extensions encoded using names are carried in a CBOR
   map containing a single item pair.  The key of this item is set to
   the namespace qualified name of the yang-data extension container;
   the value is set to the CBOR encoding of this container as defined in
   Section 3.3.

I'm not sure if this is a nit or not, so putting it here: is §3.3 the
right reference for the encoding of the container (vs. §4.2)?

Section 6.6

Requiring the string form for enumeration constants in a union seems
like a big loss on encoded size.  Is it worth putting some discussion in
the document (or an appendix thereto) about why other options like
tagged integer are not usable?  (Similar discussion might be relevant
for the bits-in-union case.)

While it's banal, we might also put an example of the integer-encoded
form (as used by non-union contexts) so that the tagged-text-string
example isn't the only one given.

   Leafs of type enumeration MUST be encoded using a CBOR unsigned
   integer (major type 0) or CBOR negative integer (major type 1),
   depending on the actual value.  [...]

I think we should also mention the tagged-text-string case here.

Section 6.7

   In a number of cases the array would only need to have one element -
   a byte string with a small number of bytes inside.  For this case, it
   is expected to omit the array element and have only the byte array
   that would have been inside.  [...]

I think it is actually required (not just "expected") based on some
later text.

Section 6.10, 6.13

I strongly encourage explicit mention of the use of a CBOR tag when
these elements appear inside a union, analogous to the text in §6.6 and
6.7.  The list in §6.12 is pretty easy to miss, and the word "tag" does
not appear at all in these sections.

Section 7

   This specification defines the media-type "application/yang-
   data+cbor", which can be used without parameters or with the
   parameter "id=name" or "id=sid".

I think the media-type discussions should refer to the "id" parameter
and the two possible values for that parameter ('=' is not allowed in a
parameter name).
[ed. I was thinking something like "with the 'id' parameter set to
either 'name' or 'sid'" in this paragraph.  The subsequent paragraphs
look to be treated well in the -17; thanks!]

   This media-type represents a CBOR YANG document containing one or
   multiple data node values.  Depending on the presence and value of
   the media-type parameter "id", each data node is identified by its
   associated namespace qualified name as defined in Section 3.3
   ("id=name"), by its associated YANG SID (represented as a SID delta
   or via tag 47) as defined in Section 3.2 ("id=sid"), or either of
   these (no "id" parameter given).

I think that for identityref and instance-identifier we don't use the
tag 47 for absolute (non-delta) SIDs, at least outside of a union
context.

Section 8

It might be worth mentioning that when the 'id' parameter to the media
type is used, it's important to properly reject identifiers of the other
type, to avoid scenarios where different implementations interpret a
given content in different ways.

Section 10.1

It's not clear to me that RFC 6241 needs to be classified as normative,
at least based on the one place in the text where we reference it.


NITS

Section 2

   *  child: A schema node defined as a child node of a container, a
      list, a case, a notification, an RPC input, an RPC output, an
      action input, or an action output.

Is this enumeration of "parent" node types guaranteed to be fixed for
all time, or should we consider a more generic wording to describe it?
(Likewise for the "parent" definition.)

Section 4

The example from RFC 7317 may have truncated a little too much of the
content; "mandatory true" for choice transport, the "inet:" prefix for
"type inet:host" and "inet:port-number", etc.

Section 4.5

   *  CBOR map keys of any inner schema nodes MUST be set to valid
      deltas or names.

   *  The CBOR array MUST contain either unique scalar values (as a
      leaf-list, see Section 4.3), or maps (as a list, see Section 4.4).

I think a parallel list structure would be "CBOR arrays MUST contain
[...]".

   *  CBOR map values MUST follow the encoding rules of one of the
      datatypes listed in Section 4.

(A recursive reference, though I mostly trust the reader to pick out the
relevant subsections.)

Found terminology that should be reviewed for inclusivity:
 * Term "natively"; alternatives might be "built-in", "fundamental",
   "ingrained", "intrinsic", "original".
See https://www.rfc-editor.org/part2/#inclusive_language for background and
more guidance.

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 3.3. , paragraph 7, nit:
>  Section 6. The following examples shows the encoding of a 'hostname' leaf u
>                                    ^^^^^
You should probably use "show".

Section 4. , paragraph 2, nit:
> ta item (major type 5). A map is comprised of pairs of data items, with each
>                               ^^^^^^^^^^^^^^^
Did you mean "comprises" or "consists of" or "is composed of"?

Section 4.1. , paragraph 5, nit:
> ection 3.3. The following examples shows the encoding of a 'system-state' co
>                                    ^^^^^
You should probably use "show".

Section 5.2. , paragraph 7, nit:
> e element -a byte string with a small number of bytes inside. For this case,
>                               ^^^^^^^^^^^^^^^^^
Specify a number, remove phrase, use "a few", or use "some".

Section 6.8. , paragraph 2, nit:
> ized-key/key-data" (SID 1734) for user name "bob" and authorized-key "admin"
>                                   ^^^^^^^^^
It"s more common nowadays to write this noun as one word.

Section 6.9. , paragraph 6, nit:
> user" (SID 1730) corresponding to user name "jack". CBOR diagnostic notation
>                                   ^^^^^^^^^
It"s more common nowadays to write this noun as one word.

Document references draft-ietf-core-sid-15, but -16 is the latest available
revision.