|
Re: TSVDIR review of draft-ietf-dhc-dhcpv4-bulk-leasequery
2012-02-17 16:48:11
Hi, Kim,
First, thank you for your detailed response to my quite lengthy review.
Some further clarifications and confirmations appear below.
Joe
On 2/17/2012 12:09 PM, Kim Kinnear wrote:
Joe,
Thank you for your review.
My responses are indented, below...
On Feb 13, 2012, at 5:00 PM, Joe Touch wrote:
Hi, all,
> I've reviewed this document as part of the transport area
directorate'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 for their information and
to allow them to address any issues raised. The authors should
consider this review together with any other last-call comments
they receive. Please always CC tsv-dir(_at_)ietf(_dot_)org if you reply to or
forward this review.
This request was received Feb. 2, 2012.
This document describes an extension to DHCPv4 for the bulk query
andtransfer of current lease status over TCP.
The following transport issues were noted, and are significant:
UPDATES- The document updates DHCP with support for TCP, and as
suchthis document seems like it should UPDATE RFC2131
While this document clearly builds on RFC 2131, it
doesn't actually change anything that anyone is doing
that is currently based on RFC 2131. My understanding of
"updates" is that, in order to understand the first RFC
(in this case, RFC 2131), you need to read all of the
RFC's that "update" it. That isn't the case here -- you
can be very happy reading and implementing DHCPv4 by
reading RFC 2131 and never have to know that DHCPv4 Bulk
Leasequery exists. In general, in the DHC WG, we seem to
set a pretty high bar for what "udpates" another RFC. I
don't see that this document has met those requirements.
But this isn't really my call. I'll let Ralph Droms and
the DHC WG chairs decide on this one, and I'll do
whatever they tell me to do.
Agreed. FWIW, my "bar" for that is whether implementing DHCPv4 as per
RFC 2131 is changed by this doc. If DHCP fundamentally starts requiring
TCP port support, then this doc then changes the spec as described in
2131 IMO.
Here are the reasons I think it DOES modify DHCP:
- Sec 10 requests option codes that extend the tables defined
in the original RFC
I couldn't locate a recommendation on whether DHCP servers MAY implement
this extension, SHOULD, or MUST (did I miss it? if not, it should be
added). Regardless, that too would be a reason for this as an update to
2131. It's only NOT an update if this service is independent, IMO, and
it doesn't read that way to me.
PORT USE- Although DHCPv4 has an assigned TCP port, this document
should clearly indicate that there is no other specified use of that
. port other than the bulk lease query described in this document
Sure.
It should further explain why no other existing DHCP exchanges are
not valid on the TCP port.
I've been taken to task by various IETF folks at
varying levels of seniority for explaining too *much* of
the "why". My understanding is that this is a protocol
specification and not a design document, and as such it
is not only not necessary but frequently
counter-productive to give lots of justifications for the
decisions made when constructing the protocol.
In this case, I can't speak for the wG at large, because
we never proposed anything else. The reason we didn't
propose anything else was that we didn't want to "update"
RFC 2131 with a new way to do DHCPv4 in general. But
that all kind of comes down to "because we said so". We
were trying to keep this draft focused. But if I put
words in the document which say that, someone in the WG
would tell me to take them out.
I have no problem motivating the "interesting" decisions
we've made, or perhaps the contentious ones, but this one
was a no-brainer that never even came up (to the best of
my memory).
Having said all of that, I'll put something in that says
that we were trying to keep it simple.
Thanks. In this case, it's important to suggest why others should not
add conventional DHCP query support to the TCP port.
CONNECTION MANAGEMENT- The document describes the use of TCP
connections for bulk transfer, but needs to be more specific about which
side (relay client or server) closes the connection, under what
circumstances, and with what mechanism (e.g., graceful CLOSE vs. ABORT,
as per RFC 793)
Here is what we have now, 7.8 is for clients, 8.5 is for
servers:
7.8. Closing Connections [Client Section]
The requestor SHOULD close the connection after the
DHCPLEASEQUERYDONE message is received for the last outstanding query
that it has sent, if it has no more queries to send.
Under what conditions is it useful to not close at this time, and if so,
when/why is it eventually closed?
In this case it's also useful to explain what "no more queries to send"
means - are clients encouraged to keep the connection open in case there
are more to send, or do you expect that the client knows when it opens
the connection whether it has more to send - and if so, shouldn't it
have already sent them?
8.5. Closing Connections [Server Section]
The DHCPv4 server SHOULD start a timer for BULK_LQ_DATA_TIMEOUT
seconds for a particular connection after it sends a
DHCPLEASEQUERYDONE message over that connection and if there is no
current query outstanding for that connection. It should clear this
Is this a SHOULD?
And should the timer be cleared or restarted?
Under what conditions is it useful to not close at this time, and if so,
when/why is it closed?
timer if a query arrives over that connection. If the timer expires,
the DHCPv4 server should close the connection.
Is this a SHOULD, as well?
Again, under what conditions is it useful to not close at this time, and
if so, when/why is it closed?
The server MUST close its end of the TCP connection if it encounters
an error sending data on the connection. The server MUST close its
end of the TCP connection if it finds that it has to abort an in-
process request. A server aborting an in-process request SHOULD
attempt to signal that to its requestors by using the QueryTerminated
status code in the status-code option in a DHCPLEASEQUERYDONE
message, including a message string indicating details of the reason
for the abort. If the server detects that the requesting end of the
connection has been closed, the server MUST close its end of the
connection.
OK. I found it a bit confusing to have this in separate places, but I
viewed this from the transport perspective. This is fine.
All connections are closed with a graceful CLOSE, and not
with an ABORT. I'll add that to the document.
I'm at a loss to know how to be more specific about which
side of the connection closes the connection. There are
rules for the client in 7.8, and rules for the server in
8.5.
Somewhere you should state that the protocol exchange typically involves
the client closing the connection, and that the server closes only under
exceptions.
sec 7.3 indicates some conditions for terminating connections; this
section should indicate which side performs this, and by what
method(CLOSE, ABORT)
Section 7.3 discusses in two places where the "requestor" may
drop the connection. The term "requestor" is used to
indicate which "side" performs the actions. All of
Section 7 is about "Requestor Behavior", and the specific
details of section 7.3 in all cases indicate that the
instructions for dealing with the connection concern the
"requestor" explicitly.
I'll enhance the explanation to include that CLOSE should
be used.
If there is a specific set of instructions that are
unclear, I don't see them, so it would help me if you
could give me more detailed guidance than "Section 7.3".
Thanks.
It's probably sufficient to replace "requestor" with "client" to be
consistent. However, this section then supercedes the description in 7.8
and 8.5 above - i.e., you are handling exceptions on the client side as
well.
I.e., 7.8 needs to include the exception cases listed in 7.3, or they
should just be moved there.
this sec also allows connections to stay open after all pending
transactions are complete (MAY); the rationale for this should be
given, or the connection MUST be closed.
I will add some words which clarify the reasons why you
might want to leave the connection open after all pending
transactions are complete.
FWIW, again this contradicts 7.8 which says the client SHOULD close
after the response is received if it has no more queries to send.
This says "MAY" leave open. That's not the complement to SHOULD close.
the same issue applies to sec 7.8 and 8 throughout; sec 8.5 is
particularly problematic on this issue because it discusses aborting a
request using in-band data, which may not be available if the connection
is closed using ABORT. the state of other pending connection shsould be
discussed too.
The in-band data is not required for proper protocol
operation, it is valuable because it can limit the amount
of data required when and if the connection is
reestablished. All connections are closed by CLOSE. I
assume that, in your comment, "other pending
connection[s]" means other data flows that might exist on
the TCP connection that is coming down. I will add some
words that say that those Bulk Leasequery data flows will
also be stopped.
OK.
TIMEOUTS- Sec 6.3 defines a timeout for the TCP connection; is
this intended to supercede any TCP timeout? or is it intended to be
the min of the TCP timeout and the specified one?
The BULK_LQ_DATA_TIMEOUT is completely application
specific and essentially independent of the underlying
connection timeout. I will add words to that effect
in the document.
OK.
This section should more carefully explain behavior when a
connection is dropped and the reason - e.g., timeout, abort,
close.
The application employing Bulk Leasequery doesn't
care about *why* the connection went away. It
doesn't have to know if it was abort, close, or
timeout. I will say that in this section.
Hmm. I would think that this information would be passed up to the
application.
INTERLEAVING- sec 7.7 says that the server MUST interleave
replies;
there doesn't seem a valid reason for this requirement. clearly the
receiver MUST tolerate interleaved replies. having the server interleave
replies is relevant only if each request/reply has its own timeout --
which should be overridden if there is another response in progress
anyway. This issue should be more clearly explained and motivated.
Robert Sparks offered clarification, which I like
a lot and have already agreed to use:
I didn't see that; can you send that to tsv-dir(_at_)ietf(_dot_)org and me?
Is the requirement you are really trying to capture "MUST
NOT block sending replies on new queries until all
replies for the existing query are complete."?
There were some other issues noted in this document. These
comments appear below, and although not focused on transport
issues, they represent significant issues that IMO should be
addressed as well.
NOTE - regarding some terminology issues, I did not survey current
DHCP RFCs for consistency, but IMO these terms should be corrected
even if they are then inconsistent with existing specs.
Joe
-----------
Major non-transport issues:
- In many places the doc allows inaction to substitute for either
positive or negative confirmation. IMO, this invites implementation
errors, and should be avoided. E.g., status return codes, data source
option missing, query-for-all-configured-IPs, etc.
While I personally tend to agree with you on this one, I
have clear direction from the DHC WG that this is *not*
the approach that the WG wants to take. Here is the
status code text:
A status-code option MAY appear in the options field of a DHCPv4
message. If the status-code option does not appear, it is assumed
that the operation was successful. The status-code option SHOULD NOT
appear in a message which is successful unless there is some text
string that needs to be communicated to the requestor.
This was wrangled over at some length in the DHC WG,
and this was the way that it needed to be to pass
last call. I don't feel that I can change it
at this time.
AOK. I'll go down on the record as noting this as a bad idea that
invites erroneous implementations, and the IESG can decide what it wants
to recommend.
- the data source option reserved codes need more detailed
specification. if these are intended for future use, then they MUST be
ignored by the receiver (at least). if they are to mean anything at all,
at least one bit (typically all of them) MUST be set to zero by the
transmitter for implementations that do not support any of the component
fields.
further, the length of this option MUST be 1
Yes, subsequent to the creation of this section,
Amanda Baber of IANA enlightened me regarding
the different between RESERVED and UNASSIGNED.
These should be UNASSIGNED, and I will add words
that say that a receiver should ignore the
bits that are unassigned.
Good.
- The protocol supports bulk transfer that is not data driven.
This could represent a security vulnerability by exposing
information that may not be on the data path (and thus already
accessible) to a relay agent. This should be discussed in the
security considerations section.
All data that is exposed has already been
exposed using UDP to regular DHCP port. But please see
my response to Stephen Farrell (which is too long to include
here) to see if it covers the issues about which you
have concerns.
Again, I didn't see it; please send a copy to tsv-dir(_at_)ietf(_dot_)org
- Integer quantities should be referred to as "unsigned 32-bit integers"
throughout.
Good point (though some are 16 bits). The signed-ness is not
specified and should be. I'll make sure that it is.
AOK.
- "VPN" is used throughout to refer to "private" addresses (RFC 1918); a VPN is
not just private addressing.
Hmm. Actually, VPN in this document does not refer to RFC 1918
private addressing. If this is important to you, could you
be more specific as to your concern?
OK. I don't understand how it's being used, but if it's consistent with
the rest of DHCP, that's fine.
However, I got thrown off by your use of 10.x.x.x.
There are appropriate addresses to use as examples in RFCs - see RFC
5737, and please change this example accordingly.
- (this is a nit with all IDs, FWIW) SHOULD and SHOULD NOT are
used throughout without context. Any time SHOULD is used instead of
MUST (or SHOULD NOT rather than MUST NOT), it is useful to explain
a viable exception. If no such exception exists, the rationale for
selecting SHOULD over MUST should be included.
This is an interesting approach, which is new to me. I
can see the value in looking for viable exceptions. I
think justifying SHOULD and not MUST is a bit much, and
wouldn't make it through the "keep it short and simple"
filters that the DHC WG seems to place on DHC drafts.
But as I said, I'll look at every SHOULD (NOT) and see if
I can find concise examples of reasons. But I'm
concerned that this will greatly lengthen the draft.
I would argue that any SHOULD that cannot be justified ought to be
converted to a MUST ;-) That still keeps it short.
- It would be useful to explain why STATUS-CODE strings MUST NOT
be null-terminated; is that a protocol violation, or are you just
saying that NULLs are valid in these strings? the description
should be clear that the string field describes the string length
without any termination characters.
I will clarify this. Nulls are not allowed in the strings.
DHCP options have an unfortunate history where some
large vendors null terminated their strings even though
they were not supposed to, so now we always make a big
deal out of it so it doesn't happen again.
Good.
- start-time-of-state is expressed as an offset from base-time;
this appears to be the only time indicated as an offset rather than
as an absolute. That inconsistency invites implementation errors;
IMO, this should be absolute too.
There are two kinds of time options here, input options
and output options. The input options are all absolute
times. The output options are all offset times from the
base time (which by design is the only absolute time in
the output packets).
That'd be very useful to explain this way. Is this typical for DHCP?
It's quite confusing IMO.
> ...There are several input options
defined in this document. The start-time-of-state is the
only output option defined in this draft. It is defined
as a offset to the base time in large part because the
*other* output options that are used in the DHCP packet
are *also* offsets to the base time. These options are
not new, but are options that already exist in other
RFC's. So, while it looks like the state-time-of-state
is the odd man out at first glance, it actually is an
offset time because the *other* options with which it
will appear are also offset times.
- option lengths: throughout, the doc refers to option lengths as
being "an octet"; they are *indicated* in one octet. Some are constant
(e.g., DCHP-STATE), some allow the contents of the octet to vary. Again,
this is an *unsigned integer* octet.
Yes, the size of an option is stored in
one octet. So, yes, the length is *indicated*
by the value in that one octet.
But the length may be 1 octet, it may be
5 octets, it may vary between 8 and 10 octets.
I don't understand your concern. Are you
saying that the length should not be
stated to be "1 octet" or "5 octets"?
The length should be "indicated in 1 octet". I'm just suggesting a
change to the way it's described. When you say "the length is an octet",
that is ambiguous and can mean that the length value is 1, not just that
this value is stored in one octet.
I will fix the integer to be specifically
unsigned (as previously indicated).
- some of the information provided (in DHCP-STATE) goes beyond
that of DHCP. It would be useful to motivate the need for this
information in a bulk query, and why it is not similarly available
for nodes using UDP (e.g., as an extension to DHCPv4, not just to
the bulk transfer command). again, absence of state information
should not indicate state. State should always be expressed
explicitly. these states are further meaningless without a state
diagram explaining them. if such a diagram is not possible (as
noted at end of sec 6.2.7), then the states are irrelevant and the
option should not be included.
The state transition diagram can certainly be drawn, but
as it is fully connected, such a picture conveys no
useful information.
It's sufficient to indicate this then.
> ... The transitions are not themselves
useful in any case, as the the Bulk Leasequery is a
snapshot of information at one instant of time. There is
every reason to assume that as the state moves from one
value to another that an external entity would not see
all (or even many) of these transitions. This is a
snapshot of the DHCP state, and could be nothing else in
the environment of Bulk Leasequery.
The reason why the state transition graph is fully
connected is due to the common practice in DHCPv4 of
having two servers running in a failover relationship.
There are then two state machines for the lease state,
and at times one "leads" and the other "follows", and at
other times the other "leads" and its partner "follows".
So, examined individually, the state machines for each
individual failover partner is fully connected because at
any time it may receive an update from its partner that
it decides to accept that will simply "set" the state to
the value supplied by the partner.
All of this is orthogonal to the Bulk Leasequery protocol
-- except that we want the user of the Bulk Leasequery
protocol to be able to make sense out of information
supplied by each of the failover partners. Or any
partners running any increased availability solution.
So, that is the reason why the dhcp-state option exists
at all -- to allow the user of Bulk Leasequery to make
sense out of the results of sending a Bulk Leasequery to
two different (but cooperating) DHCPv4 servers.
The DHC WG felt that this information was worthwhile to
have in this document, and I agree (or we wouldn't have
put it in).
Using the failover case as a justification would be helpful.
> - in sec 6.2.9 the term "not allowed" should be explained - are
they reserved for future use and thus ignored? or are they "not
allowed" - in which case the doc should indicate handling if they
appear.
Ah, good catch. The "not allowed" will become UNASSIGNED,
based on my conversation with Amanda Baber of IANA.
I will add words that suggest that they should be
ignored.
The point of this table is, actually, not how to
handle them (that is the up to the vpn-option draft/RFC),
but rather to define a single new value.
AOK.
- sec 7.4 states that the clock skew of zero is desired; this
assumes E2E delays under 1sec. An explanation of why this is
desired should be given, as well as the consequences of it not.
I have given specific instructions on how to handle non-zero
clock skews. There are no "consequences". You code it
so that it works with non-zero clock skew. Period.
It is desired because it is less confusing to humans.
I would argue that this then irrelevant, and there should be no
statement about a skew of zero being preferred, since it could drive
implementations that way unnecessarily.
Other non-transport issues:
- The document includes definitions and references to irrelevant
deployment and implementation issues, notably DSLAMs, concentrators, and
access concentrators. These should not appear formally; they should be
used only to usefully illustrate currently intended uses.
These appear in the text in order to give
the otherwise abstract design goals some concrete
examples. We already removed a lot of the design
goals, are you suggesting that they all be removed?
Actually, yes, except as examples in the intro/motivation. Elsewhere
they are IMO distracting.
- The doc refers to "real-time", which could imply requirements not supported. This
should be replaced with "rapid".
I'll fix this.
- "absolute time" *indicates* (rather than contains) the number of seconds...
I'll fix this.
- "third party agent" should be explained, i.e., neither DHCP client nor DHCP
server.
I will fix this.
- "downstream" and "upstream" should be defined as both away from the server *and towards
the client* ("away from the server" has two directions).
Sure, I'll add these words.
AOK to all above.
- sec 3 should refer to relays, and returning the entire set or
individual bindings; there is no reason to explain the goals in terms of
access concentrators.
I don't know about this. I could certainly change it,
but is this really important to you? There was quite a
bit of negotiation in the WG about these examples. I
don't want to restart that discussion and have it all
over again. Does this really matter that much to you?
The examples should focus on client/server/relay terminology. If other
example terms are used, they should be parenthetical examples:
relay (e.g., at a concentrator)
The point is that this is relevant without the concentrator. A
concentrator is just one current example of where a relay exists, not
the only one.
- sec 3.2 appears to provide contradictory advice - caching is required, but
should be avoided? it would be useful to resolve this inconsistency.
Use of Bulk Leasequery is designed to help you avoid
caching.
OK - it'd be useful to be clear on that in sec 3.2.
- sec 3.3 refers to 'fast path', but this term doesn't make sense in this
context because fast-path is a forwarding issue. it would be useful to explain
what you mean, and pick a different term
Yes, it is a forwarding issue, and many devices
will not forward unless they know that they are forwarding
to a device that has received a valid DHCPv4 address.
The point (well, one point) of Bulk Leasequery is to
allow these forwarders to gather data so that they
can make the correct decision in the fast path.
Per-IP validation of that sort of permission isn't a fast-path decision.
Fast-path involves forwarding and policy decisions (which are not
typically /32 decisions).
It would be useful to say this information is useful so such routers can
make the appropriate decision, but the path (fast or slow) is irrelevant
IMO.
------------
Regards -- Kim
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf
|
|