[U-Boot] [PATCH] TFTP: allow for adjustable retransmission timout

[Wolfgang Denk]

Dear Ben,

In message <1263768953-18819-1-git-send-email-wd at denx.de> I wrote:
> So far, TFTP negotiated a fixed retransmission timeout of 5 seconds.
> In some cases (busy networks, slow TFTP servers) this caused very
> slow transfers. Add new environment variable "tftptimeout" allows to
> set this timeout. Lowering this value may make downloads succeed
> faster in networks with high packet loss rates or with unreliable
> TFTP servers.
> 
> Signed-off-by: Wolfgang Denk <wd at denx.de>
> Cc: Ben Warren <biggerbadderben at gmail.com>
> ---
> I submitted this patch for RFC in October, but never received any
> feedback. Let's see if it goes in or gets rejected...
> 
>  README     |   19 ++++++++++++++++---
>  net/tftp.c |   22 +++++++++++++++++-----
>  2 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/README b/README
> index 22e35c3..13bad41 100644
> --- a/README
> +++ b/README
> @@ -2982,7 +2982,9 @@ environment. As long as you don't save the environment you are
>  working with an in-memory copy. In case the Flash area containing the
>  environment is erased by accident, a default environment is provided.
>  
> -Some configuration options can be set using Environment Variables:
> +Some configuration options can be set using Environment Variables.
> +
> +List of environment variables (most likely not complete):
>  
>    baudrate	- see CONFIG_BAUDRATE
>  
> @@ -3094,7 +3096,7 @@ Some configuration options can be set using Environment Variables:
>  		  available network interfaces.
>  		  It just stays at the currently selected interface.
>  
> -   netretry	- When set to "no" each network operation will
> +  netretry	- When set to "no" each network operation will
>  		  either succeed or fail without retrying.
>  		  When set to "once" the network operation will
>  		  fail when all the available network interfaces
> @@ -3110,7 +3112,18 @@ Some configuration options can be set using Environment Variables:
>    tftpdstport	- If this is set, the value is used for TFTP's UDP
>  		  destination port instead of the Well Know Port 69.
>  
> -   vlan		- When set to a value < 4095 the traffic over
> +  tftpblocksize - Block size to use for TFTP transfers; if not set,
> +		  we use the TFTP server's default block size
> +
> +  tftptimeout	- Retransmission timeout for TFTP packets (in milli-
> +		  seconds, minimum value is 1000 = 1 second). Defines
> +		  when a packet is considered to be lost so it has to
> +		  be retransmitted. The default is 5000 = 5 seconds.
> +		  Lowering this value may make downloads succeed
> +		  faster in networks with high packet loss rates or
> +		  with unreliable TFTP servers.
> +
> +  vlan		- When set to a value < 4095 the traffic over
>  		  Ethernet is encapsulated/received over 802.1q
>  		  VLAN tagged frames.
>  
> diff --git a/net/tftp.c b/net/tftp.c
> index a02463b..3f402d1 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -211,7 +211,7 @@ TftpSend (void)
>  		pkt += 5 /*strlen("octet")*/ + 1;
>  		strcpy ((char *)pkt, "timeout");
>  		pkt += 7 /*strlen("timeout")*/ + 1;
> -		sprintf((char *)pkt, "%lu", TIMEOUT / 1000);
> +		sprintf((char *)pkt, "%lu", TftpTimeoutMSecs / 1000);
>  		debug("send option \"timeout %s\"\n", (char *)pkt);
>  		pkt += strlen((char *)pkt) + 1;
>  #ifdef CONFIG_TFTP_TSIZE
> @@ -413,7 +413,6 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
>  		}
>  
>  		TftpLastBlock = TftpBlock;
> -		TftpTimeoutMSecs = TIMEOUT;
>  		TftpTimeoutCountMax = TIMEOUT_COUNT;
>  		NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
>  
> @@ -528,10 +527,24 @@ TftpStart (void)
>  {
>  	char *ep;             /* Environment pointer */
>  
> -	/* Allow the user to choose tftpblocksize */
> +	/*
> +	 * Allow the user to choose TFTP blocksize and timeout.
> +	 * TFTP protocol has a minimal timeout of 1 second.
> +	 */
>  	if ((ep = getenv("tftpblocksize")) != NULL)
>  		TftpBlkSizeOption = simple_strtol(ep, NULL, 10);
> -	debug("tftp block size is %i\n", TftpBlkSizeOption);
> +
> +	if ((ep = getenv("tftptimeout")) != NULL)
> +		TftpTimeoutMSecs = simple_strtol(ep, NULL, 10);
> +
> +	if (TftpTimeoutMSecs < 1000) {
> +		printf("TFTP timeout (%ld ms) too low, "
> +			"set minimum = 1000 ms\n);
> +		TftpTimeoutMSecs = 1000;
> +	}
> +
> +	debug("TFTP blocksize = %i, timeout = %ld ms\n",
> +		TftpBlkSizeOption, TftpTimeoutMSecs);
>  
>  	TftpServerIP = NetServerIP;
>  	if (BootFile[0] == '\0') {
> @@ -588,7 +601,6 @@ TftpStart (void)
>  
>  	puts ("Loading: *\b");
>  
> -	TftpTimeoutMSecs = TftpRRQTimeoutMSecs;
>  	TftpTimeoutCountMax = TftpRRQTimeoutCountMax;
>  
>  	NetSetTimeout (TftpTimeoutMSecs, TftpTimeout);
> -- 

What's the status of this patch? Is it on your queue?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
They're usually so busy thinking about what  happens  next  that  the
only  time they ever find out what is happening now is when they come
to look back on it.                 - Terry Pratchett, _Wyrd Sisters_