[Dots] Genart last call review of draft-ietf-dots-telemetry-19

[Robert Sparks via Datatracker <[e-mail]>]

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