[ipxe-devel] [RESEND PATCH] [dhcp] Extract timing parameters out to header and document

Alex Williamson alex.williamson at redhat.com
Tue Feb 24 14:43:00 UTC 2015


Hi Michael,

Can you suggest anything that would make you consider taking this patch?
Thanks,

Alex

On Mon, 2015-02-09 at 12:52 -0700, Alex Williamson wrote:
> iPXE uses DHCP timeouts loosely based on values recommended by the
> specification, but often abbreviated to reduce timeouts for reliable
> and/or simple network topologies.  Previous attempts to change the
> defaults to more spec-compliant values have met resistance and
> apathy, therefore this patch simply tries to extract the timing
> parameters to a config file and document them.  The resulting default
> iPXE behavior is exactly the same, but downstreams are now afforded
> the opportunity to implement spec compliant behavior via config file
> overrides.
> 
> I believe the following overrides defined in config/local/general.h
> provide sufficiently spec compliant DHCP timeouts:
> 
> /*
>  * PXE spec defines timeouts of 4, 8, 16, 32 seconds
>  */
> #undef DHCP_DISC_START_TIMEOUT_SEC
> #define DHCP_DISC_START_TIMEOUT_SEC	4
> #undef DHCP_DISC_END_TIMEOUT_SEC
> #define DHCP_DISC_END_TIMEOUT_SEC	32
> 
> /*
>  * Elapsed time used for early break waiting for ProxyDHCP, this therefore
>  * needs to be less than the cumulative time for the first 2 timeouts.
>  */
> #undef DHCP_DISC_PROXY_TIMEOUT_SEC
> #define DHCP_DISC_PROXY_TIMEOUT_SEC	11
> 
> /*
>  * Approximate PXE spec requirement using minimum timeout (0.25s) for
>  * timeouts of 0.25, 0.5, 1, 2, 4
>  */
> #undef DHCP_REQ_START_TIMEOUT_SEC
> #define DHCP_REQ_START_TIMEOUT_SEC	0
> #undef DHCP_REQ_END_TIMEOUT_SEC
> #define DHCP_REQ_END_TIMEOUT_SEC	4
> 
> /*
>  * Same as normal request phase, except non-fatal, so we extend the timer
>  * to 8 and set the early timeout to an elapsed time value that causes a
>  * break after the 4 second timeout.
>  */
> #undef DHCP_PROXY_START_TIMEOUT_SEC
> #define DHCP_PROXY_START_TIMEOUT_SEC	0
> #undef DHCP_PROXY_END_TIMEOUT_SEC
> #define DHCP_PROXY_END_TIMEOUT_SEC	8
> #undef DHCP_REQ_PROXY_TIMEOUT_SEC
> #define DHCP_REQ_PROXY_TIMEOUT_SEC	7
> 
> /*
>  * Same as above, retry each server using standard timeouts, extended by
>  * one so that we can increment to the next before a timer induced failure.
>  */
> #undef PXEBS_START_TIMEOUT_SEC
> #define PXEBS_START_TIMEOUT_SEC		0
> #undef PXEBS_END_TIMEOUT_SEC
> #define PXEBS_END_TIMEOUT_SEC		8
> #undef PXEBS_MAX_TIMEOUT_SEC
> #define PXEBS_MAX_TIMEOUT_SEC		7
> 
> Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
> ---
> 
> Michael, does this look reasonable or can you suggest a better
> mechanism for downstreams to tune DHCP timeouts for customers on
> topologies that require something more spec compliant without forking
> the code?  I know you don't want to change the default user
> experience so allowing build-time tuning via a local config header
> seemed like a compromise and I hope the additional documentation helps
> make the change worthwhile.  Thanks,
> 
> Alex
> 
>  src/config/general.h    |   61 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/include/ipxe/dhcp.h |   11 +-------
>  src/net/udp/dhcp.c      |   31 ++++++++++++++----------
>  3 files changed, 80 insertions(+), 23 deletions(-)
> 
> diff --git a/src/config/general.h b/src/config/general.h
> index 5392034..bead2ac 100644
> --- a/src/config/general.h
> +++ b/src/config/general.h
> @@ -182,6 +182,67 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  #undef	GDBUDP			/* Remote GDB debugging over UDP
>  				 * (both may be set) */
>  
> +/*
> + * DHCP and PXE Boot Server timeout parameters
> + *
> + * Initial and final timeout for DHCP discovery
> + *
> + * The PXE spec indicates discover request are sent 4 times, with timeouts
> + * of 4, 8, 16, 32 seconds.  iPXE by default uses 1, 2, 4, 8.
> + */
> +#define DHCP_DISC_START_TIMEOUT_SEC	1
> +#define DHCP_DISC_END_TIMEOUT_SEC	10
> +
> +/*
> + * ProxyDHCP offers are given precedence by continue to wait for them after
> + * a valid DHCPOFFER is received.  We'll wait through this timeout for it.
> + * The PXE spec indicates waiting through the 4 & 8 second timeouts, iPXE
> + * by default stops after 2.
> + */
> +#define DHCP_DISC_PROXY_TIMEOUT_SEC	2
> +
> +/*
> + * Per the PXE spec, requests are also tried 4 times, but at timeout intervals
> + * of 1, 2, 3, 4 seconds.  To adapt this to an exponential backoff timer, we
> + * can either do 1, 2, 4, 8, ie. 4 retires with a longer interval or start at
> + * 0 (0.25s) for 0.25, 0.5, 1, 2, 4, ie. one extra try and shorter initial
> + * timeouts.  iPXE by default does a combination of both, starting at 0 and
> + * going through the 8 second timeout.
> + */
> +#define DHCP_REQ_START_TIMEOUT_SEC	0
> +#define DHCP_REQ_END_TIMEOUT_SEC	10
> +
> +/*
> + * A ProxyDHCP offer without PXE options also goes through a request phase
> + * using these same parameters, but note the early break below.
> + */
> +#define DHCP_PROXY_START_TIMEOUT_SEC	0
> +#define DHCP_PROXY_END_TIMEOUT_SEC	10
> +
> +/*
> + * A ProxyDHCP request timeout should not induce a failure condition, so we
> + * always want to break before the above set of timers expire.  The iPXE
> + * default value of 2 breaks at the first timeout after 2 seconds, which will
> + * be after the 2 second timeout.
> + */
> +#define DHCP_REQ_PROXY_TIMEOUT_SEC	2
> +
> +/*
> + * Per the PXE spec, a PXE boot server request is also be retried 4 times
> + * at timeouts of 1, 2, 3, 4.  iPXE uses the same timeouts as discovery,
> + * 1, 2, 4, 8, but will move on to the next server if available after an
> + * elapsed time greater than 3 seconds, therefore effectively only sending
> + * 3 tries at timeouts of 1, 2, 4.
> + */
> +#define PXEBS_START_TIMEOUT_SEC		1
> +#define PXEBS_END_TIMEOUT_SEC		10
> +
> +/*
> + * Increment to the next PXE Boot server, if available, after this this much
> + * time has elapsed.
> + */
> +#define PXEBS_MAX_TIMEOUT_SEC		3
> +
>  #include <config/named.h>
>  #include NAMED_CONFIG(general.h)
>  #include <config/local/general.h>
> diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h
> index bcfb85c..d1dc5bc 100644
> --- a/src/include/ipxe/dhcp.h
> +++ b/src/include/ipxe/dhcp.h
> @@ -18,6 +18,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
>  #include <ipxe/uuid.h>
>  #include <ipxe/netdevice.h>
>  #include <ipxe/uaccess.h>
> +#include <config/general.h>
>  
>  struct interface;
>  struct dhcp_options;
> @@ -639,16 +640,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 )
> -
> -/** Maximum time that we will wait for Boot Server responses */
> -#define PXEBS_MAX_TIMEOUT ( 3 * TICKS_PER_SEC )
> -
>  /** Settings block name used for DHCP responses */
>  #define DHCP_SETTINGS_NAME "dhcp"
>  
> diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
> index 04fad04..3527c44 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;
> +	/** Timeout parameters */
> +	uint8_t min_timeout_sec;
> +	uint8_t max_timeout_sec;
>  };
>  
>  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_sec * TICKS_PER_SEC;
> +	dhcp->timer.max_timeout = state->max_timeout_sec * TICKS_PER_SEC;
>  	start_timer_nodelay ( &dhcp->timer );
>  }
>  
> @@ -415,7 +415,7 @@ static void dhcp_discovery_rx ( struct dhcp_session *dhcp,
>  	/* If we can't yet transition to DHCPREQUEST, do nothing */
>  	elapsed = ( currticks() - dhcp->start );
>  	if ( ! ( dhcp->no_pxedhcp || dhcp->proxy_offer ||
> -		 ( elapsed > PROXYDHCP_MAX_TIMEOUT ) ) )
> +		 ( elapsed > DHCP_DISC_PROXY_TIMEOUT_SEC * TICKS_PER_SEC ) ) )
>  		return;
>  
>  	/* Transition to DHCPREQUEST */
> @@ -431,7 +431,8 @@ static void dhcp_discovery_expired ( struct dhcp_session *dhcp ) {
>  	unsigned long elapsed = ( currticks() - dhcp->start );
>  
>  	/* Give up waiting for ProxyDHCP before we reach the failure point */
> -	if ( dhcp->offer.s_addr && ( elapsed > PROXYDHCP_MAX_TIMEOUT ) ) {
> +	if ( dhcp->offer.s_addr &&
> +	     ( elapsed > DHCP_DISC_PROXY_TIMEOUT_SEC * TICKS_PER_SEC ) ) {
>  		dhcp_set_state ( dhcp, &dhcp_state_request );
>  		return;
>  	}
> @@ -447,7 +448,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_sec	= DHCP_DISC_START_TIMEOUT_SEC,
> +	.max_timeout_sec	= DHCP_DISC_END_TIMEOUT_SEC,
>  };
>  
>  /**
> @@ -584,7 +586,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_sec	= DHCP_REQ_START_TIMEOUT_SEC,
> +	.max_timeout_sec	= DHCP_REQ_END_TIMEOUT_SEC,
>  };
>  
>  /**
> @@ -669,7 +672,7 @@ static void dhcp_proxy_expired ( struct dhcp_session *dhcp ) {
>  	unsigned long elapsed = ( currticks() - dhcp->start );
>  
>  	/* Give up waiting for ProxyDHCP before we reach the failure point */
> -	if ( elapsed > PROXYDHCP_MAX_TIMEOUT ) {
> +	if ( elapsed > DHCP_REQ_PROXY_TIMEOUT_SEC * TICKS_PER_SEC ) {
>  		dhcp_finished ( dhcp, 0 );
>  		return;
>  	}
> @@ -685,7 +688,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_sec	= DHCP_PROXY_START_TIMEOUT_SEC,
> +	.max_timeout_sec	= DHCP_PROXY_END_TIMEOUT_SEC,
>  };
>  
>  /**
> @@ -810,7 +814,7 @@ static void dhcp_pxebs_expired ( struct dhcp_session *dhcp ) {
>  	/* Give up waiting before we reach the failure point, and fail
>  	 * over to the next server in the attempt list
>  	 */
> -	if ( elapsed > PXEBS_MAX_TIMEOUT ) {
> +	if ( elapsed > PXEBS_MAX_TIMEOUT_SEC * TICKS_PER_SEC ) {
>  		dhcp->pxe_attempt++;
>  		if ( dhcp->pxe_attempt->s_addr ) {
>  			dhcp_set_state ( dhcp, &dhcp_state_pxebs );
> @@ -832,7 +836,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_sec	= PXEBS_START_TIMEOUT_SEC,
> +	.max_timeout_sec	= PXEBS_END_TIMEOUT_SEC,
>  };
>  
>  /****************************************************************************
> 
> _______________________________________________
> ipxe-devel mailing list
> ipxe-devel at lists.ipxe.org
> https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel






More information about the ipxe-devel mailing list