Re: [core] YANG to CBOR mapping version 1

[Michel Veillette <[e-mail]>]

Hi Peter

See answers inline [MV]

These changes will be submitted later today.

Regards,
Michel

-----Original Message-----
From: core [mailto:[e-mail]] On Behalf Of peter van der Stok
Sent: Tuesday, July 5, 2016 5:51 AM
To: Core <[e-mail]>
Subject: [core] YANG to CBOR mapping version 1

Hi authors,

Attached my review of the document.
I have not looked at the enum, union encoding as this was done by Andy.

______________________________________________________________________________
YANG to CBOR
Section 1 line 2; remove “only”

[MV] “only” removed

Page 5; the table is referenced nowhere

[MV] Adding to the text:
[MV] Table 1 below provides a summary of this notation.
[MV]
[MV] Adding after the table:
[MV] Table 1 : CBOR diagnostic notation summary

End of section 2.1 “Deltas are represented as positive number” is contradicted in section 4.2 “SIDs as keys” second bullet: “Clients and servers MUST support negative deltas.
Suggestion: Deltas are represented as numbers preceded by a + or – sign

[MV] Removing "or +123" in the first line of the table
[MV] Second note bellow the table is updated to:
[MV] •	Deltas are represented as numbers preceded by a + or – sign.
[MV] The use of the + sign for positive deltas represents an extension to
[MV] the CBOR diagnostic notation as defined by [RFC7049] section 6.

End page 5: “value need” -> “value needs”

[MV] Fixed

Section 3: “the specification supports three types of keys.”
I would suggest only 2: character strings and numbers.
The use of deltas is either application (CoOL) specific or YANG to CBOR mapping specific.
If it is YANG to CBOR specific, I would encourage the use of another CBOR type that specifies a delta. As specified here its use is ambiguous; and forces each application to specify explicitly what is meant.
In the case that the delta is CoOL specific, I recommend to remove it completely from this draft, and explain the use of Deltas in the appropriate CoOL draft.
In the latter case: What remains are two key types: number and character string.

[MV] The use of delta to encode integer based keys (SIDs) is an integral part of the YANG to CBOR mapping.
[MV] This optimization is part of the encoding and independent of the application.
[MV] The document is updated to keep only 2 type of keys

Last alinea of section 3: “entities generating” should that not be” 
agents (processes) generating”…

[MV] Updating to "application entities" (for now)
[MV] I can't use "agent" because this encoding is implemented by both the manager (client) and the agent (server).
[MV] I also want to stay generic since this mapping is not necessary restricted network management

“The CoAP content-Format Option …. In the first place.”  I don’t understand this phrase.

[MV] Sentence removed
[MV] I also don’t understand the intent of this sentence within the context of this draft.

Section 4.2 title:  ‘container” Schema Node. Is “CBOR map key” not a better title?

[MV] Titles of this draft are aligned with https://www.ietf.org/id/draft-ietf-netmod-yang-json-10.txt 
[MV] I'm renaming all titles of section 4 to "Data Node" to keep this alignment.

As suggested earlier: Reduce this to string (major type 3) and number (major type 0) keys.

[MV] Done

I think that a large number of the CBOR encoding examples in section 4.2 can be left out.
They can be generated any time by an interested implementer; and now confuse the issues.

[MV] I received many questions about the final encoding and the number of bytes required.
[MV] If I remove these examples, I'm concern that I will receive even more of these questions.
[MV] I agree that these examples are useless for peoples with a good knowledge of CBOR
[MV] but seem very welcome from newcomers. 

Section 4.4; the example is really complex; some explanation what this all illustrates would be welcome.
Why is the use of choice necessary to understand the list encoding?

[MV] I'm trying to use real life examples, the server list is the simplest example of list I found in a RFC.
[MV] If I use an subset of the list, I can probably use the "interface" list from RFC 7223.
[MV] A suggestion will be welcome.

I suggest to first present the names example followed by the number example.

[MV] SIDs should be the prevalent naming used by this mapping.
[MV] For this reason, I prefer to show SIDs first.

Section 5: I need more explanation to relate the instance examples to the type definitions.
For example section 5.1; does the 1280 come from {mtu: 1280} ?
Here we do need the CBOR encoding to see the relation between the type specification and the CBOR encoding.

