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

[[e-mail]]

Hi Francesca,

 

Please see inline Jon1>

 

Regards

 

Jon

 

From: Francesca Palombini [mailto: [e-mail]] 
Sent: 02 February 2022 13:57
To: [e-mail]; The IESG
Cc: [e-mail]; [e-mail]; [e-mail];
[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] <[e-mail]>
.



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