Re: [Dots] Francesca Palombini's Discuss on draft-ietf-dots-telemetry-20: (with DISCUSS and COMMENT)

[Francesca Palombini <[e-mail]>]

Hi Med,

Thank you! Although, to be clear, I still would have preferred the actual CBOR types and CBOR diagnostic notation being used, the new note and text in 5.6 ease my concerns enough to remove the block.

Jon: I have a feeling you didn’t fully understand my point: I did not want all figures replaced with the equivalent of figure 8 of 9132. I was concerned that readers would interpret the current examples by being CBOR diagnostic notation rather than the equivalent JSON representation, so they would have misinterpreted all types, if they did not go and look at the table and realized the examples are in JSON. My preference would have been to have CBOR diagnostic notation while keeping the parameter names as key instead of using the CBOR key for it, which would have made the examples as readable as JSON, but with the additional text it is much more clear that the examples are JSON (that was hidden in one simple sentence in a section far far away, before), so I’ll remove the block.

Thanks again for the quick response and the addition to the text,
Francesca

From: [e-mail] <[e-mail]>
Date: Thursday, 3 February 2022 at 10:05
To: [e-mail] <[e-mail]>, Francesca Palombini <[e-mail]>, 'The IESG' <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: RE: [Dots] Francesca Palombini's Discuss on draft-ietf-dots-telemetry-20: (with DISCUSS and COMMENT)
Hi Francesca, all,

FWIW, in addition to the changes already made to Section 5.6, -21 includes this NEW note right before Figure 4 to further insist on the notations:


  Note: The payload of the message depicted in Figure 4 is CBOR-

        encoded as indicated by the Content-Format set to "application/

        dots+cbor" (Section 10.3 of [RFC9132]).  However, and for the sake

        of better readability, the example (and other similar figures

        depicting a DOTS telemetry message body) follows the conventions

        set in Section 5.6: use the JSON names and types defined in

        Section 12.

Cheers,
Med

De : Dots <[e-mail]> De la part de [e-mail]
Envoyé : mercredi 2 février 2022 16:52
À : 'Francesca Palombini' <[e-mail]>; 'The IESG' <[e-mail]>
Cc : [e-mail]; [e-mail]; [e-mail]; [e-mail]
Objet : Re: [Dots] Francesca Palombini's Discuss on draft-ietf-dots-telemetry-20: (with DISCUSS and COMMENT)

Hi Francesca,

Please see inline Jon1>

Regards

Jon

From: Francesca Palombini [mailto: [e-mail]<mailto:[e-mail]>]
Sent: 02 February 2022 13:57
To: [e-mail]<mailto:[e-mail]>; The IESG
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>
Subject: Re: Francesca Palombini's Discuss on draft-ietf-dots-telemetry-20: (with DISCUSS and COMMENT)

Hi Med,

Answers inline.

Thanks,
Francesca

From: [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>
…


Thank you for the careful review. The changes can be tracked at: https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-90283074e9eba628&q=1&e=a4b1b643-167e-4fad-bbea-997afe323130&u=https%3A%2F%2Ftinyurl.com%2Fdots-telemetry-latest

Please see inline for more context.

Cheers,
Med

> -----Message d'origine-----
…
> 1. -----
>
>    JSON encoding of YANG-modeled data is used to illustrate the various
>    telemetry operations.
>
> FP: There is an inconsistency between this text and the use of
> "application/dots+cbor" in the examples. Either the Content-Format
> should be changed

[Med] The content format "application/dots+cbor" is correct.
FP: Thank you for confirming that application/dots+cbor is the correct format. The inconsistency I noted is that in all the examples, instead of using CBOR diagnostic notation (which I expected for an example in CBOR), you use the JSON encoding for the CoAP payload. I also understand that it is for readability purposes (“status”: “attack-successfully-mitigated” is more readable than “status”: 2), but that can be easily mitigated by adding comments, instead of reporting the examples body in JSON when they should be CBOR. So for example:

OLD:            "status": "attack-successfully-mitigated",

NEW:           "status": 2,      ; "attack-successfully-mitigated",



Jon> Once I had implemented the YANG-CBOR encoding with the JSON type for all of the parameters for RFC9132 as per https://datatracker.ietf.org/doc/html/rfc9132#section-6 Table 5, it was then very easy to extend RFC 9132 Table 5 with this draft’s Table 3.  Enumerations from the YANG modules were also mapped from JSON to CBOR and back as appropriate (the above enum is from https://datatracker.ietf.org/doc/html/rfc9132#section-5.2).  Within my, and other implementations I know of, everything is processed as JSON and then converted to CBOR to go out over the wire, and anything coming in over the wire was converted to JSON before any further processing. This makes the implementation’s code much easier to read.



Jon> So looking the examples could be shown as CBOR with suitable comments, but it is hard work mapping RFC9132 Figure 8 with RFC9132 Figure 7 – especially as the parameter names are encoded as CBOR numbers, not strings.



Jon> My preference is to stay as-is.



By the way most of these changes are very simple: for example "start-time": "1608336568" would become "start-time": 1608336568

Jon> Actually, because of JSON limitations for handling 64 bit numbers (max ~ 2^53), this has to be represented as a string, not a number when using JSON representation.
, or all the examples should be adapted to JSON
> Content-Format (I am not sure about what Content-Format you should use
> then, you probably know better).

[Med] We are actually using the convention/notation introduced in RFC9132 to ease the readability of the bodies. RFC9132 includes an example to illustrate the CBOR equivalent (Figure 8).
FP: Just to be clear, I don’t think it is necessary to illustrate the examples in CBOR encoding (so the equivalent of Fig 8 in 9132), but I do think it is worth using CBOR diagnostic notation instead of JSON. Also because with no note about this, one could interpret the example to be CBOR diagnostic notation with all the types wrong (string every time it’s something else).
Jon> Actually no, the types are never wrong.  The CBOR JSON mapping is very exact and fully 2-way when using this draft’s Table 3.  Any potential ‘string’ ambiguity is ruled out when the parameter name (represented as a CBOR number) is also considered.

We have also another notation flavor in RFC9132-Figure 6, that we don't use here:

"Note that this figure (and other similar figures
   depicting a schema) are non-normative sketches of the structure of
   the message."

