Re: [dhcwg] 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(_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 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(_at_)ietf(_dot_)org
> https://www.ietf.org/mailman/listinfo/dhcwg
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf