Last Call Review of draft-ietf-dnssd-srp-20
review-ietf-dnssd-srp-20-tsvart-lc-ott-2023-06-13-00

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.

This draft defines the Service Registration Protocol (SRP) to be run on
top of DNS to perform dynamic registrations for DNS-SD in a point-to-point 
style (rather than using mDNS). The specification itself defines operations
using the regular DNS as "transport" and for framing and defines operations
and operational behavior.

As such, the document itself doesn't seem to raise any transport layer issues.
However, as DNS in itself can use UDP or TCP underneath, SRP may effectively
run on top of both UDP and TCP.  The specification alludes to this to some
extent, e.g., when discussing registration spoofing and suggests preferring
TCP for certain setups.  Since support for constrained devices is explicitly
called out, UDP is likely a legitimate transport option.

This could raise the issue that not much is said about UDP transmission behavior,
not in this spec, nor actually in most of the referenced ones.  RFC 1035 discusses
a retransmission timer to be set in the range of 2-5 seconds, none of the other
cited DNS specs seem to be explicit about this at all, probably because common 
sense govern DNS implementations and DNS queries, if resolved iteratively, generally
may not have more than a single transaction outstanding.  But does this also hold
for series of updates, e.g., after synchronized reboots or something similar?

I am not a DNS expert and there may be specs that define the behavior for UDP-based
operation more precisely or stating that only a single transaction to a given server
may be outstanding at a given point in time, in which case it may be worthwhile
explicitly pointing towards those.  If such don't exist, it may be worthwhile adding
a word of caution for controlling series of transmissions in case such happen.  

Non-transport nits:
Sect. 3.2.5.5.1 talks about the KEY-LEASE value and suggests that it be the same
as before (1st para) and then "nonzero" (3rd para).  What is supposed to happen if
it is nonzero but not the same as before? Maybe state this explicitly?

The text flow in the bulleted lists in section 3.3.1 is sometimes hard to parse and,
especially in 3.3.1.2 does not necessarily yield a complete sentence.

Sect. 3.3.4 states: "To delete an individual subtype, an SRP Update must be constructed
that contains the service type and all subtypes for that service".  Does this miss
something like "except for the subtype to be deleted" in the end?

The authors may want to do a careful pass on the normative language (upper case
but also the of can vs. MAY).