Would adding a note to basically recalling this context be useful?
FP: You do have the note in 5.6 to say that JSON is used, however I still think it would be best to fix the examples so that they use CBOR diagnostic notation.
Jon> The real challenge I have with Diagnostic Notation is that the parameter name is represented as a number, whereas when the CBOR is mapped into JSON with the parameter name as a textual name rather than a number, it is much easier to get my head around what is being referred to.
> My preference is to keep CBOR and keep the Content-Format. I went
> through all the examples and reported below the inconsistencies.
>
> Section 7.1.2, Figure 4, 5,
>
> FP: If in CBOR, the percentiles should be expressed with the tagged item
> Decimal fraction (represented as a tagged array).

[Med] Agree if we have to produce a figure similar to Figure 8 of RFC9132. The type used in this figure is String following this mapping table:

  +----------------------+-------------+------+---------------+--------+
  | Parameter Name       | YANG        | CBOR | CBOR Major    | JSON   |
  |                      | Type        | Key  |    Type &     | Type   |
  |                      |             |      | Information   |        |
  +======================+=============+======+===============+========+
  | low-percentile       | decimal64   |TBA3  | 6 tag 4       |        |
  |                      |             |      |  [-2, integer]| String |

(same comment applies for the subsection comments).
FP: No, you could report that as 4([-2, int]) (with int = the right value). Plus adding the comment next to it “; 5.00”.
Jon> See above response.

>
> 2. -----
>
> Section 7.2.1, Figure 11, 13, 15, 17
>
> FP: Same comment as 1: unit and peak-g should be unsigned in CBOR.
>
> 3. -----
>
> Section 7.3.1, Figure 19, 20
>
> FP: Same comment as 1: capacity and unit should be unsigned in CBOR.
>
> 4. -----
>
> Section 7.2.1, Figure 11, 13, 15, 17
>
> FP: Same comment as 1: unit and peak-g should be unsigned in CBOR.
>
> 5. -----
>
> Section 7.3.1, Figure 19, 20
>
> FP: Same comment as 1: capacity and unit should be unsigned in CBOR.
>
> 6. -----
>
> Figure 36, figure 43
>
> FP: Same comment as 1: unit, mid-percentile-g, start-time and attack-
> severity should be unsigned in CBOR.
>
> 7. -----
>
> Figure 45, 47
>
> FP: attack-status, unit, mid-percentile-g, peak-g should be unsigned. I
> also note that some of the attributes defined in 9132 are used here and
> on previous examples and have for values the full spelled out meaning
> for readability, instead of the actual parameter (for example "status",
> that should have value 2 for "attack-successfully-mitigated"), and I
> think that should be at least noted in the text before the example, or
> if you want to be more precise the "attack-successfully-mitigated" could
> be a comment next to the value 2.
>
> 8. -----
>
> Section 7.1.3
>
>    client, it MUST respond with a 4.04 (Not Found) error Response Code.
>
> FP: I have a preference to remove this MUST here - it is expected
> behavior that if a resource is not present the server responds with
> 4.04, so it is not necessary to add the requirement here, actually
> redefining the response code (although it is the expected one in this
> case) should be discouraged. I suggest
> replacing: s/it MUST respond/it responds. (Note that 7.1.4 has the text
> I would expect, describing the behavior but not adding any already
> existing requirements).
>
>

