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@xxxxxxxx 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 and
transfer 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 such
this document seems like it should UPDATE RFC2131
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
It should further explain why no other existing DHCP exchanges are not
valid on 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)
sec 7.3 indicates some conditions for terminating connections; this
section should indicate which side performs this, and by what method
(CLOSE, ABORT)
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.
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.
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?
This section should more carefully explain behavior when a connection is
dropped and the reason - e.g., timeout, abort, close.
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.
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.
- 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
- 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.
- Integer quantities should be referred to as "unsigned 32-bit integers"
throughout.
- "VPN" is used throughout to refer to "private" addresses (RFC 1918); a
VPN is not just private addressing.
- (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.
- 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.
- 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.
- 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.
- 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.
- 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.
- 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.
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.
- The doc refers to "real-time", which could imply requirements not
supported. This should be replaced with "rapid".
- "absolute time" *indicates* (rather than contains) the number of
seconds...
- "third party agent" should be explained, i.e., neither DHCP client nor
DHCP server.
- "downstream" and "upstream" should be defined as both away from the
server *and towards the client* ("away from the server" has two directions).
- 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.
- sec 3.2 appears to provide contradictory advice - caching is required,
but should be avoided? it would be useful to resolve this inconsistency.
- 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
----
_______________________________________________
Ietf mailing list
Ietf@xxxxxxxx
https://www.ietf.org/mailman/listinfo/ietf