[ipxe-devel] [RESEND PATCH] [dhcp] More spec compliant DHCP/PXE timeouts

[Alex Williamson]

For discovery, both the DHCP and PXE specs suggest timeouts of 4, 8,
16, and 32 seconds.  This gives us a maximum timeout of 60 seconds,
versus the current timeout of 15 seconds at timeouts of 1, 2, 4, and
8 seconds.

On the request phase, the specs are quite different.  RFC2131 for DHCP
suggests the same intervals as above for discovery, while the PXE spec
recommends intervals of 1, 2, 3, and 4 seconds.  Our timer only wants
to do exponential back-off, so we compromise and adjust to intervals
of 1, 2, 4, and 8 seconds.  PXE boot server discovery appears to want
the same timeouts as the DHCP request phase.

Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
---

I posted this previously[1] and prior to that also sent a question to
the list asking why iPXE uses the non-spec compliant DHCP timeouts
that it does[2].  There was some support for this patch[3], but it was
never commited.  My ping a couple weeks later also came up short[4].
We do have customers complaining about this and unable to reliably
boot virtual machines over their production network due to these short
timeouts.  Could we consider a patch like below, or any sort of
re-implementation of it, to make iPXE DHCP behave more in accordance
with spec defined timeouts?  Thanks

[1] http://lists.ipxe.org/pipermail/ipxe-devel/2013-June/002573.html
[2] http://lists.ipxe.org/pipermail/ipxe-devel/2013-June/002562.html
[3] http://lists.ipxe.org/pipermail/ipxe-devel/2013-June/002576.html
[4] http://lists.ipxe.org/pipermail/ipxe-devel/2013-June/002610.html

 src/include/ipxe/dhcp.h |    4 ----
 src/net/udp/dhcp.c      |   22 +++++++++++++---------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h
index bcfb85c..595f0c1 100644
--- a/src/include/ipxe/dhcp.h
+++ b/src/include/ipxe/dhcp.h
@@ -639,10 +639,6 @@ struct dhcphdr {
  */
 #define DHCP_MIN_LEN 552
 
-/** Timeouts for sending DHCP packets */
-#define DHCP_MIN_TIMEOUT ( 1 * TICKS_PER_SEC )
-#define DHCP_MAX_TIMEOUT ( 10 * TICKS_PER_SEC )
-
 /** Maximum time that we will wait for ProxyDHCP responses */
 #define PROXYDHCP_MAX_TIMEOUT ( 2 * TICKS_PER_SEC )
 
diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index e6d3edd..87c8d6e 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -171,8 +171,9 @@ struct dhcp_session_state {
 	void ( * expired ) ( struct dhcp_session *dhcp );
 	/** Transmitted message type */
 	uint8_t tx_msgtype;
-	/** Apply minimum timeout */
-	uint8_t apply_min_timeout;
+	/** Timer parameters (seconds) */
+	unsigned long min_timeout;
+	unsigned long max_timeout;
 };
 
 static struct dhcp_session_state dhcp_state_discover;
@@ -272,9 +273,8 @@ static void dhcp_set_state ( struct dhcp_session *dhcp,
 	dhcp->state = state;
 	dhcp->start = currticks();
 	stop_timer ( &dhcp->timer );
-	dhcp->timer.min_timeout =
-		( state->apply_min_timeout ? DHCP_MIN_TIMEOUT : 0 );
-	dhcp->timer.max_timeout = DHCP_MAX_TIMEOUT;
+	dhcp->timer.min_timeout = state->min_timeout * TICKS_PER_SEC;
+	dhcp->timer.max_timeout = state->max_timeout * TICKS_PER_SEC;
 	start_timer_nodelay ( &dhcp->timer );
 }
 
@@ -447,7 +447,8 @@ static struct dhcp_session_state dhcp_state_discover = {
 	.rx			= dhcp_discovery_rx,
 	.expired		= dhcp_discovery_expired,
 	.tx_msgtype		= DHCPDISCOVER,
-	.apply_min_timeout	= 1,
+	.min_timeout		= 4,
+	.max_timeout		= 32,
 };
 
 /**
@@ -584,7 +585,8 @@ static struct dhcp_session_state dhcp_state_request = {
 	.rx			= dhcp_request_rx,
 	.expired		= dhcp_request_expired,
 	.tx_msgtype		= DHCPREQUEST,
-	.apply_min_timeout	= 0,
+	.min_timeout		= 1,
+	.max_timeout		= 8,
 };
 
 /**
@@ -685,7 +687,8 @@ static struct dhcp_session_state dhcp_state_proxy = {
 	.rx			= dhcp_proxy_rx,
 	.expired		= dhcp_proxy_expired,
 	.tx_msgtype		= DHCPREQUEST,
-	.apply_min_timeout	= 0,
+	.min_timeout		= 1,
+	.max_timeout		= 8,
 };
 
 /**
@@ -832,7 +835,8 @@ static struct dhcp_session_state dhcp_state_pxebs = {
 	.rx			= dhcp_pxebs_rx,
 	.expired		= dhcp_pxebs_expired,
 	.tx_msgtype		= DHCPREQUEST,
-	.apply_min_timeout	= 1,
+	.min_timeout		= 1,
+	.max_timeout		= 8,
 };
 
 /****************************************************************************