Dave, thanks for the review!
On Thu, Jun 15, 2023 at 2:56 AM David Lawrence via Datatracker <noreply@xxxxxxxx> wrote:
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.)
Good catch. I've added the following:
the key, and the LEASE applies to the other record. If more than one record that is not
a KEY record is added by the update, the LEASE (not the KEY LEASE) is applied to all such
records. Records that are removed are permanently removed.</t>
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.
I agree with you, and I've deleted this paragraph.
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.
OK
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??
I'd rather not change this this late in the process, although I agree with you in principle.
DNS-SD is only used once, in section 4, so could just have its
expansion and RFC reference provided there.
I think this isn't worth fixing, because it would bloat the place where the term is used in a confusing way, and I lack enthusiasm for coming up with a way to address that.
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.
I've deleted that text. We shouldn't be specifying how to do TSIG here anyway, as you say.
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.
Done.
s/are for example 100/are, for example, 100/ per common style guides.
Let's leave that to the RFC editor.
Section 5.1: s/records affected the previous/records affected by the
previous/
Yup.
Is there a reference (RFC 6763 section?) that could be included to how
dnssd typically does dynamic update and prerequisite handling?
We reference SRP, which is how DNSSD does DNS updates.
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 think this falls into the "don't specify the other protocol here." We're just specifying the update-lease option, really. I agree that the text about refreshes is a bit challenging, but I would have had to completely rewrite the document to fix that, and I don't think there's much point since this is better specified in the SRP document anyway. Stuart wanted to keep this text because you might use update-lease in some other way, but I think we'd need another document to clarify how that would work anyway, so it's okay if this isn't perfect--it's probably good enough.
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.)
This would require a document rewrite. I think it's okay as is. Yes, we could be more consistent, but it gets the point across.
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?
This document isn't a protocol specification for DNS update. We could remove that suggestion, but again I'd rather not make big changes at this point.
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.
Again, that would make sense in a DNS Update protocol spec, but not here.
Thanks for the review! Sorry I'm not being more cooperative. Possibly the ADs will school me... :)
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