[MV] Adding is section 5.1:
[MV] The following example shows the encoding of leaf 'mtu' set to 1280 bytes.
[MV] Adding is section 5.2:
[MV] The following example shows the encoding of leaf ' timezone-utc-offset' set to -300 minutes.
[MV] Adding is section 5.3:
[MV] The following example shows the encoding of leaf 'my-decimal' set to 2.57 minutes.
[MV] Adding is section 5.4:
[MV] The following example shows the encoding of leaf 'name' set to "eth0".
[MV] Adding is section 5.5:
[MV] The following example shows the encoding of leaf 'enabled' set to 'true'.
[MV] Adding is section 5.6:
[MV] The following example shows the encoding of leaf 'oper-status' set to 'testing'.
[MV] Adding is section 5.7:
[MV] The following example shows the encoding of leaf 'mybits' with the 'disable-nagle' and '10-Mb-only' set.
[MV] Adding is section 5.8:
[MV] The following example shows the encoding of leaf 'aes128-key' set to the value 0x1f1ce6a3f42660d888d92a4d8030476e.
[MV] Adding is section 5.9:
[MV] The following example shows the encoding of leaf 'interface-state-ref' set to the value "eth1".
[MV] Adding is section 5.10:
[MV] The following following example shows the encoding of leaf 'type' set to the value 'ethernetCsmacd'.
[MV] Adding is section 5.11:
[MV] The following example shows the encoding of leaf 'is-router' when present.
[MV] Adding is section 5.12:
[MV] The following example shows the encoding of leaf 'ip-address' when set to "2001:db8:a0b:12f0::1".
[MV] Adding is section 5.13:
More added, see http://core-wg.github.io/yang-cbor/draft-ietf-core-yang-cbor-latest.html for final result
See https://github.com/core-wg/yang-cbor/commit/28f5dae04c20c13667b5749541f036d3dbdb4b43#diff-083fd5e71ffd63b12d8cd2854c3ad1bf for a diff

Section 5.2 example illustrates major type 1? Not immediately clear unless you decode the CBOR.

[MV] Should I add more text than the note already added. (This example shows the encoding of leaf ' timezone-utc-offset' set to -300 minutes.)
[MV] If so, do you have a sentence to propose?

Section 5.3; Start phrase got lost 3 lines lower.

[MV] Fixed

In the example, where do I find in the CBOR encoding that the fraction is 2 digits?

[MV] The number of digits is not encoded. This information is considered an unnecessary metadata
[MV] similar to "units", text of an enumeration, name of a flag within bits. This was the consensus
[MV] of the group which is aligned with the statement in section 3 (In order to minimize the size of
[MV] the encoded data, the proposed mapping avoid any unnecessary meta-information beyond
[MV] those natively supported by CBOR.)
[MV] It might be worth it to raise this topic within a larger audience. 

Sections 5.4 - 5.8; see comments on section 5.1 Section 5.9 Here more text is needed to connect the complex YANG specification to “eth1” value.

[MV] Done

Section 5.10; what is a name-space qualified form? In YANG-JSON it is defined but may need some repetition here.

[MV] Adding 'namespace-qualified' in the list of terms listed in section 2.
[MV] Adding "Names and namespaces are defined in [I-D.ietf-netmod-yang-json] section 4.".

Why is the subject SIDs as identityref* split in two parts?

[MV] The intent is to keep the name and SID example together.
[MV] Replacing the title in the example section to "Encoding example using name", "Encoding example using SID",

And again I find the CBOR encoding difficult to relate to the definition example.


Section 5.11
CBOR diagnostic is: {isrouter: [null]} ?

[MV] Yes, an empty datatype have no value
[MV] When present, it is set to 'null'.
[MV] draft-ietf-netmod-yang-json-10 implements the same approach.

Section 5.13.
I don’t understand how this section relates to YANG to CBOR mapping. If the section refers to specifying an instance identifier in the request, I think it can be removed. The use of [key=value] as instance identifier is well explained in restconf draft. The use of numeric identifier is currently described in CoMI draft, and differently in CoOL draft. I propose to remove this section as it does not directly relate to CBOR encoding.

[MV] An instance-identifier is a YANG datatype which need to be supported.
[MV] See example leaf 'error-path' in http://www.netconfcentral.org/modulereport/ietf-restconf
[MV] We simply reuse this YANG datatype in the CoAP methods to minimize overhaul complexity.

However, I am missing how to transport a given list instance with its key values in the CBOR encoding. Section 4.4 describes the encoding of a complete list not a subset of its instances.

[MV] A list instance is a collection which is described in section 4.2.
_____________________________________________________________________________________________
Hope this helps,
Peter


--
Peter van der Stok
vanderstok consultancy
mailto: [e-mail]

_______________________________________________
core mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/core