[core] Benjamin Kaduk's Discuss on draft-ietf-core-yang-cbor-16: (with DISCUSS and COMMENT)
Benjamin Kaduk via Datatracker <firstname.lastname@example.org> Tue, 13 July 2021 22:33 UTC
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id DB50F3A0DD4; Tue, 13 Jul 2021 15:33:19 -0700 (PDT)
Content-Type: text/plain; charset="utf-8"
From: Benjamin Kaduk via Datatracker <email@example.com>
To: "The IESG" <firstname.lastname@example.org>
Cc: email@example.com, firstname.lastname@example.org, email@example.com, Carsten Bormann <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org
Reply-To: Benjamin Kaduk <email@example.com>
Date: Tue, 13 Jul 2021 15:33:19 -0700
Subject: [core] Benjamin Kaduk's Discuss on draft-ietf-core-yang-cbor-16: (with DISCUSS and COMMENT)
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/core>, <mailto:firstname.lastname@example.org?subject=unsubscribe>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/core>, <mailto:email@example.com?subject=subscribe>
X-List-Received-Date: Tue, 13 Jul 2021 22:33:20 -0000
Benjamin Kaduk has entered the following ballot position for draft-ietf-core-yang-cbor-16: Discuss When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-core-yang-cbor/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- (1) The statement "Bytes with no bits set at the end of the byte string are removed." in Section 6.7 seems confusing to the point of being potentially harmful, and I'm not sure why it needs to be there. In the context it appears in, it seems to leave the value to be used for the bit string offset in an ambiguous state. If the intent is that such strings should not be generated (and MAY/SHOULD/MUST be rejected by recipients), that's okay, but having them silently ignored is very surprising and may merit discussion. (2) I think we should discuss the relationship between this document and draft-ietf-core-sid, which are before the IESG at the same time. This document says that core-sid is "one example for" a specification defining the management of SIDs, but draft-ietf-core-sid claims to be the document that "defines the semantics, the registration, and assignment processes of YANG SIDs". I'm having a hard time seeing the two statements as compatible with each other, but maybe I'm missing something. (3) The second example of instance-identifier using SID (§6.13.1) seems malformed, with "key name country" appearing under both "list user" and "list authorized-key" and no "country" leaf within "list user" other than the one under "list authorized-key". (The actual identityref example appears to correctly only use "name" as the key for "list user" and not "list authorized-key".) (4) Relatedly, the second example of instance-identifier by name (§6.13.2) does not show a country for "authorized-key", and I'm not sure if that's a valid way to represent the given YANG element. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- 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.1 In the context of containers and other nodes from the data tree, CBOR map keys within inner CBOR maps can be encoded using deltas or SIDs. In the case of deltas, they MUST be encoded using a CBOR unsigned integer (major type 0) or CBOR negative integer (major type 1), depending on the actual delta value. [...] It's a little surprising that we only get this extended discussion on SID use in §4.2.1, when we already had §4.1.1 "Using SIDs in keys" that said almost nothing about it. 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). 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.4.1 Deltas of list members are equal to the SID of the current schema node minus the SID of the 'list'. This seems redundant with the list of "Delta values are computed as follows" from §4.2.1. 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.) Section 4.5.1 In some implementations, it might be simpler to use the absolute SID tag encoding for the anydata root element. The resulting encoding is as follows: Pedantically, what follows is diagnostic notation, not a CBOR encoding. Section 4.6 An anyxml schema node is used to serialize an arbitrary CBOR content, i.e., its value can be any CBOR binary object. anyxml value MAY contain CBOR data items tagged with one of the tags listed in Section 9.3. The tags listed in Section 9.3 SHALL be supported. The second sentence seems to both not start with a capital letter and have a singular/plural mismatch, both of which could be resolved by adding "An" at the start. Section 6.13.1 Single instance schema nodes MUST be encoded using a CBOR unsigned integer data item (major type 0) and set to the targeted schema node SID. Should we say something about the lack of a delta encoding in this context, analogous to §6.10.1?
- [core] Benjamin Kaduk's Discuss on draft-ietf-cor… Benjamin Kaduk via Datatracker