Re: [dhcwg] WGLC for draft-ietf-dhc-dhcpv4-active-leasequery-00 - Respond by [redacted]

[Tomek Mrugalski <[e-mail]>]

On 24.02.2014, 20:42, Bernie Volz (volz) wrote:
> Reminder that this WGLC is outstanding.
> 
> We need feedback in order to determine whether to advance this document.
I finally got to do the review. I'm ashamed it took me so long. Sorry
about that.

The draft is in pretty good shape. It requires one more (small) update
and it will be ready to go.

Here are my comments:

Section 2.Terminology - "Bulk Leasequery". the definition could
be more explicit that it is about multiple addresses and multiple
bindings. Also, a reference to RFC6926 could be useful here.

Terminology does not define "requestor". If you don't want to duplicate
definitions from existing RFCs, adding "See RFC4388" would be a bit
minimalistic, but sufficient.

DHCPv4 client definition: "Internet host" => "IPv4 node" (I think it's
ok to stick with RFC1933 terminology. Any IPv4-capable device is a node.
A node could be either host or a router. It is possible to run DHCP
client on a router.)

A DHCPv4 server definition: is an Internet host => "IPv4 node" (you can
run server on a router)

"IP address binding" definition: IPv4 IP address => IPv4 address

There are many instances of "IP" in the whole text. In 2014 context it
can be misunderstood as either IPv4 or IPv6. It's better to be explicit
and replace all of them with "IPv4".

Section 3 in second paragraph mentions connection over TCP. Framing
format should be defined. It is defined in later sections, but it may be
confusing when reading sequentially. A simple forward reference will do
the trick.

Section 5.1 explains TCP message framing. It references Bulk leasequery
RFC (6926), but also includes its own framing text. Why? Potential
implementors may be confused if both texts (here and 6926) are not 100%
aligned. I'd like the text to be gone and use references to RFC6926 only.

Section 5.3: "listen for incoming TCP connections on the DHCPv4 server
port 67." => "listen for incoming TCP connections on port 67."

Why table in section 5.3 repeats values BULK_LQ_MAX_CONNS and
BULK_LQ_DATA_TIMEOUT from RFC6926, section 6.3? If you need to mention
them, it would be better to say "Values X and Y defined in RFC6926 apply."

Why are you sending DHCPLEASEUNASSIGNED at all? Why just not send
anything for leases that are not assigned? I presume (I haven't read the
draft, sorry) that DHCPv6 active leasequery does not attempt to send
unusued v6 leases, right? Not sending LEASEUNASSIGNED would make both
protocols to work in a more similar manner. Many DHCPv4 servers just
prepopulate lease database and keep leases even if they are unassigned.
But there are servers that do not have such a concept and use the same
principle as in DHCPv6 (where keeping unused leases is infeasible).

Section 7.3: After receiving DHCPLEASEQUERYSTATUS with a QueryTerminated
status from a server, the Requestor MAY close the TCP connection to that
server. Why not SHOULD or even MUST?

General comment: packet names like DHCPACTIVELEASEQUERY and
DHCPLEASEQUERYSTATUS are difficult to read. Perhaps the LEASEQUERY part
of it could be shortened to LQ (DHCPACTIVELQ, DHCPLQSTATUS)? This is
only a suggestion, feel free to ignore it. I undertsand that many of the
types are already defined in published RFCs, so only the new ones could
be shortened. But it would still be easier to read. Alternatively, a
separator (dash or underscore) could be used. Although typically DHCPv4
packet names do no use separators, DHCPv6 does and nobody has a problem
with that. So perhaps DHCP_ACTIVE_LEASEQUERY?

Is the catch up phase mandatory to implement on the server side?
Depending on how the server is implemented, adding a journal with
changes made to any leases may be problematic. I was wondering if the
catch up phase could be made optional (from the server implementor's
perspective). If the server doesn't support it, requestor would have to
do Bulk followed by Active leasequery (without query-start-time). I
think the second to last paragraph in section 8.2 attempts to tell that
already.

One thing is not clear for me. Consider a case that there are active
requestors listening to any updates via active leasequery. A new client
comes in and the server assigns a new lease. I presume that server will
send the leasequery update after it responded to the client, right? So
it won't affect the response time. This is NOT a lockstep mechanism we
discussed in failover context, right?

Although that should be a common sense among implementors, security
considerations should recommend that the feature is disabled by default.
Otherwise less than clueful admins may find that their servers are more
accessible than they'd like after an upgrade (or fresh installating).

There are several (minor) idnits reported. Once you upload next rev,
please make sure that the draft comes itnits clean.

Ok, that's it.

As an individual contributor, I think this document is almost ready and
I support its move forward.

As a shepherd, I'd like to ask authors to publish updated revision that
addresses all comments raised and it will be ready.

As a chair, I'll send a WGLC summary in couple minutes.

Tomek

p.s.
I wish more documents could enjoy that level of maturity at -00. Good
work, guys!