[Med] We are using the normative language here to be consistent with a similar behavior in 9132:

"...it MUST respond with a 4.04 (Not Found) error Response Code"
FP: You are right, I don’t like that either (unfortunately I missed it when reviewing 9132). See section 4.6 of https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-bcp56bis#section-4.6 for more context around this. I still think this is worth changing, and I’ll make sure to discuss with Ben during the telechat.
Jon> “4.04 (Not Found)” is well defined for CoAP https://datatracker.ietf.org/doc/html/rfc7252#section-5.9.2.5 and we are not trying to change its meaning here.  Hence, I think the use of MUST is valid here.
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
>
> 9. -----
>
> FP: Related to the DISCUSS point about encoding, it would be good to
> state somewhere in the text (possibly in section 5.6) that CBOR Key
> Values can be used instead of the parameter names (see section 13.1),
> but that for readability all examples use the full names.

[Med] Updated this text:

OLD:
  JSON encoding of YANG-modeled data is used to illustrate the various telemetry operations.

NEW:
JSON encoding of YANG-modeled data is used to illustrate the various telemetry operations. Parameters names are, thus, used rather than their CBOR key values.
FP: Thanks.
>
> 10. -----
>
> Section 7.1.2, tsid definition
>
>         This is a mandatory attribute. 'tsid' MUST follow 'cuid'.
>
> FP: I am not sure I understand the use of the term "follow" here.

[Med] As the order is important to build the target, the tsid must appear after cuid. Changed to "'tsid' MUST appear after 'cuid'."

 Also I
> am confused by the fact that tsid here is named as an "attribute" while
> instead it is defined as a Uri-Path parameter, and does not appear in
> the list of attributes of Figure 3.

[Med] That's correct because we do have:

'cuid' and 'tmid' MUST NOT appear in the PUT request message body.

We used to have those in the message body but changed the design to exclusively include them as Uri-Paths.
FP: Ok, thanks for the clarification.


>
> 11. -----
>
>    Many of the pre-or-ongoing-mitigation telemetry data use a unit that
>    falls under the unit class that is configured following the procedure
>    described in Section 7.1.2.  When generating telemetry data to send
>
> FP: Section 7.1.2 does not describe the procedure to configure the unit
> (which at this point in the text I can't find anywhere).

[Med] What is configured is the unit class, not the unit. The unit will be decided based on auto-scaling.
FP: Right, but 7.1.2 still does not describe the procedure to configure the unit class (it only mentions that it is possible to configure). Anyway, this is minor.
Jon> I would be expecting auto-scaling to not be sacrificing accuracy which was a part of your original question.
 All 7.1.2 says
> is:
>
>    DOTS clients can also configure the unit class(es) to be used for
>    traffic-related telemetry data among the following supported unit
>    classes: packets per second, bits per second, and bytes per second.
>    Supplying both bits per second and bytes per second unit-classes is
>    allowed for a given telemetry data.  However, receipt of conflicting
>    values is treated as invalid parameters and rejected with 4.00 (Bad
>    Request).
>
> what am I missing?
>
> 12. -----
>
> Figure 33
>
> FP: Please either add the Reponse header (HTTP/1.1 200 OK \ Content-
> Type:
> /yang-data+json + any other header field) (this is my preferred option)
> or change the caption to indicate that this is only the response
> content.
>

[Med] Will be fixed. thanks.

> 13. -----
>
> Section 8.2
>
>         This is a mandatory attribute. 'tmid' MUST follow 'cuid'.
>
> FP: Same question as 10.

[Med] Idem as above

>
> 14. -----
>
> Section 8.1.6
>
> FP: Suggestion to add a sentence reminding the reader that this exchange
> happens over the data channel and that's why HTTP and JSON are used. (As
> Ben made me notice, this is very clearly explained in section 4.1, but I
> had forgotten by the time I got to this point in the text).
>

[Med] ACK.

> 15. -----
>
> FP: question for my understanding that comes from my low experience with
> YANG:
> I see that connection-all and connection-percentile-and-peak are groups
> of the exact same containers. Do they both need to be defined because
> the description (and hence the use) is different?
>
>

[Med] connection-all includes also current values which are not part of the structure of connection-percentile-and-peak.
FP: Ok, thanks.

_________________________________________________________________________________________________________________________



Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc

pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler

a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,

Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.



This message and its attachments may contain confidential or privileged information that may be protected by law;

they should not be distributed, used or copied without authorisation.

If you have received this email in error, please notify the sender and delete this message and its attachments.

As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.

Thank you.