Skip to main content

Last Call Review of draft-ietf-dnssd-update-lease-07
review-ietf-dnssd-update-lease-07-dnsdir-lc-tale-2023-06-14-00

Request Review of draft-ietf-dnssd-update-lease
Requested revision No specific revision (document currently at 08)
Type Last Call Review
Team DNS Directorate (dnsdir)
Deadline 2023-06-13
Requested 2023-05-30
Authors Stuart Cheshire , Ted Lemon
I-D last updated 2023-06-14
Completed reviews Dnsdir Last Call review of -07 by David C Lawrence (diff)
Secdir Last Call review of -07 by Shivan Kaul Sahib (diff)
Genart Last Call review of -07 by Dale R. Worley (diff)
Tsvart Last Call review of -07 by Brian Trammell (diff)
Dnsdir Telechat review of -08 by David C Lawrence
Intdir Telechat review of -08 by Jean-Michel Combes
Assignment Reviewer David C Lawrence
State Completed
Request Last Call review on draft-ietf-dnssd-update-lease by DNS Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/dnsdir/WSvNuKc2DPCDobwOj8wynnSFVCg
Reviewed revision 07 (document currently at 08)
Result Ready w/issues
Completed 2023-06-14
review-ietf-dnssd-update-lease-07-dnsdir-lc-tale-2023-06-14-00
A minor substantive issue that I think should be addressed, then some
style nits.

For substance, this specification leaves to inference what a server
should do if it receives multiple dynamic update 'add' operations in
one message that carries this option.  I expect that you would say
that obviously the lease time should be applied to all data added in
the update, but it did take a little extra cognitive overhead for me
to come to that conclusion.

Along those lines, to be comprehensive you should explicitly specify
that the data change caused by a 'delete' operation is not covered by
the Update Lease option.  (I don't think an implementer would really
do that, but still, explicit is good.)

The rest of this is nits, mostly stylistic, and it is not expected
that you have to do anything about them.  I think an implementor could
work with the existing document well enough, and your disagreement
with any of my style preferences is, of course, fine.

While I personally like seeing the rationale for design decisions in
an RFC, and realize there is some disagreement about that, the
discussion of why you didn't use TTL is a distraction in the
introduction.  I'd move it toward the end, after the main body and
before Security Considerations.  I'd also rephrase the final sentence
to drop "short enough to minimize stale cached data" because it felt
awkward to me and you had just immediately prior already indicated
that there were short TTLs to avoid stale data in caches.

Section 2.1: s/Mechanism/Mechanisms/.  Though I see that RFC 8490 has
used the same singular, the referenced RFC 6891 and its predecessor
RFC 2671 have been consistent in that the name via the initialism is
with the plural.

I'd go one step further and also just use EDNS instead of EDNS(0) as
the shorthand to clean up the typography, though I see that we've been
inconsistent about this across documents.  For example, 8490 uses
EDNS(0) as you have, I used EDNS0 in RFC 7871, and Mark Andrews used
just EDNS in RFC 7314 -- which is how the DNS Terminology RFCs (7719
and 8499) label it with a nod to both EDNS0 and EDNS(0).  Call it
futureproofing; some day after the universal deployment of DNSSEC and
IPv6 we will have an EDNS(1) which will be backward compatible with
EDNS(0).  Right?  Right??

DNS-SD is only used once, in section 4, so could just have its
expansion and RFC reference provided there.

Section 4: Regarding TSIG, strictly speaking the RR need not appear
after the OPT RR; the OPT data need only be included it in the digest
but the message ordering is not defined in RFC 8945.  This is also
more generally about EDNS and TSIG and not specifically about
UPDATE-LEASE, and so it shouldn't be necessary to describe that in
section 4.

4.2: Because the value type is specified in seconds, I'd make the
number of seconds the main part of the "no shorter than" sentence and
move 30 minutes into the parenthetical.  The second mention of 30
minutes is fine as-is.

s/are for example 100/are, for example, 100/ per common style guides.

Section 5.1: s/records affected the previous/records affected by the
previous/

Is there a reference (RFC 6763 section?) that could be included to how
dnssd typically does dynamic update and prerequisite handling?  I
realized as I was reading section 5.1 about refresh messages that I
didn't really see how a refresh message would be different from a
registration message, particularly as the section says that a refresh
message can act as a registration message and also says that it's
formatted the same except maybe with regard to prerequisites.

Naively, I'd just do the equivalent of:

  update delete client.example.com a
  update add client.example.com a 192.168.1.102

.. but I'm pretty sure that is too simplistic.

I was left wondering just how "Refresh Message Format" (5.1) was
different from "Registration Message Format" (no section heading).
Maybe it shouldn't have a section heading that seems to call it
out as its own format rather than maybe having an additional
constraint on prerequisites. (I'm not even sure this is the right
interpretation.)

Also in that section, "the response from the server can be used to
determine how to proceed when the Refresh fails" could helpfully
describe the basic strategy or maybe use a "For example, ..."  Perhaps
the reference to how dnssd usually does dynamic update already has
discussion of handling failures?

I'd like to see a section added with at least one example
request/response pair, demonstrating the multiple 'add' situation,
presumably of a client staking a name claim on its A and AAAA
addresses.

Thank you for providing recommended values.