Re: [core] Benjamin Kaduk's No Objection on draft-ietf-core-yang-cbor-17: (COMMENT)

[Carsten Bormann <[e-mail]>]

Hi Ben,

It seems you didn’t send email with these comments we plucked out of the datatracker.  However, we want to make sure we address all of them, so here is a reply you can read or just file :-)

The comments in this mail, together with the comments of other ADs, led to draft-ietf-core-yang-cbor-19.  Since we narrowly missed to submit this before the IETF-113 deadline, we only submitted this today.  
-19 is intended to address all outstanding COMMENTS.

https://datatracker.ietf.org/doc/draft-ietf-core-yang-cbor/
https://www.ietf.org/archive/id/draft-ietf-core-yang-cbor-19.html
https://www.ietf.org/rfcdiff?url2=draft-ietf-core-yang-cbor-19.txt

> Thanks for addressing my previous Discuss points!

As I mentioned off-list, we actually have disentangled the two drafts core-yang-cbor and core-sid now so they no longer depend on each other.  Since moving to a mutual normative reference was one of your DISCUSS points, I feel I need to alert you to this (as I already did in an earlier private mail).

> 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?

Added a “MAY accept” in 0ffcde6

> 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".

We chose “, or" in 716a206

> 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?

We believe that should now be covered; see the various occurrences of “YANG data structure” (which is an RFC 8791 term).

> 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.

Added 8d08538

> 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.

The one in 3.3 also should be a MUST.
Changed in def18b4

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

See the other ADs' comments about this :-)
Added comment in 65dc7c0

> 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)?

4.2 certainly is better here.
Fixed in 62f42c2

> 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.)

It is hard to discuss this without saying bad things about the way this has defined in YANG, and my mother has told me to only ever say good things about other people.

> 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.

Passing on this for now…

> 
>   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.

Yes: a0be5db

> 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.

Indeed, 7b2970b

> 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.

Good point; addressed in fa37812

> 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!]
> 

Right!
Now 5f4e316

>   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.

I just turned this into an example: 56d8956

> 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.

Good point, I stole your text for 2c12398

> 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.

It is now informative.

> 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.)

It is already wrong, as it doesn’t mention structures etc.
Fortunately, we already got rid of the definition when defining “representation nodes”...

> 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.

I assume this is about 4.4?
Changing that would probably also change the YANG-CBOR.
Instead, I added “simplified”…  39d5c73

> 
> 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.)

CBOR people are used to recursion (in contrast to YANG people :-)

Grüße, Carsten