Re: [Dots] [Last-Call] Genart last call review of draft-ietf-dots-telemetry-19

[Lars Eggert <[e-mail]>]

Robert, thank you for your review and thank you all for the following discussion. I have entered a Discuss ballot for this document based on my own review.

Lars

On 2022-1-22, at 1:01, Robert Sparks via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Robert Sparks
> Review result: Ready with Issues
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-dots-telemetry-19
> Reviewer: Robert Sparks
> Review Date: 2022-01-21
> IETF LC End Date: 2022-01-24
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: Ready for publication as a Proposed Standard RFC, but with issues and
> nits to consider before publication
> 
> Issues:
> -------
> 
> This document relies heavily on the requirements in the DOTS signal and data
> channel specifications (RFC9132 and RFC8783) saying authenticated encryption
> MUST be used. Explicitly calling that out in this document's security section
> would be worthwhile.
> 
> I am not a YANG Doctor. This document seems to make a reasonable but unusual
> use of YANG. I would really like to see YANG Doctor review of -19. I wonder if
> it would be possible to factor some of the more generic enums out into
> something that could be reused elsewhere. Why do you stop at discouraging
> rather than forbidding server deviations? The point of a specification like
> this is interoperability.
> 
> I'm a little uncomfortable with the passively stated requirement to use
> ietf-dots-rehoming (at "clients are assumed to follow") while that document is
> an Informative reference.
> 
> In 6.1.2, I'm confused by the requirement that "'tsid' values MUST increase
> monotonically when a new PUT is generated" combined with "If the dot server
> finds the 'tsid' parameter value conveyed in the PUT request in its
> configuration data and (it) has accepted the updated configuration parameters,
> a 2.04 (Changed) MUST be returned". What am I missing that would allow a PUT
> with a tsid that's already in the server's configuration data? Is there perhaps
> leftover tension here from earlier designs that were changed?
> 
> At the end of 7.1, there is a MUST NOT level requirement against repeating
> attack-descriptions that have been previously sent to a peer. Is it always the
> case that when these descriptions are sent, the message they are contained in
> is acknowledged? If not, consider adding discussion of how/whether a peer can
> ask for a description when it gets an attack-id it doesn't have a description
> for.
> 
> In 7.2, you require a freshly rebooted client to send a GET request to retrieve
> active 'tmid' values. Why? Why not let it just send a DELETE with no 'tmid'
> right away if that's what it would do after receiving the response to the GET
> anyhow?
> 
> In 7.3 at "The PUT request MUST be maintained in an active state", you mean the
> configuration that the PUT established must be kept. The PUT request itself has
> no state beyond its response. Consider saying something like "The
> pre-or-ongoing mitigation requests MUST be considered active" instead.
> 
> In the yang module, there's a reference at page 79 that points to "Section
> 4.4.2 of RFC UUUU." This is the only occurrence of UUUU in the document, and if
> it's meant to be replaced with 'this RFC', this document has no section 4.4.2.
> 
> Nits:
> ---------------
> 
> Last paragraph of introduction at "Nevertheless, when..". That sentence is
> particularly hard to parse, please simplify it.
> 
> Curious that you called out TCP port 80 in the discussion in 3.3. Why not 443?
> 
> At 4.1, where you say "that truly require reliable delivery", I think you mean
> something more like "that are important to deliver". Nothing here is trying to
> guarantee reliable delivery and the statement misleads the reader to look for
> reliable delivery mechanisms.
> 
> In 4.5 at "The reason for not including these keys...". That sentence does not
> parse. Please simplify.
> 
> In 4.6, the second paragraph is about validating messages, not examples. It's
> not clear the paragraph is useful as a roadmap through the document. Consider
> deleting it.
> 
> The outline structure of section 4 is odd. There are several Considerations
> subsections including "Generic Considerations" (which is jarring), and several
> subsections that are not considerations sections. Having them under a section
> titled "Design Overview" doesn't help pull them together. Perhaps a mild
> restructure/reorder would bring more clarity?
> 
> At 6.1, consider replacing "(with suitable default)." with ". Default values
> are defined later in this section."
> 
> In 6.1 you claim clients can negotiate configuration parameters with server. I
> don't find a negotiation protocol specified here. I think you mean clients can
> configure parameters at a servers, and servers can configure parameters at a
> client. There's not really a mechanism for a peer to say "no, I don't the
> percentile config you're trying to set and I won't do that".
> 
> The introduction of "a third-party DOTS server" in the narrative after Figure
> 13 is surprising. For the paragraph to make sense, the messages in the
> discussion before Figure 13 would also have to have gone to that third-part
> DOTS server. Consider calling out that the DOTS server may not be hosted at any
> of the ISPs at the beginning of 6.2.1 instead.
> 
> In 6.2.1 where you require servers to reject PUT requests with 0 capacity for
> all included links, consider saying "Use DELETE instead." Maybe say _why_ you
> require that PUT to be rejected.
> 
> In the sentence after Figure 19 (at least in the text version), you mean to
> point to Figure 20.
> 
> At 7.1 "DOTS telemetry attributes are optionally signaled and therefore MUST
> NOT be treated as mandatory fields in the DOTS signal channel protocol" is an
> odd thing to call out this way. I think this was mentioned in the earlier yang
> doctors review as well. Why is it here?
> 
> In 7.1.1 consider replacing "the same attributes" with simply "the attributes".
> 
> In 7.1.6, please replace "reiterates" with "repeats".
> 
> In 7.1.6, please replace the phrase "Concretely, " with "A".
> 
> 
> 
> --
> last-call mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/last-call