Thanks Ted for taking my comments into consideration!
On Sat, Jul 8, 2023 at 12:12 AM Ted Lemon <mellon@xxxxxxxxx> wrote:
Dhruv, thanks for the review!On Tue, Jun 13, 2023 at 5:33 AM Dhruv Dhody via Datatracker <noreply@xxxxxxxx> wrote:Reviewer: Dhruv Dhody
Review result: Has Nits
# OPSDIR review of draft-ietf-dnssd-srp
I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG. These
comments were written with the intent of improving the operational aspects of
the IETF drafts. Comments that are not addressed in the last-call may be
included in AD reviews during the IESG review. Document editors and WG chairs
should treat these comments just like any other last-call comments.
The document is very clear and well-written. The motivation is described well.
A separate section dealing with Operational Considerations would be an
excellent addition that could explicitly deal with Backward compatibility,
Logging requirements, Default values settings, Monitoring requirements etc. See
RFC 5706 for inspiration. This is just a suggestion...Thanks. I think this isn't a bad idea in theory, but in practice the use cases we know about for SRP are all situations where there is no operator in the sense you mean: e.g. a Thread border router has no operator other than the end-user, who isn't going to rely on this document. A home CE router with an SRP server is in the same boat.I think there may be some value in thinking about the operations model for SRP when deployed in an enterprise or educational setting, but we don't actually have any experience to share, so it would be purely speculative. I think this probably needs to wait for a later document, unfortunately.
The document is ready. I have a few minor comments and nits -
## Minor
* I suggest the I-D explicitly state the default values which can be overridden
via configurations. The use of the word "typically" in section 3.2.5.3 is a bit
unusual.We're reporting how this is actually used in practice. I don't know how to write a specification that would be appropriate. We actuallly at one time specified a value of one hour (or maybe it was two, I can't remember) but then we saw that in practice, people were using very different values, sometimes a different value for paired devices than unpaired, etc. So our goal became to simply talk about the tradeoffs and make suggestions.* Section 8. My preference would be to disregard brevity and list all
considerations for "service.arpa" instead of relying on "home.arpa" in RFC8375.
In my reading, the text refers to homenet at places and seems incorrect to
blindly rely on it. Again just a suggestion and something to think about.The IANA review agreed with you, and this will be in the new version of the document.## Nits
* Expand IoT in Abstract. Also, put the abbreviation next to "DNS-Based Service
Discovery" as you use the abbreviation later on.I just took IoT out—it's not really accurate anyway. We give the abbreviation for DNSSD in the Introduction--it doesn't belong in the abstract.* Section 3.1.1.
* Remove the "," at "..a registration domain, or discover the default.."
* Remove the "," at "..mechanisms are possible, but are.."
* s/out of scope for this document/out of the scope of this document/
* Add a "," at "For these names they then discover" i.e. "For these names,
they then discover"These are issues for the RFC editor.* Section 3.2.4
* Expand TSIGGood point, changed to Secret Key Transaction Signatures.* I suggest rewording this "The goal is not to provide the level of
security of a network managed by a skilled operator."!The secdir review also complained about this. I deleted it—I think it's just confusing.* Add a suitable reference for "a YXDomain RCODE" (Section 3.2.5.2)Done.* Weird capitalization in "..both the Delete An RR From An RRset update and the
Add To An RRSet update,.." (Section 3.2.5.5.2)That's how we did it. I guess we could have used single quotes instead?* Section 3.3.1
* s/RFC2136/[RFC2136]/g -- If you don't want to make this update, consider
using a hyphen as in RFC2136-implementations etc.Should be an xref, fixed.
* Should you also state what happens when the MUST in this section are not
met?See "Valid SRP Update Requirements". I don't think it will help to add more text about this here.
* s/are rejected with Refused./are rejected with Refused RCODE./ (section 3.3.6)
OK* Section 6.1
* Add reference to TCP Fast OpenOK
* s/credentials to to update/credentials to update/Yup.
* Table 1, please remove the last "." in "default.service.arpa."; See
https://www.iana.org/assignments/locally-served-dns-zones/locally-served-dns-zones.xhtmlIt's an FQDN, so it's correct to have a '.' on the end.* The IDNITS has some warnings. I guess that no change is needed, but just
making sure -
https://author-tools.ietf.org/api/idnits?url="">It doesn't like i18n. These are peoples' names, so too bad for it. :)Thanks again for the review. I have applied your suggestions except as noted above.
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
