Re: TSVDIR review of draft-ietf-dhc-dhcpv4-bulk-leasequery

>
> 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
>>> tsv-dir@... 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
>> dhcwg@...
>> https://www.ietf.org/mailman/listinfo/dhcwg
>