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

[Ben Warren]

Hi Wolfgang,

On 3/11/2010 2:44 PM, Wolfgang Denk wrote:
> 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?
>    
So sorry - I thought I'd already taken care of this.  Please feel free 
to add my SOB and apply directly.

Signed-off-by: Ben Warren <biggerbadderben at gmail.com>

regards,
Ben