On Feb 13, 2012, at 5:16 PM, Joe Touch wrote:
> Hi, all,
>
> One additional transport suggestion:
>
> - it would be useful to include recommended configurations for TCP
> connections. Given these are multi-byte request/response exchanges, Nagle
> should be disabled, e.g.
One of the co-authors had this to
say:
>
> A bone headed implementation (either client or server) might send
> bits and pieces of a message (for example, it would be easy for
> an implementation to send the 2 bytes for the length of the
> message in one request and then the message in a second). This is
> exactly what you want the TCP protections for. So, I think we
> should be silent on this issue. I also can't see how there is
> any harm in leaving it enabled (even in a 'clean'
> implementation).
>
I tend to agree with him -- I don't see that this
is going to cause a problem if we don't disable
it. But maybe I'm missing something. Is there
an important problem if we don't specify this one
way or another?
Thanks -- Kim
>
> Joe
>
> On 2/13/2012 2: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
>> [email protected] 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
>>
>> ----
>>
>>
>>
> _______________________________________________
> dhcwg mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/dhcwg
_______________________________________________
Ietf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ietf
