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@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 >> >> ---- >> >> >> > _______________________________________________ > dhcwg mailing list > dhcwg@xxxxxxxx > https://www.ietf.org/mailman/listinfo/dhcwg _______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf