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

[[e-mail]]

Hi Francesca, 

Thank you for the careful review. The changes can be tracked at: https://tinyurl.com/dots-telemetry-latest

Please see inline for more context. 

Cheers,
Med

> -----Message d'origine-----
> De : Francesca Palombini via Datatracker <[e-mail]>
> Envoyé : mardi 1 février 2022 13:31
> À : The IESG <[e-mail]>
> Cc : [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail]
> Objet : Francesca Palombini's Discuss on draft-ietf-dots-telemetry-20:
> (with DISCUSS and COMMENT)
> 
> Francesca Palombini has entered the following ballot position for
> draft-ietf-dots-telemetry-20: 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/blog/handling-iesg-ballot-
> positions/
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-dots-telemetry/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thank you for the work on this document, which I found particularly well
> written and easy to read despite its length.
> 

[Med] Thanks.

> Many thanks to James Gruessing for the ART ART review:
> https://mailarchive.ietf.org/arch/msg/art/MbTX3xfg6tMeG6mieAsT7hA3gXs/,
> and to the authors for addressing James' comments.
> 
> I have an easy to fix DISCUSS concerning the examples, and a point about
> a MUST which should not be there IMO. I also have a number of comments
> and question that I hope will help improving the document (or my
> understanding of it).
> 
> I support Roman's DISCUSS, see
> https://trac.ietf.org/trac/art/wiki/TypicalARTAreaIssues#LanguageTags.
> 
> To answer Ben's note - IMO the text about "enterprise numbers" and
> "Private Enterprise Numbers" is clear enough.

[Med] Thanks. 

> 
> As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
> DISCUSS ballot is a request to have a discussion; I really think that
> the document would be improved with a change here, but can be convinced
> otherwise.
> 
> Francesca
> 
> 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. 

, 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).  

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? 


> 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).

> 
> 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"

> ----------------------------------------------------------------------
> 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.

> 
> 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.

> 
> 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. 

 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.  

_________________________________________________________________________________________________________________________

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.