Re: [core] AD review of draft-ietf-core-senml-data-ct-04

[Carsten Bormann <[e-mail]>]

Hi Francesca,

thank you for this in-depth review.

> On 2021-08-23, at 22:07, Francesca Palombini <francesca.palombini=[e-mail]> wrote:
> 
> Thank you for the work on this document. Please take care of these minor comments at the same time as the Last Call comments.
> 
> Francesca
> 
> 1. -----
> 
> FP: First of all, I wanted to confirm with the WG that the status "proposed standard" is the appropriate one here. Just making sure this was considered by the working group and the authors, as I note that the standard track is not needed for the IANA registrations, which require only expert review.

This subject comes up so often that it is hard to remember whether we discussed it specifically for data-ct.
The document has started out as specifying standards track and it doesn’t look that ever changed, so I can’t find a specific event where we would have discussed this.
I also couldn’t find records of a discussion in the minutes of CoRE meetings of the last 2½ years or so (side note: I actually wrote a tool to mine the minutes as they are not arranged in a way that would aid that mining).

Generally, you are right that simply registering a couple of SenML labels does create the entries in the registry.  However, here the intention was to also standardize the information that goes in a field under those labels.  To make this document referenceable without causing the equivalent of a downref in an SDO that wants to depend on this field, it is best advanced as a standards-track document.

(I have said before that different treatment of the std vs. info distinction in different areas of the IETF is a leading cause for confusion, but this is probably not the document to fix this for.)

> 2. -----
> 
>  used to send various kinds of data.  In the example given in
>  Figure 1, a temperature value, an indication whether a lock is open,
>  and a data value (with SenML field "vd") read from an NFC reader is
>  sent in a single SenML pack.
> 
> 
> FP: Might be worth stating in the sentence above that the example uses JSON and base64 encoding.

Added:
+The example is given in SenML JSON representation, so the "vd" (data
+value) field is encoded as a base64url string (without
+padding), as per {{Section 5 of -senml}}.

Now in https://github.com/core-wg/senml-data-ct/commit/a3b0510 (with a few additional reference fixes).

> 3. -----
> 
> 
>  Media-Type-Name:  A combination of a type-name and a subtype-name
>     registered in [IANA.media-types] as per [RFC6838], conventionally
>     identified by the two names separated by a slash.
> 
> FP: RFC 6838 is an informative reference, I wonder if it wouldn't make more sense to bring it up to normative, given that type-name and subtype-name (and media types in general) need to be understood in this specification.

If all copies of RFC 6838 get deleted, the data-ct spec still makes sense.
The ABNF from 6838 is copied over, not referenced.
This is why this isn’t labeled as a normative reference, but there is no strong reason not to do that.

> 4. -----
> 
>  Content-Type:  A Media-Type-Name, optionally associated with
>     parameters (separated from the media type name and from each other
>     by a semicolon).  In HTTP and many other protocols, used in a
> 
> FP: Please add a reference to the RFC defining media types parameters.

That would be RFC 2045 (as far as the syntax is concerned — semantics is in RFC 2046).

Added a reference to RFC 2045 (amazingly, these RFCs don’t have a “references” section, so I don’t know whether RFC 2046 is a normative reference for 2045, but for all intents and purposes, it is):

-: A Media-Type-Name, optionally associated with parameters (separated from
+: A Media-Type-Name, optionally associated with parameters
+  ({{Section 5 of -mime1}}, separated from


RFC 6838 gives the syntax for parameter names as

      parameter-name = restricted-name

However, we opted to whole-sale copy the RFC 7231 syntax, which is a bit more permissive.

Now in https://github.com/core-wg/senml-data-ct/commit/20da9a8

> 5. ----
> 
>     "Content-Encoding"; we *never* use this term (except when we are
>     in error).
> 
> FP: It is not clear to me the goal of what stated in parenthesis.

Unfortunately, there are lots of RFCs that get this wrong (including our own Section 12.3 in RFC 7252…).
The parenthesis may be a bit terse to make this explicit.

Bron had some nice replacement text in his artart review, which turned into
https://github.com/core-wg/senml-data-ct/commit/90a3deb

> 6. -----
> 
> FP: There is a subtle difference between Content-Format and Content-Format-Spec, the latter being the "string representation" of the former. By reading the two definitions, it does feel like the same thing is specified twice, it might be worth moving the following to Content-Format-Spec:
> 
>      , identified by (1) a numeric identifier defined by the
>     "CoAP Content-Formats" subregistry of [IANA.core-parameters] as
>     per [RFC7252] or (2) a Content-Format-String.

I couldn’t quite make this work, as this then looks like Content-Format always is  represented as a pair — having the description as the alternative is best left in that definition.

> Also, Content-Format-Spec right now mentions "Content-Format number". If you don't do the change above, it might be worth adding the following clarification:
> 
> OLD:
>  Content-Format:  the combination of a Content-Type and a Content-
>     Coding, identified by (1) a numeric identifier defined by the
>     "CoAP Content-Formats" subregistry of [IANA.core-parameters] as
>     per [RFC7252] or (2) a Content-Format-String.
> NEW:
>  Content-Format:  the combination of a Content-Type and a Content-
>     Coding, identified by (1) a numeric identifier defined by the
>     "CoAP Content-Formats" subregistry of [IANA.core-parameters] as
>     per [RFC7252] (referred to as Content-Format number) or (2) a
>     Content-Format-String.
> 
> (Content-Format number is used also in section 3)

Added in https://github.com/core-wg/senml-data-ct/commit/6923d8c

> 7. -----
> 
> FP: I would have appreciated an example of use or the "bct" field in Section 4.

Added in https://github.com/core-wg/senml-data-ct/commit/e8181d6

[Ari: Can you cross-check this new example?]

> 8. -----
> 
> FP: I think an example using a Content-Format-String including a parameter could have been useful.

Added in https://github.com/core-wg/senml-data-ct/commit/78622c8

> 9. -----
> 
> FP: In Section 8, several columns for the registration of SenML labels are missing: CBOR label, exi ID.

As they should be:

RFC8428 12.2:  […] the
  CBOR labels SHOULD be left empty as CBOR will use the string encoding
  for any new labels. The EI column contains the EXI schemaId value of
  the first schema that includes this label, or it is empty if this
  label was not intended for use with EXI.

(Of course, we can discuss whether data-ct is “intended for use with EXI”.)

All the changes above, plus some related house-keeping, are in
https://github.com/core-wg/senml-data-ct/pull/5 — summary in https://github.com/core-wg/senml-data-ct/pull/5/files

Grüße, Carsten